Bug 14719

Summary: 1-5% regression in JavaScript performance
Product: WebKit Reporter: Maciej Stachowiak <mjs>
Component: JavaScriptCoreAssignee: Maciej Stachowiak <mjs>
Status: RESOLVED FIXED    
Severity: Normal CC: zwarich
Priority: P2 Keywords: InRadar
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
patch that actually makes it faster than pre-regression
darin: review+
oops, caused some test regressions; fix oliver: review+

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