Bug 179576 – Avoid composing the message + recipient name for crash logs until a failure actually occurs

• Component: New Bugs
• Status: RESOLVED FIXED

[Tim Horton]

Use StringBuilder instead of NSString to build the full message name for crash logs

[Tim Horton]

Created attachment 326681 [details]
Patch

[Tim Horton]

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.

[Tim Horton]

It looks like it doesn't, and seems likely that will make this even faster.

[Tim Horton]

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.

[Tim Horton]

Created attachment 326682 [details]
Patch

[Tim Horton]

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!

[Tim Horton]

We'll see! I've only see this used for one-off investigations, though.

[WebKit Commit Bot]

Comment on attachment 326683 [details]
Patch

Clearing flags on attachment: 326683

Committed r224728: <https://trac.webkit.org/changeset/224728>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

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.

[Tim Horton]

(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

[Radar WebKit Bug Importer]

<rdar://problem/35561994>