556277 - (EagerThis) TM: compute |this| eagerly

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

Spun out from bug 555246.

Suppose we compute 'this' eagerly except when 'this' would be the global this. In that one case we set fp->argv[-1] and fp->thisv to JSVAL_NULL.

This would cost nothing on trace or on property cache hits. It would eliminate all the guards emitted by TR::getThis. And we can inline the most common case of JS_THIS:

static JS_ALWAYS_INLINE jsval
JS_THIS(JSContext *cx, jsval *vp)
{
    return JSVAL_IS_PRIMITIVE(vp[1]) ? JS_ComputeThis(cx, vp) : vp[1];
}

Comment 1

[Jason Orendorff [:jorendorff]]

This wins 3.5% on v8, 23% on v8-richards!

But it loses 1.6% on sunspider because of a single test, unpack-code. I'll profile that one today and see what's the matter.

TEST                   COMPARISON            FROM                 TO             DETAILS

=============================================================================

** TOTAL **:           *1.016x as slow*  798.9ms +/- 0.8%   811.4ms +/- 0.6%     significant

=============================================================================

  3d:                  -                 126.2ms +/- 3.4%   122.7ms +/- 3.2% 
    cube:              -                  44.8ms +/- 8.7%    44.1ms +/- 8.4% 
    morph:             1.030x as fast     30.6ms +/- 1.6%    29.7ms +/- 1.2%     significant
    raytrace:          1.039x as fast     50.8ms +/- 1.7%    48.9ms +/- 0.8%     significant

  access:              -                 128.9ms +/- 3.0%   125.0ms +/- 0.8% 
    binary-trees:      1.040x as fast     18.3ms +/- 1.9%    17.6ms +/- 2.1%     significant
    fannkuch:          -                  72.1ms +/- 0.6%    71.6ms +/- 0.7% 
    nbody:             -                  26.5ms +/- 14.7%    24.3ms +/- 2.0% 
    nsieve:            1.043x as fast     12.0ms +/- 2.8%    11.5ms +/- 3.3%     significant

  bitops:              -                  35.0ms +/- 1.9%    34.5ms +/- 1.5% 
    3bit-bits-in-byte: -                   1.4ms +/- 26.4%     1.2ms +/- 25.1% 
    bits-in-byte:      *1.033x as slow*    9.0ms +/- 0.0%     9.3ms +/- 3.7%     significant
    bitwise-and:       -                   2.1ms +/- 10.8%     2.1ms +/- 10.8% 
    nsieve-bits:       1.027x as fast     22.5ms +/- 2.2%    21.9ms +/- 1.0%     significant

  controlflow:         ??                  8.1ms +/- 2.8%     8.2ms +/- 3.7%     not conclusive: might be *1.012x as slow*
    recursive:         ??                  8.1ms +/- 2.8%     8.2ms +/- 3.7%     not conclusive: might be *1.012x as slow*

  crypto:              -                  50.1ms +/- 1.2%    49.5ms +/- 2.6% 
    aes:               ??                 29.3ms +/- 2.3%    29.5ms +/- 4.3%     not conclusive: might be *1.007x as slow*
    md5:               -                  13.2ms +/- 2.3%    13.0ms +/- 0.0% 
    sha1:              1.086x as fast      7.6ms +/- 4.9%     7.0ms +/- 0.0%     significant

  date:                1.015x as fast    112.1ms +/- 0.5%   110.4ms +/- 0.6%     significant
    format-tofte:      *1.010x as slow*   58.2ms +/- 0.8%    58.8ms +/- 0.8%     significant
    format-xparb:      1.045x as fast     53.9ms +/- 0.4%    51.6ms +/- 0.7%     significant

  math:                -                  39.9ms +/- 1.0%    39.5ms +/- 1.5% 
    cordic:            -                  11.9ms +/- 3.4%    11.8ms +/- 2.6% 
    partial-sums:      1.005x as fast     21.0ms +/- 0.0%    20.9ms +/- 1.1%     significant
    spectral-norm:     1.029x as fast      7.0ms +/- 0.0%     6.8ms +/- 4.4%     significant

  regexp:              -                  34.5ms +/- 1.5%    34.1ms +/- 1.8% 
    dna:               -                  34.5ms +/- 1.5%    34.1ms +/- 1.8% 

  string:              *1.089x as slow*  264.1ms +/- 1.6%   287.5ms +/- 0.5%     significant
    base64:            ??                 13.3ms +/- 2.6%    13.4ms +/- 2.8%     not conclusive: might be *1.008x as slow*
    fasta:             -                  60.9ms +/- 3.1%    60.0ms +/- 1.5% 
    tagcloud:          1.028x as fast     85.5ms +/- 1.6%    83.2ms +/- 0.9%     significant
    unpack-code:       *1.35x as slow*    79.3ms +/- 4.1%   107.0ms +/- 0.4%     significant
    validate-input:    1.050x as fast     25.1ms +/- 2.8%    23.9ms +/- 0.9%     significant


TEST              COMPARISON            FROM                 TO             DETAILS

=============================================================================

** TOTAL **:      1.035x as fast    8074.9ms +/- 0.4%   7802.9ms +/- 0.4%     significant

=============================================================================

  v8:             1.035x as fast    8074.9ms +/- 0.4%   7802.9ms +/- 0.4%     significant
    crypto:       1.017x as fast     777.2ms +/- 0.8%    764.1ms +/- 0.5%     significant
    deltablue:    1.090x as fast     700.8ms +/- 1.3%    642.8ms +/- 1.8%     significant
    earley-boyer: 1.023x as fast    2489.3ms +/- 0.7%   2434.3ms +/- 0.5%     significant
    raytrace:     1.036x as fast    1375.7ms +/- 1.8%   1327.8ms +/- 1.0%     significant
    regexp:       -                  999.3ms +/- 0.3%    997.6ms +/- 0.7% 
    richards:     1.23x as fast      565.4ms +/- 0.4%    458.5ms +/- 0.9%     significant
    splay:        ??                1167.2ms +/- 0.9%   1177.8ms +/- 1.1%     not conclusive: might be *1.009x as slow*

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: WIP 1

This is the patch I used to produce the numbers in comment 1. Very similar to the patch I posted in bug 555246.

Comment 3

[Jason Orendorff [:jorendorff]]

Profiled string-unpack-code.

Before this patch, we don't call num_parseInt much, because we're on trace and it's pure.

With the patch, we call it a lot, which indicates we're not staying on trace.

Still looking.

Comment 4

[Jason Orendorff [:jorendorff]]

See test case below.  Without the patch, we get 1 abort. With the patch, we get extra aborts like these:

...
Abort recording of tree ./string-unpack-code.js:13@18 at ./string-unpack-code.js:4@0: Inner tree is an unsupported type of recursion.
trace stopped: 4861: control flow should have been recursive
Abort recording of tree ./string-unpack-code.js:7@36 at ./string-unpack-code.js:10@102: return.
Abort recording of tree ./string-unpack-code.js:13@18 at ./string-unpack-code.js:4@0: Inner tree is an unsupported type of recursion.
...



var mochi_k = ''.split('|');

function unpack() {
    var e = function(c) {
        var xxx = "";
        if (c > 62)
            xxx = e(parseInt(c/62));
        c = c % 62;
        var yyy = c > 35 ? String.fromCharCode(c + 29) : c.toString(36);
        return xxx + yyy;
    };

    for (var c = 1976; c--; )
        e(c);
}

unpack();

Comment 5

[Jason Orendorff [:jorendorff]]

This might be due to bug 556569. dvander is looking at it now.

Comment 6

[Jason Orendorff [:jorendorff]]

With the patches in bug 530988 ("another version") and bug 556569 (WIP 2), I get no aborts at all on the test case in comment 4.

With those patches plus my THIS patch (WIP 1 in this bug), I get this:

Abort recording of tree typein:13@18 at typein:6@13: Inner tree is trying to grow, abort outer recording.
Abort recording of tree typein:13@18 at typein:4@0: Inner tree is trying to stabilize, abort outer recording.

Without my patch, the two calls to e never match typemaps. unpack calls e with a NULL vp[1], and e calls itself with a non-null vp[1] due to the CALLNAME instruction. With my patch, they do match -- but then we get the aborts above.

Comment 7

[Jason Orendorff [:jorendorff]]

Below, js-before includes bug 530988 and bug 556569; js-after additionally includes my patch. The blank lines are added for clarity.

$ TMFLAGS=abort ./js-before -j t/string-unpack-code.js 
Abort recording of tree t/string-unpack-code.js:16@98 at t/string-unpack-code.js:16@0: attempt to reenter interpreter while recording.
Abort recording of tree t/string-unpack-code.js:30@98 at t/string-unpack-code.js:30@0: attempt to reenter interpreter while recording.
Abort recording of tree t/string-unpack-code.js:51@98 at t/string-unpack-code.js:51@0: attempt to reenter interpreter while recording.
Abort recording of tree t/string-unpack-code.js:66@98 at t/string-unpack-code.js:66@0: attempt to reenter interpreter while recording.

$ TMFLAGS=abort ./js-after -j t/string-unpack-code.js 
Abort recording of tree t/string-unpack-code.js:16@31 at t/string-unpack-code.js:16@7: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:16@31 at t/string-unpack-code.js:16@0: Inner tree is trying to stabilize, abort outer recording.

Abort recording of tree t/string-unpack-code.js:30@31 at t/string-unpack-code.js:30@7: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:30@31 at t/string-unpack-code.js:30@29: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:30@31 at t/string-unpack-code.js:30@0: Inner tree is trying to stabilize, abort outer recording.
Abort recording of tree t/string-unpack-code.js:30@31 at t/string-unpack-code.js:30@50: Inner tree is trying to grow, abort outer recording.

Abort recording of tree t/string-unpack-code.js:51@31 at t/string-unpack-code.js:51@7: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:51@31 at t/string-unpack-code.js:51@0: Inner tree is trying to stabilize, abort outer recording.

Abort recording of tree t/string-unpack-code.js:66@31 at t/string-unpack-code.js:66@7: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:66@31 at t/string-unpack-code.js:66@29: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:66@31 at t/string-unpack-code.js:66@0: Inner tree is trying to stabilize, abort outer recording.
Abort recording of tree t/string-unpack-code.js:66@31 at t/string-unpack-code.js:66@50: Inner tree is trying to grow, abort outer recording.

Abort recording of tree t/string-unpack-code.js:16@31 at t/string-unpack-code.js:16@4: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:16@31 at t/string-unpack-code.js:16@4: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:16@31 at t/string-unpack-code.js:16@41: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:16@98 at t/string-unpack-code.js:16@0: attempt to reenter interpreter while recording.

Abort recording of tree t/string-unpack-code.js:30@31 at t/string-unpack-code.js:30@4: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:30@31 at t/string-unpack-code.js:30@4: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:30@31 at t/string-unpack-code.js:30@41: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:30@98 at t/string-unpack-code.js:30@0: attempt to reenter interpreter while recording.

Abort recording of tree t/string-unpack-code.js:51@31 at t/string-unpack-code.js:51@4: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:51@31 at t/string-unpack-code.js:51@4: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:51@31 at t/string-unpack-code.js:51@41: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:51@98 at t/string-unpack-code.js:51@0: attempt to reenter interpreter while recording.

Abort recording of tree t/string-unpack-code.js:66@31 at t/string-unpack-code.js:66@4: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:66@31 at t/string-unpack-code.js:66@4: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:66@31 at t/string-unpack-code.js:66@41: Inner tree is trying to grow, abort outer recording.
Abort recording of tree t/string-unpack-code.js:66@98 at t/string-unpack-code.js:66@0: attempt to reenter interpreter while recording.

Comment 8

[Jason Orendorff [:jorendorff]]

Attachment: WIP 2

Unbitrotted.

Comment 9

[Jason Orendorff [:jorendorff]]

dvander gave me the patch below. With that plus my patch in this bug, SS goes 751.5 -> 750.3 (not significant) and V8 goes 7133 -> 6823 (4.5% faster, significant, 12.3% on delta-blue, 25% on richards).

diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -6001,8 +6001,8 @@ TraceRecorder::attemptTreeCall(TreeFragm
     if (f->script == cx->fp->script) {
         if (f->recursion >= Recursion_Unwinds) {
             Blacklist(cx->fp->script->code);
-            AbortRecording(cx, "Inner tree is an unsupported type of recursion");
-            return ARECORD_ABORTED;
+            //AbortRecording(cx, "Inner tree is an unsupported type of recursion");
+            return ARECORD_CONTINUE;
         }
         f->recursion = Recursion_Disallowed;
     }

dvander's patch by itself has significant effects on two tests:
  cube:      *1.193x as slow*   39.8ms +/- 2.2%    47.5ms +/- 3.4%
  morph:     1.079x as fast     32.5ms +/- 4.3%    30.1ms +/- 3.3%
  (overall 1.007x as slow)

Comment 10

[Jason Orendorff [:jorendorff]]

I need to clean up this patch a little bit. v3 will be up for review Monday.

Comment 11

[Jason Orendorff [:jorendorff]]

Attachment: v3

This changes the behavior of thisObject hooks. Before this patch, a qualified method call `obj.m()` triggers obj's thisObject hook. With the patch, we no longer do so. It seems likely our security wrappers currently require that hook. If so, I'll file a blocking bug to put that extra work in wrapper layer rather than in the engine.

Comment 12

[Brendan Eich [:brendan]]

Comment on attachment 441511 [details] [diff] [review]
v3

> js_ComputeGlobalThis(JSContext *cx, jsval *argv)
> {
>+    JSObject *thisp = JSVAL_TO_OBJECT(argv[-2])->getGlobal();
>+    thisp = thisp->thisObject(cx);

Why not chain harder here?

>@@ -71,17 +71,30 @@ struct JSStackFrame {
>     JSFrameRegs     *regs;
>     jsbytecode      *imacpc;        /* null or interpreter macro call pc */
>     jsval           *slots;         /* variables, locals and operand stack */
>     JSObject        *callobj;       /* lazily created Call object */
>     jsval           argsobj;        /* lazily created arguments object, must be
>                                        JSVAL_OBJECT */
>     JSScript        *script;        /* script being interpreted */
>     JSFunction      *fun;           /* function being called or null */
>+
>+    /*
>+     * 'this' value or null. thisv is eagerly initialized for non-function-call

Suggest a bit more verbosity, can fiddle to make the wrapping prettier too: "The value of |this| in the code that activated this stack frame, or null if |this| is being computed lazily."

>+     * frames and qualified method calls, but lazily initialized in most
>+     * unqualified function calls.  JSVAL_IS_NULL(thisv) indicates this.
>+     * See getThisObject().

"JSVAL_IS_NULL(thisv) indicates this" invites pronoun trouble, to borrow from D. Duck. Rewording the intro means you can drop this sentence (oops, I wrote "this").

>+     *
>+     * Usually if argv != NULL then thisv == argv[-1], but there is a special
>+     * case, thanks to obj_eval, where two stack frames have the same argv. If
>+     * one of the frames fills in both argv[-1] and thisv, the other frame's
>+     * thisv is left null.

There's also simply two roots, not one root, for natives to take advantage of. I.e., a slow native may overwrite argv[-1] but still use obj safely after a GC.

> /* JS stack frame flags. */
> #define JSFRAME_CONSTRUCTING   0x01 /* frame is for a constructor invocation */
>-#define JSFRAME_COMPUTED_THIS  0x02 /* frame.thisv was computed already and
>-                                       JSVAL_IS_OBJECT(thisv) */

Rotate JSFRAME_OVERRIDE_ARGS down to 0x02 to avoid a gap.

>+#define SLOW_PUSH_THISV(cx, obj)                                              \
>+    JS_BEGIN_MACRO                                                            \
>+        JSClass *clasp;                                                       \
>+        JSObject *thisp = obj;                                                \
>+        if (!thisp->getParent() ||                                            \
>+            (clasp = thisp->getClass()) == &js_CallClass ||                   \
>+            clasp == &js_BlockClass ||                                        \
>+            clasp == &js_DeclEnvClass) {                                      \
>+            /* Normal case: thisp is global or an activation record. */       \
>+            /* Callee determines 'this'. */                                   \
>+            thisp = NULL;                                                     \

Ironically (in light of the SLOW_... name), this class-testing is for performance, right?

>+        } else {                                                              \
>+            /* thisp is a With object or, even stranger, a plain nonglobal */ \
>+            /* object on the scope chain. Caller determines 'this'. */        \
>+            thisp = thisp->thisObject(cx);                                    \
>+            if (!thisp)                                                       \
>+                goto error;                                                   \

I.e., the relevant thisObject hook would do the right thing with those magic classes?

> BEGIN_CASE(JSOP_NAME)
> BEGIN_CASE(JSOP_CALLNAME)

Nice cleanup here!

>+    if (!fp->fun) {
>+        // Top-level code. It is an invariant of the interpreter that

Doc'ed on https://developer.mozilla.org/SpiderMonkey_Internals/Invariants ?

>+        // fp->thisv is populated. Furthermore, we would not be recording

Nit: suggest "initialized" not "populated".

mrbkap really should have a look too. r=me with above accounted for.

/be

Comment 13

[Jason Orendorff [:jorendorff]]

Thanks for the comments, especially this one:
> There's also simply two roots, not one root, for natives to take advantage of.

> Ironically (in light of the SLOW_... name), this class-testing is for
> performance, right?

Sort of. This property cache miss case is the slow path that has to pay for the simplification elsewhere (esp. in the tracer).

> >+        } else {                                                         \
> >+            /* thisp is a With object or, even stranger, a plain nongloba\
> >+            /* object on the scope chain. Caller determines 'this'. */   \
> >+            thisp = thisp->thisObject(cx);                               \
> >+            if (!thisp)                                                  \
> >+                goto error;                                              \
> 
> I.e., the relevant thisObject hook would do the right thing with those magic
> classes?

Yes. The only change in behavior in this case is that thisp->thisObject(cx) happens a little sooner (eagerly rather than lazily).

Comment 14

[Jason Orendorff [:jorendorff]]

Attachment: v4 - address review comments

Carrying forward brendan's r+.

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 441587 [details] [diff] [review]
v4 - address review comments

This looks mostly good to me. The only quibble I had was that this might mean that a |this| object might flow in through an API entry point (such as JS_CallFunctionValue) and never have its thisObject hook called. jorendorff and I talked about this on IRC and he has a plan.

Comment 16

[Jason Orendorff [:jorendorff]]

Attachment: v5 - rebased to tip

There is a bug; this chrome mochitest fails:

js/src/xpconnect/tests/chrome/test_wrappers.xul

    win.setTimeout(function() {
                       is(utils.getClassName(this), "XPCNativeWrapper",
                          "this is wrapped correctly");
                       SimpleTest.finish();
                   }, 0)

Here the function is chrome, but 'win' is the content window of an iframe with href="http://example.org/tests/js/src/xpconnect/tests/mochitest/chrome_wrappers_helper.html".

Expected: setTimeout calls the function with null this-argument; when we evaluate 'this' we compute callee->getGlobal()->innerize(). So we should get whatever we normally use to wrap a chrome outer window for chrome. (I don't know which kind it would be.)

Actual: I'm not sure how this happens, but we get:

  failed | this is wrapped correctly - got "XPCCrossOriginWrapper",
    expected "XPCNativeWrapper"

Comment 17

[Jason Orendorff [:jorendorff]]

Separately, I think Blake may have later realized that calling the thisObject hook earlier can cause it to produce the wrong result sometimes. At least, I think he mentioned something like that to me weeks ago, and we both forgot about it.

Comment 18

[Jason Orendorff [:jorendorff]]

Comment 17 can be fixed just by calling thisObject a little bit later in js_Invoke. Working on it.

Comment 19

[Jason Orendorff [:jorendorff]]

Attachment: v6

Carrying forward review. But the interdiff from v5 to v6 is small enough and interesting enough to deserve a second look, if you have some time.

There were two bugs.  First, as noted above, the thisObject hook wants to see a fully initialized stack frame, so I changed js_InvokeInternal/js_Invoke/etc. to call the thisObject hook later.

Second, the newly-simplified js_ComputeThis was giving the wrong answer in this particular case, because nsJSContext::CallEventHandler calls JS_CallFunctionValue with scary values and expects the engine or XPConnect to fix things up. So instead of calling js_ComputeThis, js_Invoke now computes this directly inline and does the extra work nsJSContext::CallEventHandler requires.

Comment 20

[Jason Orendorff [:jorendorff]]

When I try-servered this I got some apparently random orange:
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1275642723.1275646196.10804.gz

I don't think I caused this but it doesn't match anything known AFAICT. I'll ask philor when he shows up on IRC.

Comment 21

[Jason Orendorff [:jorendorff]]

BTW, in case comment 19 was too vague, what I "Expected:" in comment 16 just wasn't the right thing to expect.

The path was:

  nsGlobalWindow::RunTimeout
    nsJSContext::CallEventHandler
      JS_CallFunctionValue

nsGlobalWindow::RunTimeout passes the content window (not NULL) as 'this' when it calls the timeout function.

Comment 22

[Jason Orendorff [:jorendorff]]

http://hg.mozilla.org/tracemonkey/rev/52be13ea0488

Comment 23

[Robert Sayre]

I backed this out on suspicion of causing a regression in the Sunspider-0.9.1 harness of ~10%, with string-unpack-code going from 80ms to 135ms.

http://hg.mozilla.org/tracemonkey/rev/0ba3c58afb3a

A regression also appeared on arewefastyet.com. Using tinderbox builds, I confirmed that the regression appeared in this push:

http://hg.mozilla.org/tracemonkey/pushloghtml?changeset=52be13ea0488

Comment 24

[Robert Sayre]

However, we should figure out what's happening there ASAP, because this patch also took our v8-richards score from 2300 -> 3000 in the v8 harness (higher is better)

Comment 25

[Robert Sayre]

Confirmed with post-backout tinderbox build: string-unpack code is back to normal, while we lose our huge v8-richards win.

Comment 26

[Nicholas Nethercote [inactive]]

The backout is causing js1_8_5/regress/regress-555246-1.js (see bug 555246) to fail on my Linux box.  'hg bisect' says:

The first bad revision is:
changeset:   42836:0ba3c58afb3a
parent:      42832:52be13ea0488
user:        Robert Sayre <[e-mail]>
date:        Sat Jun 05 11:42:59 2010 -0400
summary:     Backed out changeset 52be13ea0488. Bug 556277 - Compute this eagerly in more cases. r=brendan. Suspected of performance regression on SunSpider unpack-code. 80ms -> 135ms.

Comment 27

[Robert Sayre]

Jason tried landing this again. It is a win on unpack-code in the shell, but it loses in browser badly, because XPC_WN_JSOp_ThisObject takes +14% of the test time.

Comment 28

[Jason Orendorff [:jorendorff]]

(In reply to comment #26)
> The backout is causing js1_8_5/regress/regress-555246-1.js (see bug 555246) to
> fail on my Linux box. [...]

This is failing for me too, and so is regress-555246-0.js. But I bet it was just as broken before this landed as after the backout.

Investigating.

Comment 29

[Jason Orendorff [:jorendorff]]

regress-555246-0.js was failing due to a patch in bug 563099 that I was trying out. Sorry for the noise.

regress-555246-1.js has always failed. However, until recently the test was marked as "fails". I changed the jstests.list file to expect it to pass in revision 8f08ae0b74df. That was a mistake. I have now changed it back.

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

Comment 30

[Jason Orendorff [:jorendorff]]

I can't work on this but I really want it to land very much. Waldo said he would take a look.

Comment 31

[Damon Sicore (:damons)]

Can we get an owner here, please?

Comment 32

[Jeff Walden [:Waldo]]

Leary, remind me what the status of this was?  It's making me hold off on strict-mode-doesn't-touch-this right now in bug 514570.

Comment 33

[Jason Orendorff [:jorendorff]]

cdleary landed this in TM, in two pieces. See bug 574697 and bug 587809.