WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
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
<
rdar://problem/7292968
>
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.
Top of Page
Format For Printing
XML
Clone This Bug