Currently we have a plain memory usage statistic based on types. Unfortunately this 'type' to 'size' information couldn't help to web developers. They need an information about instances and owners. As example even about:blank page has ~5MB of Page.Image. This fact confuses the developers. Actually this image is a part of Inspector backend because it creates and uses a canvas in InspectorOverlay. After long offline discussion we came to the conclusion that we need to record a heap graph. We need to have 2 methods for that. reportNode and reportEdge. But due to c++ multiple inheritance we need a helper method reportAssociation which associates a pointer to parent class with pointer to descendant class. patch to follow.
Created attachment 175966 [details] Patch
Comment on attachment 175966 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175966&action=review Let's split the patch into smaller parts. > Source/WTF/ChangeLog:8 > + reportNode, reportEdge and reportAssociation were added to the client API There is no reportAssociation. > Source/WTF/ChangeLog:10 > + InstrumentedPointerBase and InstrumentedPointer<T> wer renamed to PointerBase wer-> were > Source/WTF/ChangeLog:13 > + Pointer<T>::m_pointer was upstreamed to PointerBase because it needs to PointerBase::process method. it needs to PointerBase::process method -> PointerBase::process uses it
Comment on attachment 175966 [details] Patch Attachment 175966 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14981638
Comment on attachment 175966 [details] Patch Attachment 175966 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14992601
Created attachment 177282 [details] WIP
Comment on attachment 177282 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=177282&action=review > Source/WTF/wtf/MemoryInstrumentation.h:45 > +class String; unnecessary line > Source/WTF/wtf/MemoryInstrumentation.h:169 > + template<typename T> void addObjectImpl(const T* const&, MemoryObjectInfo*, MemoryOwningType, const char* name); I think it should be called edgeName.
Comment on attachment 177282 [details] WIP Attachment 177282 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15120419
Comment on attachment 177282 [details] WIP Attachment 177282 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15132023
Comment on attachment 177282 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=177282&action=review > Source/WTF/wtf/MemoryInstrumentation.h:63 > + virtual void reportHeapObject(const MemoryObjectInfo&) { } I prefer reportNode method name. > Source/WTF/wtf/MemoryInstrumentation.h:64 > + virtual void reportEdge(const void* source, const void* target, const char* name) { } looks like we need a method for leafs: reportNodeLeaf which will be used for reporting private buffers with no pointer. it is required because we have no pointer and have to report the size together with edge.
Created attachment 177371 [details] WIP
(In reply to comment #9) > (From update of attachment 177282 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177282&action=review > > > Source/WTF/wtf/MemoryInstrumentation.h:63 > > + virtual void reportHeapObject(const MemoryObjectInfo&) { } > > I prefer reportNode method name. > Done. > > Source/WTF/wtf/MemoryInstrumentation.h:64 > > + virtual void reportEdge(const void* source, const void* target, const char* name) { } > > looks like we need a method for leafs: reportNodeLeaf > which will be used for reporting private buffers with no pointer. > it is required because we have no pointer and have to report the size together with edge. Added reportLeaf for that.
Comment on attachment 177371 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=177371&action=review > Source/WTF/wtf/MemoryInstrumentation.cpp:115 > + m_memoryInstrumentation->addRawBuffer(m_memoryObjectInfo->reportedPointer(), buffer, m_objectType, size); I think we need a node name argument here with default value 0 > Source/WTF/wtf/MemoryInstrumentation.cpp:125 > + m_memoryInstrumentation->reportLinkToBuffer(m_memoryObjectInfo->reportedPointer(), 0, ownerObjectType, size); ditto > Source/WTF/wtf/MemoryInstrumentation.h:45 > +class String; unnecessary forward declaration > Source/WTF/wtf/MemoryInstrumentation.h:146 > + template<typename T> void addObject(const T& t, MemoryObjectInfo* ownerObjectInfo, const char* name) { OwningTraits<T>::addObject(this, t, ownerObjectInfo, name); } name -> edgeName
Created attachment 177380 [details] Patch
(In reply to comment #12) > (From update of attachment 177371 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177371&action=review > > > Source/WTF/wtf/MemoryInstrumentation.cpp:115 > > + m_memoryInstrumentation->addRawBuffer(m_memoryObjectInfo->reportedPointer(), buffer, m_objectType, size); > > I think we need a node name argument here with default value 0 > Done. > > Source/WTF/wtf/MemoryInstrumentation.cpp:125 > > + m_memoryInstrumentation->reportLinkToBuffer(m_memoryObjectInfo->reportedPointer(), 0, ownerObjectType, size); > > ditto > Done. > > Source/WTF/wtf/MemoryInstrumentation.h:45 > > +class String; > > unnecessary forward declaration > Done. > > Source/WTF/wtf/MemoryInstrumentation.h:146 > > + template<typename T> void addObject(const T& t, MemoryObjectInfo* ownerObjectInfo, const char* name) { OwningTraits<T>::addObject(this, t, ownerObjectInfo, name); } > > name -> edgeName Done.
Comment on attachment 177380 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177380&action=review > Source/WTF/wtf/MemoryInstrumentation.cpp:71 > + memoryObjectInfo.reportObjectInfo(buffer, ownerObjectType, size); What about assigning nodeName?
Comment on attachment 177380 [details] Patch Attachment 177380 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15135001
Comment on attachment 177380 [details] Patch otherwise looks good
Created attachment 177388 [details] Patch
Comment on attachment 177388 [details] Patch Attachment 177388 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15098851
Committed r136563: <http://trac.webkit.org/changeset/136563>