| Summary: | Web Inspector: Speed up heap profiler snapshot parsing | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| 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, yurys | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
|
Description
Alexei Filippov
2012-03-27 05:03:21 PDT
Created attachment 134031 [details]
Patch
Comment on attachment 134031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134031&action=review > Source/WebCore/inspector/front-end/HeapSnapshot.js:1231 > + for (var i = 1, l = nodes.length; i < l; ++count) { rootNodeIndex? > Source/WebCore/inspector/front-end/HeapSnapshot.js:1238 > + for (var i = 1, l = nodes.length; i < l; ++count) { ditto > Source/WebCore/inspector/front-end/HeapSnapshot.js:1282 > + var indexArray = this._dominatedIndex = new Int32Array(nodeCount + 1); Uint32Array > Source/WebCore/inspector/front-end/HeapSnapshot.js:1291 > + var dominatedNodes = this._dominatedNodes = new Int32Array(nodeCount + 1); ditto Comment on attachment 134031 [details] Patch 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? Comment on attachment 134031 [details] Patch 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. done >> Source/WebCore/inspector/front-end/HeapSnapshot.js:1231 >> + for (var i = 1, l = nodes.length; i < l; ++count) { > > rootNodeIndex? done >> Source/WebCore/inspector/front-end/HeapSnapshot.js:1238 >> + for (var i = 1, l = nodes.length; i < l; ++count) { > > ditto done >> Source/WebCore/inspector/front-end/HeapSnapshot.js:1272 >> + var nodeIndexes = this.nodeIndexes; > > Consider inlining these as well. done >>> Source/WebCore/inspector/front-end/HeapSnapshot.js:1291 >>> + var dominatedNodes = this._dominatedNodes = new Int32Array(nodeCount + 1); >> >> ditto > > Int32Array -> Uint32Array, length of the dominatedNodes array should be equal to sum of all indexArrat values at this point, not nodeCount+1 it is nodeCount - 1, as each node except for root has one dominator. >> Source/WebCore/inspector/front-end/HeapSnapshot.js:1389 >> + var flags = this._flags; > > Please inline these. done >> 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? node.edges is an iterator which I'd like to avoid. Don't wanna introduce a setter, we don't have them for nodeIndex and edgeIndex. Created attachment 134056 [details]
Patch
Comment on attachment 134056 [details] Patch Clearing flags on attachment: 134056 Committed r112271: <http://trac.webkit.org/changeset/112271> All reviewed patches have been landed. Closing bug. |