Bug 109578 - Web Inspector: add initial implementation of native memory graph to Timeline
Summary: Web Inspector: add initial implementation of native memory graph to Timeline
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: Yury Semikhatsky
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-12 06:11 PST by Yury Semikhatsky
Modified: 2013-02-12 07:58 PST (History)
8 users (show)

See Also:


Attachments
Patch (22.29 KB, patch)
2013-02-12 06:16 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Screenshot with the patch applied (105.31 KB, image/png)
2013-02-12 06:17 PST, Yury Semikhatsky
no flags Details
Patch (22.26 KB, patch)
2013-02-12 07:06 PST, Yury Semikhatsky
apavlov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2013-02-12 06:11:30 PST
We'd like to show native memory size on the Timeline much like we already do with DOM counters.
Comment 1 Yury Semikhatsky 2013-02-12 06:16:58 PST
Created attachment 187853 [details]
Patch
Comment 2 Yury Semikhatsky 2013-02-12 06:17:48 PST
Created attachment 187855 [details]
Screenshot with the patch applied
Comment 3 Ilya Tikhonovsky 2013-02-12 06:37:50 PST
Comment on attachment 187853 [details]
Patch

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

looks good to me

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:50
> +    debugger;

please remove this line.

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:54
> +    hsl[2] -= 3;

magic constant

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:294
> +        valueGetter = function(counter) {
> +            return counter.total;
> +        }

why do yo need this function?
Comment 4 Yury Semikhatsky 2013-02-12 07:06:44 PST
Created attachment 187862 [details]
Patch
Comment 5 Yury Semikhatsky 2013-02-12 07:08:00 PST
(In reply to comment #3)
> (From update of attachment 187853 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=187853&action=review
> 
> looks good to me
> 
> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:50
> > +    debugger;
> 
> please remove this line.
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:54
> > +    hsl[2] -= 3;
> 
> magic constant
> 
Introduced constant.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:294
> > +        valueGetter = function(counter) {
> > +            return counter.total;
> > +        }
> 
> why do yo need this function?
Don't need, removed.
Comment 6 Alexander Pavlov (apavlov) 2013-02-12 07:25:08 PST
Comment on attachment 187862 [details]
Patch

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

> Source/WebCore/WebCore.vcproj/WebCore.vcproj:77345
> +					RelativePath="..\inspector\front-end\NativeMemoryGraph.js"

Move down below

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:53
> +    var borderLightnessDifference = 3;

const

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:103
> +        function getCounterValue(name, entry)

jsdoc?

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:115
> +                var counterUI = new WebInspector.NativeMemoryCounterUI(this, name, [i*20, 65, 63], getCounterValue.bind(this, name))

i * 20

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:127
> +    _onRecordAdded: function(event)

jsdoc here and below

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:140
> +            for (var n in nativeCounters) {

n -> name ?

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:199
> +            imageData: imageData };

"}" on the next line

> Source/WebCore/inspector/front-end/WebKit.qrc:116
> +    <file>NativeMemoryGraph.js</file>

Move down below
Comment 7 Yury Semikhatsky 2013-02-12 07:50:55 PST
(In reply to comment #6)
> (From update of attachment 187862 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=187862&action=review
> 
> > Source/WebCore/WebCore.vcproj/WebCore.vcproj:77345
> > +					RelativePath="..\inspector\front-end\NativeMemoryGraph.js"
> 
> Move down below
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:53
> > +    var borderLightnessDifference = 3;
> 
> const
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:103
> > +        function getCounterValue(name, entry)
> 
> jsdoc?
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:115
> > +                var counterUI = new WebInspector.NativeMemoryCounterUI(this, name, [i*20, 65, 63], getCounterValue.bind(this, name))
> 
> i * 20
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:127
> > +    _onRecordAdded: function(event)
> 
> jsdoc here and below
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:140
> > +            for (var n in nativeCounters) {
> 
> n -> name ?
> 
> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:199
> > +            imageData: imageData };
> 
> "}" on the next line
> 
Done.

> > Source/WebCore/inspector/front-end/WebKit.qrc:116
> > +    <file>NativeMemoryGraph.js</file>
> 
> Move down below
Done.
Comment 8 Yury Semikhatsky 2013-02-12 07:58:12 PST
Committed r142626: <http://trac.webkit.org/changeset/142626>