Bug 78825 – Web Inspector: use static counters for estimation of allocated Documents, Nodes and JS EventListeners

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

[Yury Semikhatsky]

This counters cost nothing but will be very useful in the Timeline panel. Values of the counters could be sent along with each timeline event unlike DOM counters collected by traversing DOM on each event since their calculation is quite expensive.

[Yury Semikhatsky]

Created attachment 127392 [details]
Patch

[Yury Semikhatsky]

Created attachment 127394 [details]
Patch

[Pavel Feldman]

Comment on attachment 127392 [details]
Patch

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

> Source/WebCore/bindings/v8/V8AbstractEventListener.cpp:38
> +#include "InspectorInstrumentation.h"

I would introduce InspectorCounters class that would have all the counter fields and guarantee a no-op upon the instrumentation call. WebCore/dom would update its values directly with no need for s_* in Node.h.

> Source/WebCore/inspector/InspectorInstrumentation.cpp:334
> +    ++InspectorMemoryAgent::counter(InspectorMemoryAgent::JSEventListenerCounter);

You could merge memory counters into the new InspectorCounter class.

[Ilya Tikhonovsky]

Comment on attachment 127394 [details]
Patch

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

> Source/WebCore/inspector/InspectorInstrumentation.cpp:334
> +    ++InspectorMemoryAgent::counter(InspectorMemoryAgent::JSEventListenerCounter);

it looks strange. I'd prefer to implement the counter as a class with int& operator[](int index) { .... }

[Yury Semikhatsky]

Created attachment 127532 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #3)
> (From update of attachment 127392 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127392&action=review
> 
> > Source/WebCore/bindings/v8/V8AbstractEventListener.cpp:38
> > +#include "InspectorInstrumentation.h"
> 
> I would introduce InspectorCounters class that would have all the counter fields and guarantee a no-op upon the instrumentation call. WebCore/dom would update its values directly with no need for s_* in Node.h.
> 
Done.

> > Source/WebCore/inspector/InspectorInstrumentation.cpp:334
> > +    ++InspectorMemoryAgent::counter(InspectorMemoryAgent::JSEventListenerCounter);
> 
> You could merge memory counters into the new InspectorCounter class.
Done.

(In reply to comment #4)
> (From update of attachment 127394 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127394&action=review
> 
> > Source/WebCore/inspector/InspectorInstrumentation.cpp:334
> > +    ++InspectorMemoryAgent::counter(InspectorMemoryAgent::JSEventListenerCounter);
> 
> it looks strange. I'd prefer to implement the counter as a class with int& operator[](int index) { .... }

There is no method which would return a reference to the counter anymore. Instead we have more restrictive inc/dec methods and value getter.

[Yury Semikhatsky]

Committed r108047: <http://trac.webkit.org/changeset/108047>