[webkit-reviews] review denied: [Bug 91734] Web Inspector: native memory instrumentation: cover MemoryCache with MemoryInstrumentation : [Attachment 153245] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 91734: Web Inspector: native memory instrumentation: cover MemoryCache with
MemoryInstrumentation
https://bugs.webkit.org/show_bug.cgi?id=91734

Attachment 153245: Patch
https://bugs.webkit.org/attachment.cgi?id=153245&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=153245&action=review


> Source/WebCore/ChangeLog:8
> +	   Old version of native memory instrumentation doesn't remember the
objects in MemoryCache.

mark the memory cache objects as visited.

> Source/WebCore/ChangeLog:10
> +	   As the result MI could count the same objects two times.

As a result ... multiple time

> Source/WebCore/ChangeLog:11
> +	   The first time via MemoryCache's size and the second time via DOM
traversing.

nuke

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:482
> +    PassRefPtr<InspectorMemoryBlock> dumpCacheStatistics() const

buildObjectForMemoryCache

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:489
> +	   addMemoryBlockFor(children.get(),
m_totalSizes[MemoryInstrumentation::MemoryCacheStructures],
MemoryBlockName::memoryCacheStructures);

It sounds like it could be implemented more efficiently.

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:504
> +    PassRefPtr<InspectorMemoryBlock> dumpDOMStatistics() const

buildObjectForDOM

> Source/WebCore/loader/cache/CachedResource.cpp:798
> +    info.addInstrumentedHashSet(m_handlesToRevalidate);

How do we make sure that whoever adds a member counts it here?