604858 - Tracer hang

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Alon Zakai (:azakai)]

Attachment: script that hangs with tracer enabled

Running the attached script in tracemonkey hangs (stays at 100% CPU forever) with |-j| or |-j -m|. With no parameters it completes properly, with |-m| it completes properly (faster).

Tested on latest tracemonkey, optimized build, on Linux 32.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Is this a regression?  If so, from when?

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: TMFLAGS=minimal output

Attached is the output if you run with TMFLAGS=minimal (I killed it after a while).

It looks like it's in an infinite loop trying to record stuff for the loops at lines 209, 224, 233, 236, 261, 267.  Each of these loops is a "for(;;) { ... }" loop.

Someone who knows more about tree recording might be able to diagnose the problem with this info.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Hmm.  There are multiple loops with the same name in there, right?  Could that be confusing things?

Comment 4

[Luke Wagner [:luke]]

Fun fact:
time js -m  0.8s
time jsc    2.6s
time d8     1.6s

Comment 5

[Alon Zakai (:azakai)]

(In reply to comment #4)
> Fun fact:
> time js -m  0.8s
> time jsc    2.6s
> time d8     1.6s

I believe that is in part due to that code utilizing typed arrays, when they are available. They are in spidermonkey, but I don't know a way to enable them in the jsc or v8 shells.

Comment 6

[Boris Zbarsky [:bzbarsky]]

JM has no good typed arrays support yet, though.

Comment 7

[Alon Zakai (:azakai)]

(In reply to comment #6)
> JM has no good typed arrays support yet, though.

Well, for this benchmark here, I get

tm -j with typed arrays:    0.69s
v8 (no typed arrays):       1.21s
tm -j without typed arrays: 1.35s

so typed arrays already win the day. Can't wait for them to be even faster :)

(To disable them in the benchmark, replace the tests for |this.Int32Array| and |this.Float64Array| with false).

Comment 8

[Luke Wagner [:luke]]

So its not an infinite loop, rather, loops keep getting un-blacklisted, which means the code is effectively running 100x slower than the interpreter.  I haven't found the cause of the un-blacklist bug, but I uncovered a (non-correctness) bug: for labelled breaks, we don't endLoop(), rather, we keep recording out of the loop.  (I suspect this "record out of the loop" situation is the underlying cause of the blacklisting bug too.)  Adding "|| SN_TYPE(sn) == SRC_BREAK2LABEL" to record_JSOP_GOTO causes the benchmark to complete (in 1.25s).

Note: It's still not staying on trace due to aborts when accessing FHEAP:
  trace stopped: 9395: hitting the global object via a prototype chain
If we could figure out why this is hitting, I bet the whole thing would run a bunch faster.  Asking on IRC...

Comment 9

[Luke Wagner [:luke]]

(In reply to comment #8)
Filed bug 611107 for the abort.  Looks like this can be fixed.

Comment 10

[Luke Wagner [:luke]]

So it looks like the fix in comment 8 is the real fix: if recording can continue out of the loop, it hits the back edge of an outer loop and, assuming its an inner loop, ignores blacklisting and starts recording the (outer) loop.  Hence, not endLoop()'ing for labelled break was breaking an implicit acyclicy invariant.

Comment 11

[Luke Wagner [:luke]]

Attachment: fix and asserts

endLoop on labelled-break.  I also added an assert that, at each opcode when we are recording, we are within the loop we think we are.  Passes jit/ref tests.  Bring it on fuzzers...

Comment 12

[Brendan Eich [:brendan]]

Comment on attachment 489682 [details] [diff] [review]
fix and asserts

For a Texan, your Anglicized misspelling of labeled is unnerving to me. Can you redownload regional/national conventions and override the MI6 virus? ;-)

-    if (sn && (SN_TYPE(sn) == SRC_BREAK || SN_TYPE(sn) == SRC_CONT2LABEL)) {
+    if (sn && (SN_TYPE(sn) == SRC_BREAK || SN_TYPE(sn) == SRC_CONT2LABEL ||
+               SN_TYPE(sn) == SRC_BREAK2LABEL)) {

Ultra-nit: break after && and avoid right-heavy appearance (fit on two lines total, avoiding a break after || if you can).

/be

Comment 13

[Boris Zbarsky [:bzbarsky]]

> tm -j

That's TM, not JM.  TM does typed arrays fast, yes.  ;)

Comment 14

[Luke Wagner [:luke]]

Drat, try server shows the assertion hitting sporadically on win32 mochi5.  I'll try to repro locally.

Comment 15

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/eaa6114ee405

Comment 16

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/eaa6114ee405