Bug 157433 - [COCOA] Disable HAVE_DTRACE at build time
Summary: [COCOA] Disable HAVE_DTRACE at build time
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-05-06 15:00 PDT by Chris Dumez
Modified: 2016-05-09 07:37 PDT (History)
6 users (show)

See Also:


Attachments
Patch (16.88 KB, patch)
2016-05-06 15:14 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (17.57 KB, patch)
2016-05-08 20:04 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (18.05 KB, patch)
2016-05-08 21:02 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-05-06 15:00:45 PDT
Disable HAVE_DTRACE at build time by default for performance reasons. Also remove the JSC-related code which seems unused.
Comment 1 Chris Dumez 2016-05-06 15:01:05 PDT
<rdar://problem/26148841>
Comment 2 Chris Dumez 2016-05-06 15:14:15 PDT
Created attachment 278278 [details]
Patch
Comment 3 Chris Dumez 2016-05-08 20:04:26 PDT
Created attachment 278385 [details]
Patch
Comment 4 Mark Lam 2016-05-08 20:59:02 PDT
The EWS bots are sad.  Please fix.
Comment 5 Chris Dumez 2016-05-08 21:02:38 PDT
Created attachment 278386 [details]
Patch
Comment 6 Mark Lam 2016-05-08 21:09:39 PDT
Comment on attachment 278386 [details]
Patch

r=me if the bots are happy.
Comment 7 WebKit Commit Bot 2016-05-08 21:54:49 PDT
Comment on attachment 278386 [details]
Patch

Clearing flags on attachment: 278386

Committed r200568: <http://trac.webkit.org/changeset/200568>
Comment 8 WebKit Commit Bot 2016-05-08 21:54:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 mitz 2016-05-08 22:02:21 PDT
(In reply to comment #7)
> Comment on attachment 278386 [details]
> Patch
> 
> Clearing flags on attachment: 278386
> 
> Committed r200568: <http://trac.webkit.org/changeset/200568>

Changes to WebKit2 need to be reviewed and or authored by one of the people listed in <http://trac.webkit.org/browser/trunk/Source/WebKit2/Owners?rev=198224>.
Comment 10 mitz 2016-05-08 22:47:09 PDT
Comment on attachment 278386 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=278386&action=review

> Source/WebKit2/Platform/IPC/MessageRecorder.cpp:30
> +#if HAVE(DTRACE)
> +

It seems like the complaint here was that UUID generation in the MessageRecorder constructor had a measurable impact on performance. The right way to address this would be to either use a cheaper unique identifier (a sequence number in the low bits and the process ID in the upper bits, for example) or the generate the unique identifiers only when the DTrace probe is enabled.

Pretending that OS X and iOS don’t have DTrace is an unnecessarily heavy-handed approach to take.
Comment 11 Chris Dumez 2016-05-09 07:37:57 PDT
(In reply to comment #10)
> Comment on attachment 278386 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=278386&action=review
> 
> > Source/WebKit2/Platform/IPC/MessageRecorder.cpp:30
> > +#if HAVE(DTRACE)
> > +
> 
> It seems like the complaint here was that UUID generation in the
> MessageRecorder constructor had a measurable impact on performance. The
> right way to address this would be to either use a cheaper unique identifier
> (a sequence number in the low bits and the process ID in the upper bits, for
> example) or the generate the unique identifiers only when the DTrace probe
> is enabled.
> 
> Pretending that OS X and iOS don’t have DTrace is an unnecessarily
> heavy-handed approach

Sure, we should make this less expensive if we need to re enable DTRACE. I did discuss this with Sam and Tim (who added this code) and they agreed to disable it for now. It does not seem people (besides Tim) were using this code, it has also never really worked on iOS as far as I know.