Last Comment Bug 693895 - Remove slow calls to js_ReconstructStackDepth
: Remove slow calls to js_ReconstructStackDepth
Status: RESOLVED FIXED
[qa-]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla10
Assigned To: Luke Wagner [:luke]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-11 19:19 PDT by David Mandelin [:dmandelin]
Modified: 2012-01-23 14:24 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
+
affected
+
affected
+
fixed


Attachments
rm JSOP_FUNAPPLY slow path in StackIter (6.61 KB, patch)
2011-10-12 10:16 PDT, Luke Wagner [:luke]
dmandelin: review+
Details | Diff | Splinter Review

Description David Mandelin [:dmandelin] 2011-10-11 19:19:26 PDT
Luke, you know what to do. :-) Let's clear this out.
Comment 1 Luke Wagner [:luke] 2011-10-12 10:16:59 PDT
Created attachment 566578 [details] [diff] [review]
rm JSOP_FUNAPPLY slow path in StackIter

For posterity: StackIter added this pathological slow case so as not to lose native calls from the debugger's perspective.  The assumption was that this slowness wouldn't matter since it would take quite a coincidence to be observable (giant script + many exceptions + apply() calls in the giant script).  This turned out not to be the case and JSBench seems to generate code that incurs a 10x slowdown.

The simple fix is just to ignore JSOP_FUNAPPLY and lose calls.  Currently, this information isn't even exposed by the debugger (jsdbg2 work is not complete) so there is zero loss of functionality.  Also good news for the future: dvander says IonMonkey will have mini activation records for native calls so that none of this pc/sp snooping hackery will be necessary at all!
Comment 3 Marco Bonardo [::mak] 2011-10-12 15:22:09 PDT
backed out for make check failures:

TEST-UNEXPECTED-FAIL | jit_test.py -a -m          | /builds/slave/m-in-lnx-dbg/build/js/src/jit-test/tests/basic/testCompileScript.js: Assertion failure: sp_ >= fp_->base() && sp_ <= fp_->slots() + fp_->script()->nslots, at ../../../js/src/vm/Stack.cpp:1063
TEST-UNEXPECTED-FAIL | jit_test.py -a -m -j       | /builds/slave/m-in-lnx-dbg/build/js/src/jit-test/tests/basic/testCompileScript.js: Assertion failure: sp_ >= fp_->base() && sp_ <= fp_->slots() + fp_->script()->nslots, at ../../../js/src/vm/Stack.cpp:1063
TEST-UNEXPECTED-FAIL | jit_test.py -a -m -j -p    | /builds/slave/m-in-lnx-dbg/build/js/src/jit-test/tests/basic/testCompileScript.js: Assertion failure: sp_ >= fp_->base() && sp_ <= fp_->slots() + fp_->script()->nslots, at ../../../js/src/vm/Stack.cpp:1063
TEST-UNEXPECTED-FAIL | jit_test.py -a -a -m       | /builds/slave/m-in-lnx-dbg/build/js/src/jit-test/tests/basic/testCompileScript.js: Assertion failure: sp_ >= fp_->base() && sp_ <= fp_->slots() + fp_->script()->nslots, at ../../../js/src/vm/Stack.cpp:1063
TEST-UNEXPECTED-FAIL | jit_test.py -a -a -m -j    | /builds/slave/m-in-lnx-dbg/build/js/src/jit-test/tests/basic/testCompileScript.js: Assertion failure: sp_ >= fp_->base() && sp_ <= fp_->slots() + fp_->script()->nslots, at ../../../js/src/vm/Stack.cpp:1063
TEST-UNEXPECTED-FAIL | jit_test.py -a -a -m -j -p | /builds/slave/m-in-lnx-dbg/build/js/src/jit-test/tests/basic/testCompileScript.js: Assertion failure: sp_ >= fp_->base() && sp_ <= fp_->slots() + fp_->script()->nslots, at ../../../js/src/vm/Stack.cpp:1063
TEST-UNEXPECTED-FAIL | jit_test.py -j             | /builds/slave/m-in-lnx-dbg/build/js/src/jit-test/tests/basic/testStackIter.js: uncaught exception: Length 3 not equal 2
TEST-UNEXPECTED-FAIL | jit_test.py -a -m -j       | /builds/slave/m-in-lnx-dbg/build/js/src/jit-test/tests/basic/testStackIter.js: uncaught exception: Length 3 not equal 2
make[1]: *** [check-jit-test] Error 2
make: *** [check] Error 2
Comment 4 Luke Wagner [:luke] 2011-10-12 17:45:22 PDT
Oops, that assertion was valid when JSOP_FUNAPPLY was being handled separately.  Relanded with JSOP_FUNAPPLY excused.  Retested with a bunch of --jitflags this time :)

https://hg.mozilla.org/integration/mozilla-inbound/rev/ccd092244e4c
Comment 5 Marco Bonardo [::mak] 2011-10-13 07:33:36 PDT
https://hg.mozilla.org/mozilla-central/rev/ccd092244e4c
Comment 6 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-10-13 11:29:08 PDT
qa- as there does not appear to be much QA can do to verify this fix. Please set to qa+ if this is not the case.

Note You need to log in before you can comment on or make changes to this bug.