Bug 136118 - Stop implicitly skipping a function's own activation when walking the scope chain
Summary: Stop implicitly skipping a function's own activation when walking the scope c...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on: 136123
Blocks:
  Show dependency tree / graph
 
Reported: 2014-08-20 13:39 PDT by Oliver Hunt
Modified: 2014-08-21 16:12 PDT (History)
2 users (show)

See Also:

Attachments
Patch (18.36 KB, patch)
2014-08-20 13:42 PDT, Oliver Hunt 		ggaren: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2014-08-20 13:39:43 PDT 
Stop implicitly skipping a function's own activation when walking the scope chain
Comment 1 Oliver Hunt 2014-08-20 13:42:21 PDT 
Created attachment 236894 [details]
Patch
Comment 2 Geoffrey Garen 2014-08-20 13:46:03 PDT 
Comment on attachment 236894 [details]
Patch

r=me

If the extra pointer dereference is a performance hit, perhaps we need a DFG/FTL optimization to infer that your parent scopes are constants. It seems like, at each level up the scope chain, the odds of a constant scope must increase exponentially.
Comment 3 Oliver Hunt 2014-08-20 13:54:05 PDT 
Committed r172808: <http://trac.webkit.org/changeset/172808>
Comment 4 Csaba Osztrogonác 2014-08-21 00:46:17 PDT 
(In reply to comment #3)
> Committed r172808: <http://trac.webkit.org/changeset/172808>

It broke 6 different tests on Apple Mac 32 bit:

http://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/3692/steps/webkit-32bit-jsc-test/logs/stdio

jsc-layout-tests.yaml/js/script-tests/array-filter.js (7 failures)
jsc-layout-tests.yaml/js/script-tests/dfg-cse-cfa-discrepancy.js (7 failures)
jsc-layout-tests.yaml/js/script-tests/reentrant-caching.js (7 failures)
mozilla-tests.yaml/js1_5/Scope/regress-208496-001.js (1 failure)
regress/script-tests/Float32Array-matrix-mult.js (8 failures)
v8-v6/v8-earley-boyer.js (11 failures)
Comment 5 Csaba Osztrogonác 2014-08-21 00:48:24 PDT 
(In reply to comment #4)
> (In reply to comment #3)
> > Committed r172808: <http://trac.webkit.org/changeset/172808>
> 
> It broke 6 different tests on Apple Mac 32 bit:
> 
> http://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/3692/steps/webkit-32bit-jsc-test/logs/stdio
> 
> jsc-layout-tests.yaml/js/script-tests/array-filter.js (7 failures)
> jsc-layout-tests.yaml/js/script-tests/dfg-cse-cfa-discrepancy.js (7 failures)
> jsc-layout-tests.yaml/js/script-tests/reentrant-caching.js (7 failures)
> mozilla-tests.yaml/js1_5/Scope/regress-208496-001.js (1 failure)
> regress/script-tests/Float32Array-matrix-mult.js (8 failures)
> v8-v6/v8-earley-boyer.js (11 failures)

It isn't related to x86, the same regression occurs on other 
32 bit platforms, ARM Traditional and ARM Thumb2.
Comment 6 Csaba Osztrogonác 2014-08-21 01:57:51 PDT 
new bug report to track this regression: https://bugs.webkit.org/show_bug.cgi?id=136123
Comment 7 Mark Lam 2014-08-21 16:12:48 PDT 
(In reply to comment #6)
> new bug report to track this regression: https://bugs.webkit.org/show_bug.cgi?id=136123

FYI, this has been fixed in r172838: <http://trac.webkit.org/r172838>.