Summary: | REGRESSION: Dromaeo DOM test is 14% slower | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Smith <catfish.man> | ||||
Component: | DOM | Assignee: | Oliver Hunt <oliver> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, bweinstein, darin, gavin.sharp, jens, levin, mjs, mrowe, oliver, slewis | ||||
Priority: | P2 | Keywords: | InRadar, Regression | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Mac (Intel) | ||||||
OS: | OS X 10.5 | ||||||
URL: | http://dromaeo.com/?id=75494,75493 | ||||||
Attachments: |
|
Description
David Smith
2009-10-10 02:24:15 PDT
The cited link shows a 14% regression. Brian Weinstein tells me that some of the slowdown is from the recent changes to lower() although I don't know the details. 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. 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. 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. 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! 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. 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. (In reply to comment #9) > a big regression between 48330 and 48334. That points to <http://trac.webkit.org/changeset/48331> then. (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. I confirmed that this is also a regression on Windows, and that it is from http://trac.webkit.org/changeset/48331, assigning to Oliver. Brian: So if that’s 8% of the regression, we still have a significant regression to track down. Over 5%, right? 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. Tracked dow the rest of this. There was a 4.5% regression from <http://trac.webkit.org/changeset/49409> Created attachment 41477 [details]
Make the DOM bindings use StructureFlags
Am now adding logging to see what cases we're hitting where we believe things aren't cachable Comment on attachment 41477 [details] Make the DOM bindings use StructureFlags Landed in r49835 Accidentally removed important part of that patch, with it actually present the regression caused by r48331 goes away. Fixed in r49841. Why is this marked fixed if there was a 4.5% regression from <http://trac.webkit.org/changeset/49409>? Was that fixed? (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. |