Bug 30273 – REGRESSION: Dromaeo DOM test is 14% slower

• Component: DOM
• Status: RESOLVED FIXED

[David Smith]

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.

[Maciej Stachowiak]

The cited link shows a 14% regression.

[David Kilzer (:ddkilzer)]

<rdar://problem/7292968>

[Darin Adler]

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]

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]

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]

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]

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]

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]

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]

(In reply to comment #9)
> a big regression between 48330 and 48334. 

That points to <http://trac.webkit.org/changeset/48331> then.

[Brian Weinstein]

(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]

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]

Brian: So if that’s 8% of the regression, we still have a significant regression to track down. Over 5%, right?

[Brian Weinstein]

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]

Tracked dow the rest of this.  There was a 4.5% regression from <http://trac.webkit.org/changeset/49409>

[Oliver Hunt]

Created attachment 41477 [details]
Make the DOM bindings use StructureFlags

[Oliver Hunt]

Am now adding logging to see what cases we're hitting where we believe things aren't cachable

[Oliver Hunt]

Comment on attachment 41477 [details]
Make the DOM bindings use StructureFlags

Landed in r49835

[Oliver Hunt]

Accidentally removed important part of that patch, with it actually present the regression caused by r48331 goes away.

Fixed in r49841.

[Darin Adler]

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)]

(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.