Bug 139346 - Web Inspector: Enable runtime API for JSC's control flow profiler
Summary: Web Inspector: Enable runtime API for JSC's control flow profiler
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-12-06 12:47 PST by Saam Barati
Modified: 2014-12-08 21:01 PST (History)
6 users (show)

See Also:


Attachments
patch (5.41 KB, patch)
2014-12-06 13:14 PST, Saam Barati
joepeck: review+
joepeck: commit-queue-
Details | Formatted Diff | Diff
patch (5.45 KB, patch)
2014-12-08 18:23 PST, Saam Barati
no flags Details | Formatted Diff | Diff
patch (5.46 KB, patch)
2014-12-08 18:27 PST, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2014-12-06 12:47:21 PST
JSC now has control flow profiler functionality.
Create an inspector API for this feature.
Comment 1 Radar WebKit Bug Importer 2014-12-06 12:47:32 PST
<rdar://problem/19168297>
Comment 2 Saam Barati 2014-12-06 13:14:33 PST
Created attachment 242725 [details]
patch
Comment 3 Joseph Pecoraro 2014-12-08 12:14:19 PST
Comment on attachment 242725 [details]
patch

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

> Source/JavaScriptCore/inspector/protocol/Runtime.json:160
> +            "id": "BasicBlockLocation",
> +            "type": "object",
> +            "description": "Describes the location of a basic block and if that basic block has executed.",

What exactly is a "Basic Block"? That sounds like a JSC specific name, but is there a more generic name we could use here? Is there a language agnostic term for what a basic block is?

It sounds like this could just be a Type named "Basic Block" with a startOffset, endOffset, and hasExecuted. I don't think a "hasExecuted" property makes sense inside an object named "FooLocation".
Comment 4 Brian Burg 2014-12-08 12:17:28 PST
Comment on attachment 242725 [details]
patch

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

> Source/JavaScriptCore/ChangeLog:9
> +        to get information about which basic blocks have exectued

typo: executed

>> Source/JavaScriptCore/inspector/protocol/Runtime.json:160
>> +            "description": "Describes the location of a basic block and if that basic block has executed.",
> 
> What exactly is a "Basic Block"? That sounds like a JSC specific name, but is there a more generic name we could use here? Is there a language agnostic term for what a basic block is?
> 
> It sounds like this could just be a Type named "Basic Block" with a startOffset, endOffset, and hasExecuted. I don't think a "hasExecuted" property makes sense inside an object named "FooLocation".

(Almost) every compiler uses the term 'basic block'. A different name would be very confusing.
Comment 5 Joseph Pecoraro 2014-12-08 12:20:48 PST
> What exactly is a "Basic Block"? That sounds like a JSC specific name, but
> is there a more generic name we could use here? Is there a language agnostic
> term for what a basic block is?

Saam set me straight. This is the ideal name:
http://en.wikipedia.org/wiki/Basic_block
Comment 6 Saam Barati 2014-12-08 18:23:57 PST
Created attachment 242868 [details]
patch

Made the recommended name changes.
Comment 7 Saam Barati 2014-12-08 18:27:08 PST
Created attachment 242869 [details]
patch

(Same as above but w/ const variable that can be const).
Comment 8 WebKit Commit Bot 2014-12-08 21:01:30 PST
Comment on attachment 242869 [details]
patch

Clearing flags on attachment: 242869

Committed r177008: <http://trac.webkit.org/changeset/177008>
Comment 9 WebKit Commit Bot 2014-12-08 21:01:34 PST
All reviewed patches have been landed.  Closing bug.