Bug 87382 - Web Inspector: convert HeapSnapshotGridNode._provider into getter.
Summary: Web Inspector: convert HeapSnapshotGridNode._provider into getter.
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: Ilya Tikhonovsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-24 06:00 PDT by Ilya Tikhonovsky
Modified: 2012-05-24 06:38 PDT (History)
15 users (show)

See Also:


Attachments
Patch (13.46 KB, patch)
2012-05-24 06:09 PDT, Ilya Tikhonovsky
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2012-05-24 06:00:54 PDT
I found that we create a provider for child nodes for the each DataGrid node in advance.
It cost us one async call to the HeapSnapshot's worker per each such the node.
I converted the property into the getter which is initializing the provider lazily.
Comment 1 Yury Semikhatsky 2012-05-24 06:05:47 PDT
You should be careful not to break DataGridNodeNode.hasChildren functionality for the heap profiler nodes. Some nodes use their providers to learn if it has children or not so the provider should exist when the node is being displayed.
Comment 2 Ilya Tikhonovsky 2012-05-24 06:09:22 PDT
Created attachment 143809 [details]
Patch
Comment 3 Yury Semikhatsky 2012-05-24 06:16:54 PDT
Comment on attachment 143809 [details]
Patch

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

> Source/WebCore/bindings/v8/ScriptHeapSnapshot.cpp:71
> +    int GetChunkSize() { return 102400; }

Please revert this.

> Source/WebCore/inspector/front-end/HeapSnapshotGridNodes.js:54
> +     * @return {WebInspector.HeapSnapshotProviderProxy|WebInspector.HeapSnapshotDiffNodesProvider}

WebInspector.HeapSnapshotDiffNodesProvider now extends WebInspector.HeapSnapshotProviderProxy so the annotation can be simplified.

> Source/WebCore/inspector/front-end/HeapSnapshotGridNodes.js:56
> +    _createProvider: function()

This should be public as it is overridden in the descendants.

> Source/WebCore/inspector/front-end/HeapSnapshotGridNodes.js:61
> +    get _provider()

Can you used provider() function instead of a getter?
Comment 4 Yury Semikhatsky 2012-05-24 06:17:37 PDT
What is the performance impact of this change?
Comment 5 Ilya Tikhonovsky 2012-05-24 06:38:18 PDT
Committed r118361: <http://trac.webkit.org/changeset/118361>