Bug 103564 – Native Memory Instrumentation: instrument a part of RenderObject class tree.

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

[Ilya Tikhonovsky]

Many RenderObject descendants have OwnPtrs and RefPtrs members.
Looks like we have to instrument all the RenderObject classes and visit
all the render objects instead of counting entire render arena memory.
The only think we need to do is count render arena free size.

[Ilya Tikhonovsky]

Created attachment 176590 [details]
Patch

[Early Warning System Bot]

Comment on attachment 176590 [details]
Patch

Attachment 176590 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15028592

[Early Warning System Bot]

Comment on attachment 176590 [details]
Patch

Attachment 176590 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15014681

[EFL EWS Bot]

Comment on attachment 176590 [details]
Patch

Attachment 176590 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15018631

[Build Bot]

Comment on attachment 176590 [details]
Patch

Attachment 176590 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15027597

[Ilya Tikhonovsky]

Created attachment 176607 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 176607 [details]
Patch

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

I agree this is the right approach (since the RenderArena won't end up counting everything hung off the rendering tree).  But I will note that this current patch is far from complete. :)  Definitely many many more RenderObject subclasses to implement here.

Should we be separating out the layer tree from the rendering tree?

Similarly the linebox trees might be interesting to count separately. :)

> Source/WebCore/rendering/RenderView.cpp:1006
> +    info.addMember(m_widgets);

Should these be counted as owned by the RenderView?

> Source/WebCore/rendering/RenderView.cpp:1007
> +    info.addWeakPointer(m_layoutState);

The RenderView owns the layout state, doesn't it?  It will die if the RenderView does?  (It's ephemeral and only used during layout())

[Simon Fraser (smfr)]

Comment on attachment 176607 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Web Inspector: NMI instrument a part of RenderObject class tree.

I don't know what "NMI" means. Please expand it.

> Source/WebCore/platform/graphics/GraphicsLayer.h:429
> +    virtual void reportMemoryUsage(MemoryObjectInfo*) const;

Did you see backingStoreMemoryEstimate()?

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:895
> +    MemoryClassInfo info(memoryObjectInfo, this, PlatformMemoryTypes::Rendering);
> +    GraphicsLayer::reportMemoryUsage(memoryObjectInfo);
> +    info.addMember(m_nameBase);
> +    info.addMember(m_layer);
> +    info.addMember(m_transformLayer);
> +    info.addMember(m_imageLayer);
> +    info.addMember(m_contentsLayer);
> +    info.addMember(m_linkHighlight);
> +    info.addMember(m_opaqueRectTrackingContentLayerDelegate);
> +    info.addMember(m_animationIdMap);
> +    info.addMember(m_scrollableArea);

I don't know if any of this accounts for the actual backing store, which is by far the largest memory consumer.

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.h:155
> +    virtual void reportMemoryUsage(MemoryObjectInfo*) const OVERRIDE;

What about the other platform's GraphicsLayers?

[Ilya Tikhonovsky]

(In reply to comment #7)
> (From update of attachment 176607 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=176607&action=review
> 
> I agree this is the right approach (since the RenderArena won't end up counting everything hung off the rendering tree).  But I will note that this current patch is far from complete. :)  Definitely many many more RenderObject subclasses to implement here.

Yeah. The difference between RenderArena size and Page.Rendering size is quite big at the moment.
It is about 57kb vs 27kb on ntp and 744kb vs 397 on gmail. I'd like to keep the size of patch relatively small.

The main reason of the patch is the fact that there is no other ways to reach GraphicsLayer objects.


> 
> Should we be separating out the layer tree from the rendering tree?
> Similarly the linebox trees might be interesting to count separately. :)

Will do.

> 
> 
> > Source/WebCore/rendering/RenderView.cpp:1006
> > +    info.addMember(m_widgets);
> 
> Should these be counted as owned by the RenderView?

Actually we have no "Ownership" at the moment.
It is counting as sizeof with type Rendering because it has no reportMemoryUsage method.

> 
> > Source/WebCore/rendering/RenderView.cpp:1007
> > +    info.addWeakPointer(m_layoutState);
> 
> The RenderView owns the layout state, doesn't it?  It will die if the RenderView does?  (It's ephemeral and only used during layout())

I missed the fact that LayoutState could be allocated but am I right that it is not null only during layout?

[Build Bot]

Comment on attachment 176607 [details]
Patch

Attachment 176607 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15010720

[Build Bot]

Comment on attachment 176607 [details]
Patch

Attachment 176607 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15018660

[kov's GTK+ EWS bot]

Comment on attachment 176607 [details]
Patch

Attachment 176607 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15044081

[Ilya Tikhonovsky]

(In reply to comment #8)
> (From update of attachment 176607 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=176607&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        Web Inspector: NMI instrument a part of RenderObject class tree.
> 
> I don't know what "NMI" means. Please expand it.

It is Native Memory Instrumentation.  I could use this prefix instead of Web Inspector. wdyt?

> 
> > Source/WebCore/platform/graphics/GraphicsLayer.h:429
> > +    virtual void reportMemoryUsage(MemoryObjectInfo*) const;
> 
> Did you see backingStoreMemoryEstimate()?

It  gives us an estimation about used memory but doesn't provide precise data and actual pointers to the memory allocated fragments. Pointers give us a guaranty that count a fragment only once. Also we use them for validating the collected data against tcmalloc. I'll use backingStoreMemoryEstimate if the actual buffers aren't accessible by some reason. 

> 
> > Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:895
> > +    MemoryClassInfo info(memoryObjectInfo, this, PlatformMemoryTypes::Rendering);
> > +    GraphicsLayer::reportMemoryUsage(memoryObjectInfo);
> > +    info.addMember(m_nameBase);
> > +    info.addMember(m_layer);
> > +    info.addMember(m_transformLayer);
> > +    info.addMember(m_imageLayer);
> > +    info.addMember(m_contentsLayer);
> > +    info.addMember(m_linkHighlight);
> > +    info.addMember(m_opaqueRectTrackingContentLayerDelegate);
> > +    info.addMember(m_animationIdMap);
> > +    info.addMember(m_scrollableArea);
> 
> I don't know if any of this accounts for the actual backing store, which is by far the largest memory consumer.

Current approach gives us good results and requires very simple instrumentation. You can do this almost mechanically. You need to calculate actual numbers manually when you manually control the sizes like here https://code.google.com/p/chromium/source/search?q=addRawBuffer+file%3ACSSSelector&origq=addRawBuffer+file%3ACSSSelector&btnG=Search+Trunk

or here https://code.google.com/p/chromium/source/search?q=addRawBuffer+file%3ASharedBuffer&origq=addRawBuffer+file%3ASharedBuffer&btnG=Search+Trunk

The only problem is that you need to instrument almost everything. :)
Unfortunately it is not possible to do this automatically even for simple classes because not all the platforms use clang.

> 
> > Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.h:155
> > +    virtual void reportMemoryUsage(MemoryObjectInfo*) const OVERRIDE;
> 
> What about the other platform's GraphicsLayers?

I'd like to instrument one platform at a time. It will be easier to instrument platform specific code for the other platforms when the instrumentation become stable and tools become mature. I hope someone else help us to instrument nontrivial code for other platforms.

[Ilya Tikhonovsky]

Created attachment 176627 [details]
Patch

[Build Bot]

Comment on attachment 176627 [details]
Patch

Attachment 176627 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15026671

[EFL EWS Bot]

Comment on attachment 176627 [details]
Patch

Attachment 176627 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15031498

[Ilya Tikhonovsky]

Created attachment 176756 [details]
with fix for mac and elf

[Ilya Tikhonovsky]

Created attachment 176757 [details]
second with fix for mac and elf

[Simon Fraser (smfr)]

(In reply to comment #13)
> (In reply to comment #8)
> > (From update of attachment 176607 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=176607&action=review
> > 
> > > Source/WebCore/ChangeLog:3
> > > +        Web Inspector: NMI instrument a part of RenderObject class tree.
> > 
> > I don't know what "NMI" means. Please expand it.
> 
> It is Native Memory Instrumentation.  I could use this prefix instead of Web Inspector. wdyt?

I prefer Web Inspector.

> > > Source/WebCore/platform/graphics/GraphicsLayer.h:429
> > > +    virtual void reportMemoryUsage(MemoryObjectInfo*) const;
> > 
> > Did you see backingStoreMemoryEstimate()?
> 
> It  gives us an estimation about used memory but doesn't provide precise data and actual pointers to the memory allocated fragments. Pointers give us a guaranty that count a fragment only once. Also we use them for validating the collected data against tcmalloc. I'll use backingStoreMemoryEstimate if the actual buffers aren't accessible by some reason. 

On Mac, at least, you can't get to the actual pointers. You'll find the same when you try to account for image data (CGImageRefs).

[Ilya Tikhonovsky]

(In reply to comment #19)
> (In reply to comment #13)
> > (In reply to comment #8)
> > > (From update of attachment 176607 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=176607&action=review
> > > 
> > > > Source/WebCore/ChangeLog:3
> > > > +        Web Inspector: NMI instrument a part of RenderObject class tree.
> > > 
> > > I don't know what "NMI" means. Please expand it.
> > 
> > It is Native Memory Instrumentation.  I could use this prefix instead of Web Inspector. wdyt?
> 
> I prefer Web Inspector.
> 
> > > > Source/WebCore/platform/graphics/GraphicsLayer.h:429
> > > > +    virtual void reportMemoryUsage(MemoryObjectInfo*) const;
> > > 
> > > Did you see backingStoreMemoryEstimate()?
> > 
> > It  gives us an estimation about used memory but doesn't provide precise data and actual pointers to the memory allocated fragments. Pointers give us a guaranty that count a fragment only once. Also we use them for validating the collected data against tcmalloc. I'll use backingStoreMemoryEstimate if the actual buffers aren't accessible by some reason. 
> 
> On Mac, at least, you can't get to the actual pointers. You'll find the same when you try to account for image data (CGImageRefs).

Yeah. The same situation happens with WTF containers because Maciej is against intrusive instrumentation of them.

If an object reports a member pointer to a child object then MemoryInstrumentation calls reportMemoryUsage method on this child object or uses sizeof if the child object has no such method.

If an object has a pointer to something (CGImage*) and we know that it uses N megs of memory but we have no chances to instrument it then I will report a raw pointer with manually counted size via addRawBuffer.

If an object knows just size but has no access to the actual pointer then I use addPrivateBuffer and it associates this size with the object.

[Yury Semikhatsky]

Comment on attachment 176757 [details]
second with fix for mac and elf

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

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:-449
> -    memoryInstrumentationClient.countObjectSize(0, WebCoreMemoryTypes::RenderTreeUsed, arenaSize.treeSize);

Let's keep using arenaSize.treeSize while it is more accurate than current instrumentation of render objects.

> Source/WebCore/rendering/RenderLayer.h:713
> +    void reportMemoryUsage(MemoryObjectInfo*) const;

Shouldn't it be virtual?

> Source/WebCore/rendering/RenderLayerBacking.h:187
> +    void reportMemoryUsage(MemoryObjectInfo*) const;

Shouldn't it be virtual?

[Yury Semikhatsky]

(In reply to comment #9)
> (In reply to comment #7)
> > (From update of attachment 176607 [details] [details])
> > > Source/WebCore/rendering/RenderView.cpp:1006
> > > +    info.addMember(m_widgets);
> > 
> > Should these be counted as owned by the RenderView?
> 
> Actually we have no "Ownership" at the moment.
> It is counting as sizeof with type Rendering because it has no reportMemoryUsage method.
> 

At the moment the memory instrumentation doesn't provide any means to discriminate between owning and and non-owning references except for reportWeakPointer. We are going to change this soon and allow to report additional information about links between objects including whether the connection is one that should be considered as ownership.

We are also planning to extend memory instrumentation so that it produces a heap graph with additional information about certain heap objects. Based on that graph we can both build current high-level aggregated statistics of the memory distribution and additionally expand expand components to see memory consumption of select individual objects(e.g. if CachedImage consumes X Mb it may make sense to show how many of those are occupied by pixel buffers).

[Ilya Tikhonovsky]

Created attachment 176979 [details]
Patch

[Ilya Tikhonovsky]

Created attachment 176981 [details]
Patch

[Ilya Tikhonovsky]

2eric: comments addressed

2yurys: comments addressed


> > Source/WebCore/rendering/RenderLayerBacking.h:187
> > +    void reportMemoryUsage(MemoryObjectInfo*) const;
> 
> Shouldn't it be virtual?

No. It is a class with no descendants.

[EFL EWS Bot]

Comment on attachment 176981 [details]
Patch

Attachment 176981 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15065270

[Build Bot]

Comment on attachment 176981 [details]
Patch

Attachment 176981 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15065269

[Ilya Tikhonovsky]

Created attachment 176991 [details]
Patch

[Early Warning System Bot]

Comment on attachment 176991 [details]
Patch

Attachment 176991 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15054497

[Early Warning System Bot]

Comment on attachment 176991 [details]
Patch

Attachment 176991 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15064331

[EFL EWS Bot]

Comment on attachment 176991 [details]
Patch

Attachment 176991 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15044865

[Ilya Tikhonovsky]

Created attachment 176998 [details]
next try

[Eric Seidel (no email)]

Comment on attachment 176998 [details]
next try

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

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:89
> +        // FIXME: We filter out Rendering type because the coverage is not good enough at the moment
> +        // and report RenderArena size instead.

Do you mean to just filter out Rendering and still count layers?

[Ilya Tikhonovsky]

Comment on attachment 176998 [details]
next try

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

>> Source/WebCore/inspector/InspectorMemoryAgent.cpp:89
>> +        // and report RenderArena size instead.
> 
> Do you mean to just filter out Rendering and still count layers?

yes. If the map has Page.Rendering.Layers size then this method generates Rendering item and puts Layers leaf under it.

[Ilya Tikhonovsky]

(In reply to comment #33)
> (From update of attachment 176998 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=176998&action=review
> 
> > Source/WebCore/inspector/InspectorMemoryAgent.cpp:89
> > +        // FIXME: We filter out Rendering type because the coverage is not good enough at the moment
> > +        // and report RenderArena size instead.
> 
> Do you mean to just filter out Rendering and still count layers?

We have two kind of layers RenderLayer and GraphicsLayer and I use Layer type only for GraphicsLayers.
Probably Page.Rendering.GraphicsLayers sounds better than Page.Rendering.Layers. wdyt?

[Simon Fraser (smfr)]

Is this tree exposed to end users, or is it just for some accounting that gets rolled up to some summary number?

[Ilya Tikhonovsky]

Created attachment 177041 [details]
screenshot

it is exposed to the end user

[Ilya Tikhonovsky]

2 eric && simon do you have any other comments or concerns?

[Simon Fraser (smfr)]

How is the average web developer supposed to know what all these things are?

[Ilya Tikhonovsky]

(In reply to comment #39)
> How is the average web developer supposed to know what all these things are?

Initially we thought that a top level overview would be enough for the web developer.
But now when we see these generic numbers it is clear that they need to see more precise data like 'how much memory was used for an image with this url'.

This problem could be solved by the same memory instrumentation code which traverses the heap objects graph. Yurys is preparing a patch which extends memory instrumentation client api and gives us ability to generate a graph with objects, their names, sizes etc. We have solved the same problem for javascript heap and know what we need on frontend UI side.

We are in MtV until Thursday. If you have questions or suggestions we could meet together in your or our office.

[Simon Fraser (smfr)]

(In reply to comment #40)
> (In reply to comment #39)
> > How is the average web developer supposed to know what all these things are?
> 
> Initially we thought that a top level overview would be enough for the web developer.
> But now when we see these generic numbers it is clear that they need to see more precise data like 'how much memory was used for an image with this url'.

Sure, but you need to present it in a way that developers understand, and relate to their content. Showing that "Layers" take 34KB doesn't tell them a thing. Saying that "using overflow:hidden on thousands of divs increases memory use" does. Don't force developers to learn WebKit just to optimize their site; tell them what they need to know to improve their content.

[Eric Seidel (no email)]

Personally I find this data interesting, even if it never makes it to a user-facing feature.  I think Simon's point about finding a way to expose this to a Web Developer in a way they understand is a good one, and important.  But I find this data (and this experimental feature) useful even as-is.  I'm personally interested in using this to work on memory leaks in WebCore as it gets more mature.

I guess another way to get what I want might be to turn on RTTI and turn off tcmalloc and recompile :p  But I really like the idea of being able to expose this knowledge to some of the 1% customers of WebKit who are already clamoring for more information about our internals (and already understand WebKit much better than we might think).  Again, I agree, the graphs as currently exposed are likely confusing to the 99%.

[Yury Semikhatsky]

(In reply to comment #41)
> (In reply to comment #40)
> > (In reply to comment #39)
> > > How is the average web developer supposed to know what all these things are?
> > 
> > Initially we thought that a top level overview would be enough for the web developer.
> > But now when we see these generic numbers it is clear that they need to see more precise data like 'how much memory was used for an image with this url'.
> 
> Sure, but you need to present it in a way that developers understand, and relate to their content. Showing that "Layers" take 34KB doesn't tell them a thing. Saying that "using overflow:hidden on thousands of divs increases memory use" does. Don't force developers to learn WebKit just to optimize their site; tell them what they need to know to improve their content.

The plan was to at first show high-level overview of the memory usage by various components, preferably in a format that can be understood by web developers who don't develop WebKit. To make this information more useful for web developers we should expose it in therms that they understand and show how their code/resources expand into the WebKit structures whose footprint we measure. In some cases it should be quite easy - e.g. show that a pixelbuffer was allocated for an image with given url, in other it may be more more tricky like in the Simon's example. I don't think it is possible to provide a generic solution for this but  we can build a heap graph and annotate some of its nodes so that it can be later processed by inspector code to identify possible places for optimizations and present them in a user-friendly way to web developers.

[Eric Seidel (no email)]

Comment on attachment 176998 [details]
next try

This seems reasonable to me. I'm sure this will require many more iterations of refinement as we start to be able to use the tool to better understand memory.

[Ilya Tikhonovsky]

Committed r137108: <http://trac.webkit.org/changeset/137108>