Bug 99383 - [v8] 15+% of Dromaeo/dom-query is spent creating AtomicStrings
Summary: [v8] 15+% of Dromaeo/dom-query is spent creating AtomicStrings
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 99191
  Show dependency treegraph
 
Reported: 2012-10-15 16:18 PDT by Eric Seidel (no email)
Modified: 2014-12-16 00:47 PST (History)
3 users (show)

See Also:


Attachments
the pprof output, showing how much time we spend due to these needless AtomicString creations (103.68 KB, image/svg+xml)
2012-10-15 16:25 PDT, Eric Seidel (no email)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2012-10-15 16:18:28 PDT
[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
Comment 1 Eric Seidel (no email) 2012-10-15 16:21:37 PDT
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.
Comment 2 Eric Seidel (no email) 2012-10-15 16:25:41 PDT
Created attachment 168805 [details]
the pprof output, showing how much time we spend due to these needless AtomicString creations
Comment 3 Eric Seidel (no email) 2012-10-17 14:45:05 PDT
It looks like the the WebCoreStringResource cache should already be catching this case.  I'm curious as to why it's not... investigating.
Comment 4 Eric Seidel (no email) 2012-10-17 17:27:42 PDT
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.
Comment 5 Eric Seidel (no email) 2012-10-17 17:46:24 PDT
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).
Comment 6 Brian Burg 2014-12-16 00:47:54 PST
Closing some V8-related work items.