Bug 89106 – Web Inspector: Implement native memory bar diagram

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Alexei Filippov]

We currently have native memory pie chart implemented.
Bar diagram has one advantage over a pie chart: you can see the absolute amount of memory allocated.

[Alexei Filippov]

Created attachment 147594 [details]
screenshot

[Alexei Filippov]

Created attachment 147597 [details]
Patch

[Ilya Tikhonovsky]

Comment on attachment 147597 [details]
Patch

I'd like to see Total at the top of the list and percentages

[Alexei Filippov]

Created attachment 147797 [details]
Patch

[Alexei Filippov]

Created attachment 147799 [details]
screenshot

[Alexei Filippov]

Created attachment 147830 [details]
Patch

[Alexei Filippov]

Created attachment 147831 [details]
screenshot

[Ilya Tikhonovsky]

Comment on attachment 147830 [details]
Patch

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

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:444
> +    _paint: function()

too long method. please split it.

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:495
> +                /*
> +                if (subchildSize < 0.95 * child.size)
> +                    sizeText = toMB(subchildSize).toFixed(1) + " of " + sizeText;
> +                */

We don't keep unused code in WebKit.

> Source/WebCore/inspector/front-end/ProfileLauncherView.js:56
> +    if (WebInspector.experimentsSettings.nativeMemorySnapshots.isEnabled()) {

please use separate experimental flag

[Ilya Tikhonovsky]

lgtm: please fix nits before landing

[Alexei Filippov]

Comment on attachment 147830 [details]
Patch

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

>> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:444
>> +    _paint: function()
> 
> too long method. please split it.

It's hard to split it as it has lots of dependencies. E.g. to move the loop body into a separate method would require it to have six arguments.
However I managed to make it a bit shorter.

>> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:495
>> +                */
> 
> We don't keep unused code in WebKit.

done.

>> Source/WebCore/inspector/front-end/ProfileLauncherView.js:56
>> +    if (WebInspector.experimentsSettings.nativeMemorySnapshots.isEnabled()) {
> 
> please use separate experimental flag

done

[Alexei Filippov]

Created attachment 147850 [details]
Patch

[Alexei Filippov]

Created attachment 147852 [details]
Patch

[Pavel Feldman]

Comment on attachment 147852 [details]
Patch

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

> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:475
> +        for (var i = 0; i < memoryBlock.children.length; ++i) {

You should render it using DOM instead!

[Alexei Filippov]

Created attachment 148109 [details]
Patch

[Alexei Filippov]

Comment on attachment 147852 [details]
Patch

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

>> Source/WebCore/inspector/front-end/NativeMemorySnapshotView.js:475
>> +        for (var i = 0; i < memoryBlock.children.length; ++i) {
> 
> You should render it using DOM instead!

Done

[Alexei Filippov]

Created attachment 148137 [details]
screenshot

[Ilya Tikhonovsky]

Comment on attachment 148109 [details]
Patch

Clearing flags on attachment: 148109

Committed r120621: <http://trac.webkit.org/changeset/120621>

[Ilya Tikhonovsky]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by 89518

[Alexei Filippov]

Created attachment 148522 [details]
Patch

[Alexei Filippov]

Put the live bars behind an experimental flag.

[WebKit Review Bot]

Comment on attachment 148522 [details]
Patch

Clearing flags on attachment: 148522

Committed r120819: <http://trac.webkit.org/changeset/120819>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

Does "native memory" include memory consumed by compositing layers?

[Yury Semikhatsky]

(In reply to comment #24)
> Does "native memory" include memory consumed by compositing layers?

By "native memory" we mean all memory consumed by the process owning the inspected Page. As described in http://wkb.ug/87262 our goal is to give the user a clear insight into where the memory goes in the inspected process.

If compositing layers are allocated in the inspected process then we would like to plot it on the chart. At the moment we show total RenderArena size for the inspected page. Could you point us at the places where the compositing layers are allocated so that we can add them to the diagram?

[Simon Fraser (smfr)]

You'll need to build up a comprehensive list of memory consumers that are outside the DOM. Some of these will be in subsystems outside of WebKit, so I'm not sure you'll be able to get them all.

Off the top of my head:

media elements (<audio> and <video>)
Web Audio data
image resources
plugins
canvas backing store
compositing layers
image buffers
caches in webcore code

Compositing layers are allocated via RenderLayerBacking. There's a backingStoreArea() method that returns the number of pixels. Multiply by 4 to get bytes (we could change this to return bytes).

You should file new bugs for the various memory consumers that you intend to track.