Summary: | Web Inspector: Speed up edges iteration in heap profiler | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexei Filippov <alph> | ||||||
Component: | Web Inspector (Deprecated) | Assignee: | Alexei Filippov <alph> | ||||||
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-05-23 11:00:03 PDT
Created attachment 143604 [details]
Patch
Comment on attachment 143604 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143604&action=review > Source/WebCore/ChangeLog:3 > + Web Inspector: Speed up edges iteration in heap profiler Does it have any noticeable impact on the performance? Please attach measurement results for the heap profiler perf test. > Source/WebCore/inspector/front-end/HeapSnapshot.js:724 > + this._nodesLength = this._nodes.length; _nodesLength name is confusing because _nodes.length is actually different, you can use this.nodeCount * this._nodeFieldCount for nodesLength where necessary or pick a better name that would reflect the difference with this._nodes.length Before: RESULT heap-snapshot: take-snapshot= 2169 ms RESULT heap-snapshot: transfer-snapshot= 1719 ms RESULT heap-snapshot: _buildRetainers= 448 ms RESULT heap-snapshot: _markDetachedDOMTreeNodes= 2 ms RESULT heap-snapshot: _markQueriableHeapObjects= 32 ms RESULT heap-snapshot: _markPageOwnedNodes= 43 ms RESULT heap-snapshot: _calculateFlags= 80 ms RESULT heap-snapshot: _calculateObjectToWindowDistance= 144 ms RESULT heap-snapshot: _buildPostOrderIndex= 73 ms RESULT heap-snapshot: _buildDominatorTree= 364 ms RESULT heap-snapshot: _calculateRetainedSizes= 83 ms RESULT heap-snapshot: _buildDominatedNodes= 17 ms RESULT heap-snapshot: show-snapshot= 2259 ms RESULT heap-snapshot: _buildAggregates= 631 ms RESULT heap-snapshot: _calculateClassesRetainedSize= 719 ms RESULT heap-snapshot: switch-to-containment-view= 1470 ms RESULT heap-snapshot: full-summary-snapshot-time= 7620 ms RESULT heap-snapshot: clear-snapshot= 58 ms RESULT heap-delta: heap-snapshot= 139 kB After: RESULT heap-snapshot: take-snapshot= 2139 ms RESULT heap-snapshot: transfer-snapshot= 1721 ms RESULT heap-snapshot: _buildRetainers= 86 ms RESULT heap-snapshot: _markDetachedDOMTreeNodes= 2 ms RESULT heap-snapshot: _markQueriableHeapObjects= 33 ms RESULT heap-snapshot: _markPageOwnedNodes= 44 ms RESULT heap-snapshot: _calculateFlags= 83 ms RESULT heap-snapshot: _calculateObjectToWindowDistance= 163 ms RESULT heap-snapshot: _buildPostOrderIndex= 78 ms RESULT heap-snapshot: _buildDominatorTree= 401 ms RESULT heap-snapshot: _calculateRetainedSizes= 83 ms RESULT heap-snapshot: _buildDominatedNodes= 17 ms RESULT heap-snapshot: show-snapshot= 2077 ms RESULT heap-snapshot: _buildAggregates= 615 ms RESULT heap-snapshot: _calculateClassesRetainedSize= 719 ms RESULT heap-snapshot: switch-to-containment-view= 1443 ms RESULT heap-snapshot: full-summary-snapshot-time= 7382 ms RESULT heap-snapshot: clear-snapshot= 69 ms RESULT heap-delta: heap-snapshot= 142 kB Created attachment 143855 [details]
Patch
Comment on attachment 143855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143855&action=review > Source/WebCore/inspector/front-end/HeapSnapshot.js:724 > + this._realNodesLength = this._nodes.length; I'd call it just _nodesLength > Source/WebCore/inspector/front-end/HeapSnapshot.js:730 > + this._nodes = new Uint32Array(this._realNodesLength + this._nodeFieldCount); Actually _nodes array has extra elements that were stripped in WebInspector.Uint32Array.array getter. Thus we can avoid additional array reallocation here. Comment on attachment 143855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143855&action=review >> Source/WebCore/inspector/front-end/HeapSnapshot.js:730 >> + this._nodes = new Uint32Array(this._realNodesLength + this._nodeFieldCount); > > Actually _nodes array has extra elements that were stripped in WebInspector.Uint32Array.array getter. > Thus we can avoid additional array reallocation here. Can we pass an array with nodeLength + nodeFieldCount elements so that we don't need to allocate new array? Comment on attachment 143855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143855&action=review >> Source/WebCore/inspector/front-end/HeapSnapshot.js:724 >> + this._realNodesLength = this._nodes.length; > > I'd call it just _nodesLength I'd too, but Yury asked to rename it. >>> Source/WebCore/inspector/front-end/HeapSnapshot.js:730 >>> + this._nodes = new Uint32Array(this._realNodesLength + this._nodeFieldCount); >> >> Actually _nodes array has extra elements that were stripped in WebInspector.Uint32Array.array getter. >> Thus we can avoid additional array reallocation here. > > Can we pass an array with nodeLength + nodeFieldCount elements so that we don't need to allocate new array? @Ilya actually it does not, because the exact number of nodes is allocated in HeapSnapshotLoader. @Yury That was my initial implementation, but I didn't like it because in this case the logic has to go into the HeapSnapshotLoader. (In reply to comment #7) > @Yury That was my initial implementation, but I didn't like it because in this case the logic has to go into the HeapSnapshotLoader. Do you have numbers of what it costs us? Committed r118528: <http://trac.webkit.org/changeset/118528> (In reply to comment #8) > (In reply to comment #7) > > @Yury That was my initial implementation, but I didn't like it because in this case the logic has to go into the HeapSnapshotLoader. > > Do you have numbers of what it costs us? it cost us 11ms |