1030446 - OdinMonkey: simplify stack-walking

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Luke Wagner [:luke]]

These patches simplify the existing asm.js stack-walking, preserve existing behavior/performance, and prepare for async stack-walking in bug 1027885.

Comment 1

[Luke Wagner [:luke]]

Attachment: move-iterator

This patch simply moves the AsmJSFrameIterator code to its own file.  This will be useful later when there is more code.

Comment 2

[Luke Wagner [:luke]]

Attachment: tweak-tests

This patch just changes tests to use setJitCompilerOption which allows lower iteration counts and is also useful for later testing.

Comment 3

[Luke Wagner [:luke]]

Attachment: use-exit-fp

This patch switches exit paths from saving the exit sp to saving the exit fp.  I really should've done it this way to begin with, it's much simpler.  It's also necessary for async stack-walking: if the profiler samples a thread while it is in a signal handler called from asm.js code (out-of-bounds, interrupt handler), we need the pointer in the stack to be coherent at every instruction.  With exitSP, there are several instructions before and after the call where no return address is pushed.  With exitFP, we can ensure that, whenever exitFP is non-null, it points to a valid frame.  This also avoids the InterruptedSP hack.

Comment 4

[Luke Wagner [:luke]]

Attachment: add-code-ranges

Instead of deriving the containing function from CallStack, we can instead store a separate sorted array of code ranges.  This has several advantages:
 - CallSite (which are much more numerous than CodeRange) shrinks a word
 - We can later store extra info for CodeRange that would otherwise bloat CallSite (e.g., the function start line is currently missing)
 - This will be useful with async profiling for handling entries/exites.

Comment 5

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d925c77d4b3f
https://hg.mozilla.org/integration/mozilla-inbound/rev/988f19a5ea1f
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e87a7cf7eac
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c761a455bbf

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/1d6a452e4369 for b2g non-unified build bustage: https://tbpl.mozilla.org/php/getParsedLog.php?id=42669108&tree=Mozilla-Inbound

Comment 7

[Luke Wagner [:luke]]

Note to self: need #include "mozilla/NullPtr.h" when using nullptr on old compilers that aren't run on try by default...

https://hg.mozilla.org/integration/mozilla-inbound/rev/4bc102bd4067
https://hg.mozilla.org/integration/mozilla-inbound/rev/c11c8733e76a
https://hg.mozilla.org/integration/mozilla-inbound/rev/addc1a7459a7
https://hg.mozilla.org/integration/mozilla-inbound/rev/457b38b8094c

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/457b38b8094c
https://hg.mozilla.org/mozilla-central/rev/addc1a7459a7
https://hg.mozilla.org/mozilla-central/rev/c11c8733e76a
https://hg.mozilla.org/mozilla-central/rev/4bc102bd4067

Comment 9

[Phil Ringnalda (:philor)]

Speculatively backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/5cc5f26b237e

This landed in https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=4bc102bd4067 and everything was fine for ten pushes.

Mats landed the should-be-unrelated https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=0b5918ec6521 and Android 2.2 Armv6 mochitest-4 started failing all the dom/asmjscache/ tests, failing "asm.js compilation is available".

I backed that out, and merged the cset before it around. The backout stopped the failure on mozilla-inbound, and the merge was fine on mozilla-central and b2g-inbound, but fx-team has the same Armv6 failure, despite not having Mats' patch.

Luke told me to just disable the tests on Armv6 and reland Mats, but I went to sleep instead, and this morning while I was discovering that we apparently don't have a condition to just disable on Armv6, and according to comments in webgl tests, we don't even have a way to detect it from runtime JS, Ms2ger landed the should-be-unrelated https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=48ad95c9ad89 and restarted the failure on mozilla-inbound.

My extremely handwavy theory is that this is being misoptimized into reading uninitialized memory that random other patches change into something that causes failures.

Comment 10

[Luke Wagner [:luke]]

The dom/asmjscache tests were failing b/c isAsmJSCompilationAvailable() is returning false.  This can be false for a number of valid reasons (no VFP, signal handlers broken, page size, etc) so it really wasn't valid for the tests to be requiring this to be true.  Re-pushing these patches with an additional patch that changes the tests to succeed even if isAsmJSCompilationAvailable() = false.  Independently, we can see if this shows up (we'll see "isAsmJSCompilationAvailable is false" in the test output) and investigate.

https://hg.mozilla.org/integration/mozilla-inbound/rev/32ad89bbe075
https://hg.mozilla.org/integration/mozilla-inbound/rev/7abc1d5a4e4c
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1235dfcbda0
https://hg.mozilla.org/integration/mozilla-inbound/rev/46136920eafe
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b57ff9dd413

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3b57ff9dd413
https://hg.mozilla.org/mozilla-central/rev/46136920eafe
https://hg.mozilla.org/mozilla-central/rev/d1235dfcbda0
https://hg.mozilla.org/mozilla-central/rev/7abc1d5a4e4c
https://hg.mozilla.org/mozilla-central/rev/32ad89bbe075