Bug 14719 - 1-5% regression in JavaScript performance
Summary: 1-5% regression in JavaScript performance
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Maciej Stachowiak
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2007-07-22 22:32 PDT by Maciej Stachowiak
Modified: 2007-07-23 01:49 PDT (History)
1 user (show)

See Also:


Attachments
patch that actually makes it faster than pre-regression (6.20 KB, patch)
2007-07-22 22:46 PDT, Maciej Stachowiak
darin: review+
Details | Formatted Diff | Diff
oops, caused some test regressions; fix (2.85 KB, patch)
2007-07-23 01:45 PDT, Maciej Stachowiak
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 2007-07-22 22:32:47 PDT
The following revision caused a measurable JS performance regression:

http://trac.webkit.org/projects/webkit/changeset/24287

This results in a 1% slowdown on JS iBench and a 5% slowdown on the "Celtic Kane" JS benchmark. The problem is that we now always walk the scope chain doing variable lookups, which results in extra hashtable traffic and a more complex function.
Comment 1 Darin Adler 2007-07-22 22:41:31 PDT
<rdar://problem/5353293>
Comment 2 Maciej Stachowiak 2007-07-22 22:46:17 PDT
Created attachment 15643 [details]
patch that actually makes it faster than pre-regression
Comment 3 Darin Adler 2007-07-22 23:20:53 PDT
Comment on attachment 15643 [details]
patch that actually makes it faster than pre-regression

r=me, with the comments from IRC
Comment 4 Cameron Zwarich (cpst) 2007-07-22 23:53:26 PDT
Removing the jsString() is in contradiction with section 12.2 of the ECMA spec, which says that variable statements return the name of the identifier as a string. Not that I think this is a sane choice or even useful in a single case, but it's why I copied it in from the other code.

I recently noticed that even with the jsString() there now it won't return the string value if you do something like:

function f() {
  var x = 1;
}

alert(f());

It's worth noting that neither Firefox nor Opera return the string.
Comment 5 Maciej Stachowiak 2007-07-23 01:45:34 PDT
Created attachment 15645 [details]
oops, caused some test regressions; fix