Bug 99383

Summary: [v8] 15+% of Dromaeo/dom-query is spent creating AtomicStrings
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: abarth, dcarney, haraken
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 99191    
Attachments:
Description Flags
the pprof output, showing how much time we spend due to these needless AtomicString creations none

Eric Seidel (no email)
Reported 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
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
Eric Seidel (no email)
Comment 1 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.
Eric Seidel (no email)
Comment 2 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
Eric Seidel (no email)
Comment 3 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.
Eric Seidel (no email)
Comment 4 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.
Eric Seidel (no email)
Comment 5 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).
Brian Burg
Comment 6 2014-12-16 00:47:54 PST
Closing some V8-related work items.
Note You need to log in before you can comment on or make changes to this bug.