Bug 95739 – Web Inspector: NMI: replace ObjectType enum with static const char* string identifiers.

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

[Ilya Tikhonovsky]

When we go deeper into different parts of browser like skia, chromium itself etc.
we can't use a single enum for all reported object types.
We need a code that allows us to register object types on the fly.
On the next step we will make string identifiers hierarchical like DOM, DOM.Bindings, DOM.Other etc.

[Ilya Tikhonovsky]

Created attachment 162024 [details]
Patch

[Build Bot]

Comment on attachment 162024 [details]
Patch

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

[Yury Semikhatsky]

Comment on attachment 162024 [details]
Patch

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

> Source/WebCore/inspector/MemoryInstrumentationImpl.cpp:40
> +Vector<String> WebCoreMemoryTypesImpl::registerObjectTypes(unsigned firstFreeId)

I'd rather use strings instead of numbers for identifying object type. We could switch to the scheme with the type registration later if there is a noticeable performance impact. WDYT?

[Early Warning System Bot]

Comment on attachment 162024 [details]
Patch

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

[Early Warning System Bot]

Comment on attachment 162024 [details]
Patch

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

[Peter Beverloo (cr-android ews)]

Comment on attachment 162024 [details]
Patch

Attachment 162024 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13745476

[Ilya Tikhonovsky]

(In reply to comment #3)
> (From update of attachment 162024 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162024&action=review
> 
> > Source/WebCore/inspector/MemoryInstrumentationImpl.cpp:40
> > +Vector<String> WebCoreMemoryTypesImpl::registerObjectTypes(unsigned firstFreeId)
> 
> I'd rather use strings instead of numbers for identifying object type. We could switch to the scheme with the type registration later if there is a noticeable performance impact. WDYT?

Looks like the version which uses char* as ObjectType is 30% slower than current implementation. 990ms vs 750ms on 10m insertions with disabled visited check.
I think we could use this approach as the solution because we will have simpler business logic.

[Ilya Tikhonovsky]

Created attachment 162046 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 162046 [details]
Patch

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

> Source/WebCore/dom/MemoryInstrumentation.h:53
> +    static const char* Other;

const char* -> ObjectType

[Ilya Tikhonovsky]

Committed r127571: <http://trac.webkit.org/changeset/127571>

[James Robinson]

The "speedTest" unit test takes over 20 seconds to run on my extremely fast desktop.  That's waaaaaaaaaaaaaaaaay too long for a unit test.