Summary: | Avoid composing the message + recipient name for crash logs until a failure actually occurs | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||||
Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | andersca, commit-queue, darin, mitz, simon.fraser, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Tim Horton
2017-11-10 23:33:51 PST
Created attachment 326681 [details]
Patch
Comment on attachment 326681 [details]
Patch
Not totally sure what the trailing colon is about, but I kept it.
Comment on attachment 326681 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326681&action=review > Source/WebKit/Platform/IPC/mac/ConnectionMac.mm:291 > + message->setMessageName(messageNameBuilder.toString().createCFString().get()); I wonder why MachMessage’s messageName needs to be a CFString. It looks like it doesn't, and seems likely that will make this even faster. That makes it a 90% reduction instead. But I think we can do yet better, and only compose the string in the failure case, instead of always. Created attachment 326682 [details]
Patch
Created attachment 326683 [details]
Patch
Comment on attachment 326683 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326683&action=review > Source/WebKit/Platform/IPC/mac/ConnectionMac.mm:250 > + WebKit::setCrashReportApplicationSpecificInformation((CFStringRef)[NSString stringWithFormat:@"Unhandled error code %x, message '%s::%s'", kr, message->messageReceiverName().data(), message->messageName().data()]); The :: makes more sense, but I wonder if the change in format would disturb some long-running data aggregation. Probably not! We'll see! I've only see this used for one-off investigations, though. Comment on attachment 326683 [details] Patch Clearing flags on attachment: 326683 Committed r224728: <https://trac.webkit.org/changeset/224728> All reviewed patches have been landed. Closing bug. Comment on attachment 326683 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326683&action=review > Source/WebKit/Platform/IPC/mac/MachMessage.h:47 > + void setMessageReceiverName(const CString& messageReceiverName) { m_messageReceiverName = messageReceiverName; } CString&& and WTFMove would give us slightly more efficient code here. > Source/WebKit/Platform/IPC/mac/MachMessage.h:50 > + void setMessageName(const CString& messageName) { m_messageName = messageName; } Ditto. (In reply to Darin Adler from comment #12) > Comment on attachment 326683 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=326683&action=review > > > Source/WebKit/Platform/IPC/mac/MachMessage.h:47 > > + void setMessageReceiverName(const CString& messageReceiverName) { m_messageReceiverName = messageReceiverName; } > > CString&& and WTFMove would give us slightly more efficient code here. > > > Source/WebKit/Platform/IPC/mac/MachMessage.h:50 > > + void setMessageName(const CString& messageName) { m_messageName = messageName; } > > Ditto. Did that in https://trac.webkit.org/changeset/224782/webkit |