[webkit-reviews] review denied: [Bug 82325] Web Inspector: Speed up heap profiler snapshot parsing : [Attachment 134031] Patch

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied alexeif at chromium.org's request
for review:
Bug 82325: Web Inspector: Speed up heap profiler snapshot parsing
https://bugs.webkit.org/show_bug.cgi?id=82325

Attachment 134031: Patch
https://bugs.webkit.org/attachment.cgi?id=134031&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=134031&action=review


> Source/WebCore/inspector/front-end/HeapSnapshot.js:1227
> +	   var edgesCountOffset = this._edgesCountOffset;

We can just inline these as there is no performance gain.

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

Consider inlining these as well.

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1282
> +	   var indexArray = this._dominatedIndex = new Int32Array(nodeCount +
1);

Int32Array -> Uint32Array

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1291
> +	   var dominatedNodes = this._dominatedNodes = new Int32Array(nodeCount
+ 1);

Int32Array -> Uint32Array, length of the dominatedNodes array should be equal
to sum of all indexArrat values at this point, not nodeCount+1

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1389
> +	   var flags = this._flags;

Please inline these.

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1403
> +	       edge._edges = node.rawEdges;

Why not take node.edges? Could you at least introduce a public setter on the
edge?