Remove slow calls to js_ReconstructStackDepth

RESOLVED FIXED in Firefox 10

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: dmandelin, Assigned: luke)

Tracking

Trunk
mozilla10
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(firefox7 affected, firefox8+ affected, firefox9+ affected, firefox10+ fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Luke, you know what to do. :-) Let's clear this out.
(Assignee)

Updated

6 years ago
Summary: Remove slow calls to js_GetSrcNoteCachedJS → Remove slow calls to js_ReconstructStackDepth
(Assignee)

Comment 1

6 years ago
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!
Attachment #566578 - Flags: review?(dmandelin)
(Reporter)

Updated

6 years ago
Attachment #566578 - Flags: review?(dmandelin) → review+
(Assignee)

Comment 2

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9572dd78af53
Target Milestone: --- → mozilla10
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
(Assignee)

Comment 4

6 years ago
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
https://hg.mozilla.org/mozilla-central/rev/ccd092244e4c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
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.
Whiteboard: [qa-]

Updated

5 years ago
status-firefox10: affected → ---

Updated

5 years ago
status-firefox10: --- → fixed
You need to log in before you can comment on or make changes to this bug.