Bug 136118

Summary: Stop implicitly skipping a function's own activation when walking the scope chain
Product: WebKit Reporter: Oliver Hunt <oliver>
Component: New BugsAssignee: Oliver Hunt <oliver>
Status: RESOLVED FIXED    
Severity: Normal CC: mark.lam, ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 136123    
Bug Blocks:    
Attachments:
Description Flags
Patch ggaren: review+

Oliver Hunt
Reported 2014-08-20 13:39:43 PDT
Stop implicitly skipping a function's own activation when walking the scope chain
Attachments
Patch (18.36 KB, patch)
2014-08-20 13:42 PDT, Oliver Hunt 		ggaren: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Oliver Hunt
Comment 1 2014-08-20 13:42:21 PDT
Created attachment 236894 [details] Patch
Geoffrey Garen
Comment 2 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.
Oliver Hunt
Comment 3 2014-08-20 13:54:05 PDT
Committed r172808: <http://trac.webkit.org/changeset/172808>
Csaba Osztrogonác
Comment 4 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)
Csaba Osztrogonác
Comment 5 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.
Csaba Osztrogonác
Comment 6 2014-08-21 01:57:51 PDT
new bug report to track this regression: https://bugs.webkit.org/show_bug.cgi?id=136123
Mark Lam
Comment 7 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>.
Note You need to log in before you can comment on or make changes to this bug.