Bug 99383 – [v8] 15+% of Dromaeo/dom-query is spent creating AtomicStrings

• Status: RESOLVED
• Resolution: INVALID
• Component: New Bugs

[Eric Seidel (no email)]

[v8] 15+% of Dromaeo/dom-query is spent creating AtomicStrings for "div", "*" constants passed from V8

bug 99118 is the JSC equivalent of this bug.

I don't know how v8 stores it's constant strings, but it may be possible for us to do this AtomicString lookup for these constants once and cache it, instead of repeatedly creating a new AtomicString for "*", "div", etc. every time it's passed to getElementsByTagName, etc. in a loop:

Example sub-test:

        test( "getElementsByTagName(div)", function(){
                for ( var i = 0; i < num; i++ ) {
                        var elems = document.getElementsByTagName("div");
                        ret = elems[elems.length-1].nodeType;
                }
        });

15+% of time will be spent creating the AtomicString for "div", as we have to create a new one each time we call getElementsByTagNameCallback in V8DocumentInternals.cpp

[Eric Seidel (no email)]

The data I gathered for the JSC bug:
https://bugs.webkit.org/show_bug.cgi?id=99118#c15
suggests that this could have a real effect on web perf, as we have do some 24k of these needless hashings when loading a small docs.google.com spreadsheet.  The numbers I generated there were from a JSC build, but I expect them to be very similar for v8, since the profiles of v8/jsc are somewhat similar for this test.

[Eric Seidel (no email)]

Created attachment 168805 [details]
the pprof output, showing how much time we spend due to these needless AtomicString creations

[Eric Seidel (no email)]

It looks like the the WebCoreStringResource cache should already be catching this case.  I'm curious as to why it's not... investigating.

[Eric Seidel (no email)]

This does not appear to be the same trouble that JSC is having.  v8 already does the caching I proposed in bug 99118 for JSC.

All the AtomicString creation for V8 seems to be coming from this test:

        test( "getElementById", function(){
                for ( var i = 0; i < num * 30; i++ ) {
                        ret = document.getElementById("testA" + num).nodeType;
                        ret = document.getElementById("testB" + num).nodeType;
                        ret = document.getElementById("testC" + num).nodeType;
                        ret = document.getElementById("testD" + num).nodeType;
                        ret = document.getElementById("testE" + num).nodeType;
                        ret = document.getElementById("testF" + num).nodeType;
                }
        });

I'm not entirely clear why the test* strings are not externalized, and thus kept in WebCoreStringResource's m_atomicString members beyond each loop iteration, but they don't seem to be.

[Eric Seidel (no email)]

I added some printfs, and found that these strings return false to CanMakeExternal from v8, which is probably correct.

I'm starting to think this isn't really a bug.  At least not on my Mac.  Perhaps I was somehow hitting something different on my Linux box (where I generated the attache pprof.svg from).

[Brian Burg]

Closing some V8-related work items.