Bug 109687 – Web Inspector: add experimental native heap graph to Timeline panel

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

[Yury Semikhatsky]

Statistics on the native memory usage will be collected after each top level task. The data will be represented as a graph instead of DOM counters for now.

[Yury Semikhatsky]

Created attachment 188068 [details]
Patch

[Yury Semikhatsky]

Created attachment 188069 [details]
Screenshot with the patch applied

[Ilya Tikhonovsky]

Comment on attachment 188068 [details]
Patch

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

> Source/WebCore/inspector/InspectorTimelineAgent.cpp:548
> +    m_memoryAgent->getProcessMemoryDistributionMap(&map);

we have null m_memoryAgent here in case of Worker.

> Source/WebCore/inspector/InspectorTimelineAgent.cpp:552
> +//            printf("%s -> %ld\n", it->key.ascii().data(), it->value);

please remove this line

[Yury Semikhatsky]

Created attachment 188077 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #3)
> (From update of attachment 188068 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=188068&action=review
> 
> > Source/WebCore/inspector/InspectorTimelineAgent.cpp:548
> > +    m_memoryAgent->getProcessMemoryDistributionMap(&map);
> 
> we have null m_memoryAgent here in case of Worker.
> 
Done.

> > Source/WebCore/inspector/InspectorTimelineAgent.cpp:552
> > +//            printf("%s -> %ld\n", it->key.ascii().data(), it->value);
> 
> please remove this line
Done.

[Alexander Pavlov (apavlov)]

Comment on attachment 188077 [details]
Patch

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

> Source/WebCore/inspector/Inspector.json:2634
> +                    { "name": "enabled", "type": "boolean", "description": "True to start collecting statisics on native memory usage." }

Please rephrase to include the disablement (false) case

> Source/WebCore/inspector/Inspector.json:2626
> +                    { "name": "enabled", "type": "boolean", "description": "True to start reporting DOM counters." }

Please rephrase to include the stop (false) case

> Source/WebCore/inspector/Inspector.json:2634
> +                    { "name": "enabled", "type": "boolean", "description": "True to start collecting statisics on native memory usage." }

Please rephrase to include the stop (false) case

> Source/WebCore/inspector/Inspector.json:2637
> +                "description": "Starts sending statistics on native memory usage along with timeline events."

"Starts sending" -> "Whether to send"?

[Yury Semikhatsky]

(In reply to comment #6)
> (From update of attachment 188077 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=188077&action=review
> 
> > Source/WebCore/inspector/Inspector.json:2634
> > +                    { "name": "enabled", "type": "boolean", "description": "True to start collecting statisics on native memory usage." }
> 
> Please rephrase to include the disablement (false) case
> 
Done.

> > Source/WebCore/inspector/Inspector.json:2626
> > +                    { "name": "enabled", "type": "boolean", "description": "True to start reporting DOM counters." }
> 
> Please rephrase to include the stop (false) case
> 
Done.

> > Source/WebCore/inspector/Inspector.json:2634
> > +                    { "name": "enabled", "type": "boolean", "description": "True to start collecting statisics on native memory usage." }
> 
> Please rephrase to include the stop (false) case
> 
Done.

> > Source/WebCore/inspector/Inspector.json:2637
> > +                "description": "Starts sending statistics on native memory usage along with timeline events."
> 
> "Starts sending" -> "Whether to send"?
Done.

[Yury Semikhatsky]

Committed r142746: <http://trac.webkit.org/changeset/142746>

[Joseph Pecoraro]

Comment on attachment 188077 [details]
Patch

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

> Source/WebCore/inspector/Inspector.json:2624
> +                "name": "setIncludeDomCounters",

The Inspector Protocol and WebKit project consistently use "DOM" capitalization. The "Dom" here looks out of place:

    getDOMCounters, addDOMStorage, setDOMBreakpoint, removeDOMBreakpoint, ...

Quick follow-up?