Bug 112569 - Web Inspector: use individual samples to construct CPU profile flame chart
Summary: Web Inspector: use individual samples to construct CPU profile flame chart
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-18 07:13 PDT by Yury Semikhatsky
Modified: 2013-03-22 04:03 PDT (History)
19 users (show)

See Also:


Attachments
Patch (14.43 KB, patch)
2013-03-18 07:16 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (14.32 KB, patch)
2013-03-18 07:18 PDT, Yury Semikhatsky
pfeldman: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (18.40 KB, patch)
2013-03-21 01:00 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (15.54 KB, patch)
2013-03-22 03:59 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2013-03-18 07:13:53 PDT
Now we build the chart based on the aggregated top-down tree. Using samples stream for that would allow to tie stack traces to the timeline.
Comment 1 Yury Semikhatsky 2013-03-18 07:16:24 PDT
Created attachment 193554 [details]
Patch
Comment 2 Yury Semikhatsky 2013-03-18 07:16:48 PDT
(In reply to comment #1)
> Created an attachment (id=193554) [details]
> Patch

This patch depends on the V8 change: https://codereview.chromium.org/12919002/ which is not landed yet.
Comment 3 Yury Semikhatsky 2013-03-18 07:18:38 PDT
Created attachment 193556 [details]
Patch
Comment 4 Ilya Tikhonovsky 2013-03-18 07:41:28 PDT
Comment on attachment 193556 [details]
Patch

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

> Source/WebCore/inspector/front-end/FlameChart.js:166
> +        var nodeCount = samples.length;

it is intervalCount and it is not equal to samples.length
profileNodeCount <= intervalCount <= samplesCount
So the arrays will use more memory than actually needed.

BTW duration and startTime are integers now.
Comment 5 WebKit Review Bot 2013-03-18 07:48:06 PDT
Comment on attachment 193556 [details]
Patch

Attachment 193556 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17160558
Comment 6 WebKit Review Bot 2013-03-18 08:11:56 PDT
Comment on attachment 193556 [details]
Patch

Attachment 193556 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17205550
Comment 7 Peter Beverloo (cr-android ews) 2013-03-18 08:19:56 PDT
Comment on attachment 193556 [details]
Patch

Attachment 193556 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17218396
Comment 8 Build Bot 2013-03-18 10:25:34 PDT
Comment on attachment 193556 [details]
Patch

Attachment 193556 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17236083

New failing tests:
inspector/profiler/cpu-profiler-profile-removal.html
Comment 9 Build Bot 2013-03-18 11:36:11 PDT
Comment on attachment 193556 [details]
Patch

Attachment 193556 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17201699

New failing tests:
inspector/profiler/cpu-profiler-profile-removal.html
Comment 10 Build Bot 2013-03-18 16:42:05 PDT
Comment on attachment 193556 [details]
Patch

Attachment 193556 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17218466

New failing tests:
inspector/profiler/cpu-profiler-profile-removal.html
Comment 11 Pavel Feldman 2013-03-19 09:49:49 PDT
Comment on attachment 193556 [details]
Patch

Please mind the test failures.
Comment 12 Yury Semikhatsky 2013-03-21 01:00:25 PDT
Created attachment 194198 [details]
Patch for landing
Comment 13 Yury Semikhatsky 2013-03-21 01:00:43 PDT
(In reply to comment #4)
> (From update of attachment 193556 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=193556&action=review
> 
> > Source/WebCore/inspector/front-end/FlameChart.js:166
> > +        var nodeCount = samples.length;
> 
> it is intervalCount and it is not equal to samples.length
> profileNodeCount <= intervalCount <= samplesCount
> So the arrays will use more memory than actually needed.
> 
> BTW duration and startTime are integers now.

Fixed.
Comment 14 Ilya Tikhonovsky 2013-03-21 01:18:53 PDT
Comment on attachment 194198 [details]
Patch for landing

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

> Source/WebCore/inspector/front-end/FlameChart.js:216
> +                ++index;

it has to be replaced with entries.length;
Comment 15 Yury Semikhatsky 2013-03-22 03:59:32 PDT
Created attachment 194503 [details]
Patch
Comment 16 Yury Semikhatsky 2013-03-22 04:03:19 PDT
Committed r146587: <http://trac.webkit.org/changeset/146587>