RESOLVED FIXED 30273
REGRESSION: Dromaeo DOM test is 14% slower
https://bugs.webkit.org/show_bug.cgi?id=30273
Summary REGRESSION: Dromaeo DOM test is 14% slower
David Smith
Reported 2009-10-10 02:24:15 PDT
The URL is comparing the result of running dromaeo.com/?dom on stock Safari 4.0.3 and Safari 4.0.3 + r49412, with the nightly build coming in significantly behind. 2.4GHz/2GB MacBook running 10.6.1.
Attachments
Make the DOM bindings use StructureFlags (19.64 KB, patch)
2009-10-19 20:55 PDT, Oliver Hunt
no flags
Maciej Stachowiak
Comment 1 2009-10-10 02:29:28 PDT
The cited link shows a 14% regression.
David Kilzer (:ddkilzer)
Comment 2 2009-10-10 08:27:21 PDT
Darin Adler
Comment 3 2009-10-12 07:58:04 PDT
Brian Weinstein tells me that some of the slowdown is from the recent changes to lower() although I don't know the details.
Jens Alfke
Comment 4 2009-10-12 09:41:15 PDT
I'd like to see any profiling data, preferably with backtraces, that implicates lower(). There's certainly potential for the optimization I made to backfire if there are huge numbers of non-no-op calls to that method; I just wasn't seeing many such calls when I was profiling.
Darin Adler
Comment 5 2009-10-12 10:00:14 PDT
Jens, our primary technique is actually narrowing down which revision caused the performance change. We typically don't start profiling until we have pinpointed the revision number.
Jens Alfke
Comment 6 2009-10-12 11:57:46 PDT
OK. Just now I ran the core DOM tests on r49402 and r49403 (just before and after my StringImpl::lower change), and found a 1.5% speedup. I also did some profiling and saw StringImpl::isLower showing up in some of the tests; the patch I'm working on right now (bug 30261) eliminates those tests, which will help slightly.
Darin Adler
Comment 7 2009-10-12 11:59:36 PDT
Brian, let us know when you've pinpointed which revisions caused the slowdown. Here's hoping Jens's changes are not to blame. I suspect they may even provide a speedup!
Brian Weinstein
Comment 8 2009-10-12 12:03:11 PDT
I had a 10.8% regression between 4.0.3 and revision 49414 on my machine. I was able to track down an 8-9% regression between revisions 48316 and 48351, and I will comment again when I have it down to the exact revision/revision + build fixes around it.
Brian Weinstein
Comment 9 2009-10-12 13:02:20 PDT
Here are my results after testing on Mac: 4.0.3: 670.73 runs/second r48325: 671.44 runs/second r48330: 663.40 runs/second r48334: 613.25 runs/second Tip of Tree (r49414): 598.18 runs/second It looks like there is a slight regression between r48325 and r48330, and a big regression between 48330 and 48334. However, the mac doesn't have any builds between these two revisions, so I will move over to Windows to see if I can reproduce the regressions there, and get an exact revision number.
Darin Adler
Comment 10 2009-10-12 13:06:46 PDT
(In reply to comment #9) > a big regression between 48330 and 48334. That points to <http://trac.webkit.org/changeset/48331> then.
Brian Weinstein
Comment 11 2009-10-12 13:10:02 PDT
(In reply to comment #10) > (In reply to comment #9) > > a big regression between 48330 and 48334. > > That points to <http://trac.webkit.org/changeset/48331> then. That was my guess, but I'm going to verify on Windows to double check.
Brian Weinstein
Comment 12 2009-10-12 13:34:26 PDT
I confirmed that this is also a regression on Windows, and that it is from http://trac.webkit.org/changeset/48331, assigning to Oliver.
Darin Adler
Comment 13 2009-10-12 13:46:00 PDT
Brian: So if that’s 8% of the regression, we still have a significant regression to track down. Over 5%, right?
Brian Weinstein
Comment 14 2009-10-12 13:48:10 PDT
I never saw 14%, so in my tests, there's about 2-3% of the regression that still needs to be spoken for, but I'll take a look at that later today.
Stephanie Lewis
Comment 15 2009-10-15 17:28:29 PDT
Tracked dow the rest of this. There was a 4.5% regression from <http://trac.webkit.org/changeset/49409>
Oliver Hunt
Comment 16 2009-10-19 20:55:35 PDT
Created attachment 41477 [details] Make the DOM bindings use StructureFlags
Oliver Hunt
Comment 17 2009-10-19 21:06:03 PDT
Am now adding logging to see what cases we're hitting where we believe things aren't cachable
Oliver Hunt
Comment 18 2009-10-19 21:06:43 PDT
Comment on attachment 41477 [details] Make the DOM bindings use StructureFlags Landed in r49835
Oliver Hunt
Comment 19 2009-10-19 23:58:27 PDT
Accidentally removed important part of that patch, with it actually present the regression caused by r48331 goes away. Fixed in r49841.
Darin Adler
Comment 20 2009-10-20 10:43:18 PDT
Why is this marked fixed if there was a 4.5% regression from <http://trac.webkit.org/changeset/49409>? Was that fixed?
Mark Rowe (bdash)
Comment 21 2009-10-20 11:17:27 PDT
(In reply to comment #20) > Why is this marked fixed if there was a 4.5% regression from > <http://trac.webkit.org/changeset/49409>? Was that fixed? That was almost certainly due to Geoff disabling the JIT in that revision (<http://trac.webkit.org/changeset/49409/trunk/JavaScriptCore/wtf/Platform.h>).It was reenabled shortly after.
Note You need to log in before you can comment on or make changes to this bug.