Bug 81763

Summary: Web Inspector: build retainers phase it too slow in heap profiler
Product: WebKit Reporter: Alexei Filippov <alph>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 81912    
Bug Blocks: 78411    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Alexei Filippov 2012-03-21 05:04:54 PDT 
the phase takes more than 4 secs on gmail.com
Comment 1 Alexei Filippov 2012-03-21 05:33:27 PDT 
Created attachment 133023 [details]
Patch
Comment 2 Ilya Tikhonovsky 2012-03-21 06:56:13 PDT 
Comment on attachment 133023 [details]
Patch

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

looks good with nits

> Source/WebCore/inspector/front-end/HeapSnapshot.js:969
> +        if (!this._nodeIndex)
> +            this._buildNodeIndex();

unnecessary check

> Source/WebCore/inspector/front-end/HeapSnapshot.js:976
> +        var indexArray = this["_retainerIndex"] = new Int32Array(this._nodeIndex.length);

nodeCount

> Source/WebCore/inspector/front-end/HeapSnapshot.js:979
> +        var nodeCount = this.nodeCount;

move it 4 lines up

> Source/WebCore/inspector/front-end/HeapSnapshot.js:998
> +        for (i = 0, l = indexArray.length; i < l; ++i)

nodeCount

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1004
> +        for (i = 0, l = indexArray.length; i < l; ++i) {

ditto
Comment 3 Alexei Filippov 2012-03-21 07:53:32 PDT 
Created attachment 133041 [details]
Patch
Comment 4 Yury Semikhatsky 2012-03-21 08:09:51 PDT 
Comment on attachment 133023 [details]
Patch

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

>> Source/WebCore/inspector/front-end/HeapSnapshot.js:976
>> +        var indexArray = this["_retainerIndex"] = new Int32Array(this._nodeIndex.length);
> 
> nodeCount

Please use this._retainerIndex syntax

>> Source/WebCore/inspector/front-end/HeapSnapshot.js:979
>> +        var nodeCount = this.nodeCount;
> 
> move it 4 lines up

Remember that this._nodeIndex.length !== nodeCount :)

> Source/WebCore/inspector/front-end/HeapSnapshot.js:999
> +            backRefsCount += indexArray[i];

backRefsCount === total edges count so you can calculate this in the previous loop and avoid additional pass through all elements.

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1017
> +                var retNode = nodePositions[nodes[j * edgeFieldsCount + edgesOffset + edgeToNodeOffset]];

j * edgeFieldsCount + edgesOffset + edgeToNodeOffset -> edgeIndex + edgeToNodeOffset
Comment 5 Pavel Feldman 2012-03-21 08:33:57 PDT 
I would like to see a test with this.
Comment 6 Alexei Filippov 2012-03-21 11:14:45 PDT 
Created attachment 133080 [details]
Patch
Comment 7 WebKit Review Bot 2012-03-21 12:05:58 PDT 
Comment on attachment 133080 [details]
Patch

Rejecting attachment 133080 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
 /mnt/git/webkit-commit-queue/

Parsed 2 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/inspector/front-end/HeapSnapshot.js
Hunk #1 FAILED at 965.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/inspector/front-end/HeapSnapshot.js.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Yury Semik..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12071554
Comment 8 Alexei Filippov 2012-03-22 06:57:13 PDT 
Created attachment 133249 [details]
Patch
Comment 9 Yury Semikhatsky 2012-03-22 07:12:42 PDT 
Comment on attachment 133249 [details]
Patch

Clearing flags on attachment: 133249

Committed r111688: <http://trac.webkit.org/changeset/111688>
Comment 10 Yury Semikhatsky 2012-03-22 07:12:50 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 11 Alexei Filippov 2012-03-26 07:22:32 PDT 
Reopening to attach new patch.
Comment 12 Alexei Filippov 2012-03-26 07:22:37 PDT 
Created attachment 133804 [details]
Patch
Comment 13 Yury Semikhatsky 2012-03-26 07:33:03 PDT 
Comment on attachment 133804 [details]
Patch

Clearing flags on attachment: 133804

Committed r112090: <http://trac.webkit.org/changeset/112090>
Comment 14 Yury Semikhatsky 2012-03-26 07:33:12 PDT 
All reviewed patches have been landed.  Closing bug.