Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Don't alias stack variables

RESOLVED FIXED in mozilla16

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: dvander, Assigned: luke)

Tracking

(Blocks: 1 bug)

unspecified
mozilla16
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:p1:fx15])

Attachments

(7 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Call objects are allocated with a slots vector, and closed variables initially live on the interpreter stack. When an activation is finished, those closed variables are copied into the call object's slots.

We should consider removing these variables off the stack and making callObj->dslots their canonical location. They wouldn't contribute to |script->nfixed| and we'd have new opcodes like GETCALLSLOT etc. For closed arguments, the act of creating a call object would copy closed arguments.

Pros:
 * Greatly simplify JIT work required to support closed variables.
 * Eliminate callObj->fp() testing.
 * Eliminate PutCallObject from the critical path - it shows up in profiles,
   like in earley-boyer.

Cons:
 * Accessing closed variables within their defining scope gets a little more
   expensive.
(Assignee)

Comment 1

6 years ago
Sounds great.  And hey, the obvious implementation may even end up fixing bug 577572.
Blocks: 579390
(Assignee)

Updated

6 years ago
Assignee: general → luke
Summary: Eliminate stack storage for closed variables → Eliminate stack storage for closed variables / create activation objects eagerly or not at all
(Assignee)

Updated

6 years ago
Duplicate of this bug: 671360
(Assignee)

Updated

6 years ago
Depends on: 632064
(Assignee)

Updated

6 years ago
Depends on: 690135
(Assignee)

Updated

6 years ago
Depends on: 692274
(Assignee)

Updated

6 years ago
Depends on: 718022
(Assignee)

Updated

5 years ago
Depends on: 736555
(Assignee)

Updated

5 years ago
Duplicate of this bug: 494637
(Assignee)

Updated

5 years ago
Blocks: 753158
(Assignee)

Comment 4

5 years ago
Created attachment 622646 [details] [diff] [review]
combined patch (WIP)

WIP note: the combined set of patches (including dependent bugs) is passing shell and browser debugger tests with --disable-methodjit.  So, pretty close.
(Assignee)

Comment 5

5 years ago
Created attachment 623970 [details] [diff] [review]
combined patch (WIP 2)

Last patch (finally) basically done and passing tests (still need to try-server and fill in TODO comments etc).

On early-boyer running in the shell, my machine is really noisy, but I think I see a ~10% score improvement.  Cachegrind shows a 14% icount reduction and 12% dref reduction.
Attachment #622646 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Depends on: 755396
(Assignee)

Comment 6

5 years ago
Created attachment 624577 [details] [diff] [review]
don't use ScopeObject::maybeStackFrame part 1

This patch fixes up all the DebugScopeObject code added by bug 690135 to not depend on ScopeObject::maybeStackFrame (since it is removed by this bug).

The strategy is pretty simple: maintain a map ScopeObject* --> StackFrame* that simulates maybeStackFrame (with a memoization strategy to avoid O(n^2) stack walking).
Attachment #623970 - Attachment is obsolete: true
Attachment #624577 - Flags: review?(jimb)
(Assignee)

Comment 7

5 years ago
Created attachment 624580 [details] [diff] [review]
don't use ScopeObject::maybeStackFrame part 2

This patch removes the indirect dependency on maybeStackFrame: since ScopeObjects will no longer alias the stack frame, we need to stop proxying all reads/writes.  This requires hoisting this logic into a new DebugScopeProxy::unaliasedAccess.  (witness the power of DebugScopeProxy!)

Another trick is that, since js_PutCallObject isn't doing it for us, we have to copy unaliased locals/args into Call/Block objects before they are popped.
Attachment #624580 - Flags: review?(jimb)
(Assignee)

Comment 8

5 years ago
Created attachment 624581 [details] [diff] [review]
emit ScopeCoordinate::hops

This adds the final bit to the front-end to actually emit ScopeCoordinate::hops.  This patch just asserts we got it right and still aliases the stack frame as always.
Attachment #624581 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 9

5 years ago
Created attachment 624583 [details] [diff] [review]
put the blockChain into ScopeCoordinate

This patch stores the block index in the opcode instead of the atom index.  The atom index is derivable from the block index (see ScopeCoordinateAtom), but the additional block index information allows us to avoid the hard question "what is the block index at some random pc" (normally we rely on fp->maybeBlockChain, but that only works if you are asking about fp's current pc).
Attachment #624583 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 10

5 years ago
Created attachment 624584 [details] [diff] [review]
stop aliasing StackFrame, completely breaking the mjit

This patch finally does the deed.
Attachment #624584 - Flags: review?(bhackett1024)
(Assignee)

Comment 11

5 years ago
Created attachment 624587 [details] [diff] [review]
fix the mjit

This patch fixes the mjit by emitting code to access ArgumentsObject and ScopeObject.  The thought-provoking part was the change (made in the previous patch) of having a single StackFrame::prologue that is always executed by the callee (instead of the Invoke).  I think this makes things a good bit simpler and, later, dvander and I would like to see the prologue split out (again) into a JSOP_BEGIN.
Attachment #624587 - Flags: review?(bhackett1024)
(Assignee)

Comment 12

5 years ago
The whole stack is green on try:
https://tbpl.mozilla.org/?tree=Try&rev=5c4ac9959dfc
Comment on attachment 624584 [details] [diff] [review]
stop aliasing StackFrame, completely breaking the mjit

Review of attachment 624584 [details] [diff] [review]:
-----------------------------------------------------------------

Yay!

::: js/src/vm/ScopeObject.cpp
@@ +626,5 @@
>  
> +    /*
> +     * Copy in the closed-over locals. Closed-over locals don't need
> +     * any fixup since the initial value is 'undefined'.
> +     */

I don't understand what this comment means.
Attachment #624584 - Flags: review?(bhackett1024) → review+
Attachment #624587 - Flags: review?(bhackett1024) → review+
Comment on attachment 624581 [details] [diff] [review]
emit ScopeCoordinate::hops

Review of attachment 624581 [details] [diff] [review]:
-----------------------------------------------------------------

I really haven't been following this work well enough to review this well -- I don't recognize ScopeCoordinate or any of the aliased-var code or terminology, except for a gist I've gleaned from the occasional discussion -- but whatever.

::: js/src/jsinterp.cpp
@@ +1191,5 @@
>  # define END_CASE_LEN8      len = 8; goto advance_pc;
>  # define END_CASE_LEN9      len = 9; goto advance_pc;
>  # define END_CASE_LEN10     len = 10; goto advance_pc;
> +# define END_CASE_LEN11     len = 11; goto advance_pc;
> +# define END_CASE_LEN12     len = 12; goto advance_pc;

Maybe we should round up to page size for better cache locality?

::: js/src/vm/ScopeObject-inl.h
@@ +47,5 @@
>  
>  inline
>  ScopeCoordinate::ScopeCoordinate(jsbytecode *pc)
> +  : hops(GET_UINT16(pc)), binding(GET_UINT16(pc + 2)),
> +    frameBinding(GET_UINT16(pc + 8))

pc + 2 + 2 + 4 perhaps.

@@ +130,5 @@
> +        } else {
> +            unsigned var = bindings.bindingToLocal(sc.binding);
> +            JS_ASSERT(fp->script()->varIsAliased(var));
> +            fp->localSlot(var) = v;
> +        }

Parity with aliasedVar would seem to suggest an early return here, rather than the else structure.
Attachment #624581 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 624583 [details] [diff] [review]
put the blockChain into ScopeCoordinate

Review of attachment 624583 [details] [diff] [review]:
-----------------------------------------------------------------

Also kinda graspy.

::: js/src/vm/ScopeObject.h
@@ +76,5 @@
> +extern StaticBlockObject *
> +ScopeCoordinateBlockChain(JSScript *script, jsbytecode *pc);
> +
> +/* Return the name being accessed by the given ALIASEDVAR op. */
> +extern JSAtom *

PropertyName
Attachment #624583 - Flags: review?(jwalden+bmo) → review+
Whiteboard: [js:p1:fx15][js:ni]

Comment 16

5 years ago
Comment on attachment 624577 [details] [diff] [review]
don't use ScopeObject::maybeStackFrame part 1

Review of attachment 624577 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit-test/tests/basic/eif-generator.js
@@ +1,2 @@
>  // |jit-test| debug
> +// |jit-test| debug

This looks like a cut-and-paste error?

::: js/src/vm/ScopeObject.cpp
@@ +1672,5 @@
> +DebugScopes::updateLiveScopes(JSContext *cx)
> +{
> +    JS_CHECK_RECURSION(cx, return false);
> +
> +    for (AllFramesIter i(cx->runtime->stackSpace); !i.done(); ++i) {

Would this text be correct as a comment? It took me a bit to figure this out:

Note that we must always update the top frame's scope objects' entries in liveScopes, because we can't be sure code hasn't run in that frame to change the scope chain since we were last called. This means that each call to GetDebugScopeForFrame or GetDebugScopeForFunction will run the inner loop below, over this frame's virtual scope objects.

The fp->prevUpToDate() flag indicates whether the frames older than fp are already accurately described by liveScopes. It might seem simpler to have fp instead carry a flag indicating whether fp itself is accurately described, but then we would need to clear that flag whenever fp ran code. By storing the 'up to date' bit for fp->prev() in fp, simply popping fp effectively clears the flag for us, at exactly the time when execution resumes in the older frame.

@@ +1682,5 @@
> +            if (si.hasScopeObject() && !liveScopes.put(&si.scope(), fp))
> +                return false;
> +        }
> +
> +        /* liveScopes will accurately reflect fp->prev() until fp is popped. */

I was confused by this comment. I think it should say:

/* liveScopes will accurately reflect all frames older than fp until fp is popped. */

::: js/src/vm/ScopeObject.h
@@ +465,5 @@
> +     * two lazy updates, liveScopes becomes incomplete (but not invalid, onPop*
> +     * removes scopes as they are popped). Thus, two consecutive debugger lazy
> +     * updates of liveScopes need only fill in the new scopes.
> +     */
> +    typedef HashMap<ScopeObject *, StackFrame *, DefaultHasher<ScopeObject *>, RuntimeAllocPolicy> LiveScopeMap;

nit: Source code should fit in 100 columns.

::: js/src/vm/Stack.cpp
@@ +292,5 @@
>  void
>  StackFrame::popWith(JSContext *cx)
>  {
> +    if (cx->compartment->debugMode())
> +        cx->runtime->debugScopes->onPopWith(this);

It's possible to leave debug mode with frames on the stack; if you add an assertion to JSCompartment::updateForDebugMode, then you can see that jit-test/test/debug/Debugger-debuggees-{15,16}.js cause this to happen.

If that occurs, then it seems like this code will leave referencse to dead frames in liveScopes.

The following extant code has the same issue (all r=me, sigh):
- StackFrame::functionEpilogue calling onPopCall
- StackFrame::popBlock calling onPopBlock
- js::ExecuteKernel calling onPopStrictEvalScope (in this patch)
- StackFrame::stealFrameAndSlots calling onGeneratorFrameChange(maybe not?)

Perhaps it would be sufficient to, when the compartment leaves debug mode, pre-emptively remove all liveScopes entries for objects in that compartment.

Updated

5 years ago
Attachment #624577 - Flags: review?(jimb) → review-

Comment 17

5 years ago
Is this an accurate diagram of what we're aiming for (once aliased variables no longer ever live on the stack)?

https://github.com/jimblandy/DebuggerDocs/blob/master/debugger-scopes.png

Comment 18

5 years ago
Better link:
https://github.com/jimblandy/DebuggerDocs/raw/master/debugger-scopes.png
(Assignee)

Comment 19

5 years ago
(In reply to Jim Blandy :jimb from comment #16)
> Perhaps it would be sufficient to, when the compartment leaves debug mode,
> pre-emptively remove all liveScopes entries for objects in that compartment.

This is what onCompartmentLeaveDebugMode already does in the patch :)  While I get to the nits, perhaps you'd like to reconsider the r-?

(In reply to Jim Blandy :jimb from comment #17)
> Is this an accurate diagram of what we're aiming for (once aliased variables
> no longer ever live on the stack)?

Looks good to me.  The only interesting addition is that, when the stack frame is popped, 'scope' holds *all* the slots, closed or not.  (I think that's in part 2, though.)
(Assignee)

Updated

5 years ago
Attachment #624577 - Flags: review- → review?(jimb)

Comment 20

5 years ago
Comment on attachment 624577 [details] [diff] [review]
don't use ScopeObject::maybeStackFrame part 1

Review of attachment 624577 [details] [diff] [review]:
-----------------------------------------------------------------

D'oh! :( I see it now.

I'm always willing to reconsider an r-. :)

::: js/src/vm/ScopeObject.cpp
@@ +1497,5 @@
>          if (IsAboutToBeFinalized(e.front().value))
>              e.removeFront();
>      }
> +
> +    /* Scopes can be finalized when a suspended generator becomes garbage. */

I know I'm supposed to know how generators work, but it would still be nice to have a little more of a comment here.

In the usual case, we remove scope objects from liveScopes as soon as we leave their dynamic scope; and scopeChain_ itself holds them live until that time. But in the generator case, we do not pop BlockObjects and WithObjects when we yield from within them --- so they may get collected before they're popped. Right?

@@ +1573,5 @@
>  {
>      if (fp->isYielding())
>          return;
>  
> +    if (fp->fun()->isHeavyweight()) {

So there's a symmetry here: reified environment ribs need to have entries removed from liveScopes; whereas non-reified environment ribs may need to have entries for fake scope chain objects we created for the debugger. Both BlockObjects and WithObjects get the same logic.
Attachment #624577 - Flags: review?(jimb) → review+

Comment 21

5 years ago
Comment on attachment 624580 [details] [diff] [review]
don't use ScopeObject::maybeStackFrame part 2

Review of attachment 624580 [details] [diff] [review]:
-----------------------------------------------------------------

Wow, looks great.

::: js/src/jit-test/tests/debug/Frame-eval-13.js
@@ +7,5 @@
> +g.eval("var surprise = null");
> +
> +var dbg = new Debugger(g);
> +dbg.onDebuggerStatement = function(hFrame) {
> +    hit = true;

This variable isn't used.

::: js/src/vm/ScopeObject.cpp
@@ +1312,5 @@
> +     *
> +     * unaliasedAccess returns 'true' if the access was unaliased and completed
> +     * by unaliasedAccess.
> +     */
> +    bool unaliasedAccess(JSContext *cx, ScopeObject &scope, jsid id, Action action, Value *vp)

I like this a lot. This is sort of the kind of thing I've been expecting DebugObjectScope to do from the start.

nit: I'd prefer the name be verby, like handleUnaliasedAccess, or accessUnaliased. At first I misread its uses below as a call to a predicate, not the thing which *did* the access. But only fix if you agree.

@@ +1324,5 @@
> +        if (scope.isCall() && !scope.asCall().isForEval()) {
> +            CallObject &callobj = scope.asCall();
> +            JSScript *script = callobj.getCalleeFunction()->script();
> +            if (!script->ensureHasTypes(cx))
> +                return false;

This is an error return, right? Is it a problem that this function uses 'false' to mean both 'this references is not my business' and 'an error occurred'? (If it's guaranteed that ensureHasTypes only throws OOM errors, then this could be okay, but it's a little strange.)

@@ +1328,5 @@
> +                return false;
> +
> +            if (shape->setterOp() == CallObject::setVarOp) {
> +                unsigned i = shape->shortid();
> +                if (!script->varIsAliased(i)) {

nit: it might be clearer to just say:

if (script->valIsAliased(i))
  return false;

and leave the rest unindented. Similarly for the arg and block cases.

@@ +1344,5 @@
> +                    if (action == SET)
> +                        TypeScript::SetLocal(cx, script, i, *vp);
> +                    return true;
> +                }
> +            } else if (shape->setterOp() == CallObject::setArgOp) {

nit: Brendan liked to discourage 'return after else' in SM. I think he'd either put the 'return false' in the final 'else', and make the final return in the function return true, or just have each clause return and remove the 'else's.

@@ +1799,5 @@
>                  liveScopes.add(livePtr, &toIter.scope(), to);
>          } else {
>              if (MissingScopeMap::Ptr p = missingScopes.lookup(ScopeIter(toIter, from))) {
>                  DebugScopeObject &debugScope = *p->value;
> +                liveScopes.lookup(&debugScope.scope())->value = to;

The reason we no longer need to change the frames here is that the ScopeObjects synthesized for the debugger no longer use their stack frame pointers; and in fact, they are set to NULL in GetDebugScopeForMissing. Is that right?
Attachment #624580 - Flags: review?(jimb) → review+

Comment 22

5 years ago
Is this going to land for fx15?
(Assignee)

Comment 23

5 years ago
(In reply to Jim Blandy :jimb from comment #16)
Thanks for all the great comments!  Sorry for taking so long to respond after the review; I've been unexpectedly swamped.

> Would this text be correct as a comment? It took me a bit to figure this out:

Exactly.

(In reply to Jim Blandy :jimb from comment #20)
> But in the generator case, we do not pop BlockObjects and WithObjects
> when we yield from within them --- so they may get collected before they're
> popped. Right?

Right

(In reply to Jim Blandy :jimb from comment #21)
> I like this a lot. This is sort of the kind of thing I've been expecting
> DebugObjectScope to do from the start.

It was the plan from the start, just had to build up to it :)

> (If it's guaranteed that ensureHasTypes only throws OOM errors,
> then this could be okay, but it's a little strange.)

That's the case.  It is weird, but it seemed preferable to adding a whole new case (which grosses up callers) for this extreme corner case.

> The reason we no longer need to change the frames here is that the
> ScopeObjects synthesized for the debugger no longer use their stack frame
> pointers; and in fact, they are set to NULL in GetDebugScopeForMissing. Is
> that right?

Correct
(Assignee)

Updated

5 years ago
Summary: Eliminate stack storage for closed variables / create activation objects eagerly or not at all → Don't alias stack variables
(Assignee)

Comment 24

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7636c7036e2e
https://hg.mozilla.org/integration/mozilla-inbound/rev/35d64eea8385
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee37d2b88eeb
https://hg.mozilla.org/integration/mozilla-inbound/rev/ddc63b39ab57
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee940e4debd0
Target Milestone: --- → mozilla15
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/727f3e801afb

https://tbpl.mozilla.org/php/getParsedLog.php?id=12262384&tree=Mozilla-Inbound (and the rest of the debugs)
TEST-UNEXPECTED-FAIL | jit_test.py -a -m -d | /builds/slave/m-in-lnx64-dbg/build/js/src/jit-test/tests/jaeger/testBug550743.js: Assertion failure: gen->state == JSGEN_OPEN, at ../../../js/src/vm/ScopeObject.cpp:1476
Target Milestone: mozilla15 → ---
(Assignee)

Comment 26

5 years ago
That's what I get for tweaking after try'ing.
(Assignee)

Comment 27

5 years ago
Created attachment 629401 [details] [diff] [review]
combined patch for fuzzing (applies to cset 123be6501757)

On second thought, I would really appreciate a dose of fuzzing.
Attachment #629401 - Flags: feedback?(gary)
Attachment #629401 - Flags: feedback?(choller)
https://hg.mozilla.org/mozilla-central/rev/7636c7036e2e
https://hg.mozilla.org/mozilla-central/rev/35d64eea8385
https://hg.mozilla.org/mozilla-central/rev/ee37d2b88eeb
https://hg.mozilla.org/mozilla-central/rev/ddc63b39ab57
https://hg.mozilla.org/mozilla-central/rev/ee940e4debd0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Backed out: https://hg.mozilla.org/mozilla-central/rev/727f3e801afb
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Luke Wagner [:luke] from comment #27)
> Created attachment 629401 [details] [diff] [review]
> combined patch for fuzzing (applies to cset 123be6501757)
> 
> On second thought, I would really appreciate a dose of fuzzing.

Patched on top of m-c revision 68abc86fde1c:

(function({
    l
}) {
    eval()
})()

Assertion failure: JSID_IS_STRING(iden),

===

(with -d:)

try {
    function f() {}
    (1 for (x in []))
} catch (e) {}
gc()

Assertion failure: gen->state == JSGEN_OPEN,
Attachment #629401 - Flags: feedback?(gary) → feedback-
(Assignee)

Comment 31

5 years ago
Created attachment 629550 [details] [diff] [review]
combined patch for fuzzing (applies to cset 123be6501757) (v.2)

Ah, 1 overly-strict assert (need to add || state == JSGEN_NEWBORN) and 1 decompile-value-generator + destructuring formals.  Let's try one more time.
Attachment #629401 - Attachment is obsolete: true
Attachment #629401 - Flags: feedback?(choller)
Attachment #629550 - Flags: feedback?(gary)
Attachment #629550 - Flags: feedback?(choller)
Comment on attachment 629550 [details] [diff] [review]
combined patch for fuzzing (applies to cset 123be6501757) (v.2)

Almost there, not there yet:

Function("for(;(function(){([x],0)});x){}var x")

Assertion failure: bce->sc->funIsHeavyweight(),

(tested on 64-bit debug js shell with patch on top of m-c changeset d0ebcaa7efb5)
Attachment #629550 - Flags: feedback?(gary) → feedback-
Comment on attachment 629550 [details] [diff] [review]
combined patch for fuzzing (applies to cset 123be6501757) (v.2)

To be fair, other than the:

Assertion failure: bce->sc->funIsHeavyweight(),

in comment 32, at this point in time I don't think jsfunfuzz has found anything more, so turning to feedback+ pending this assertion fix.
Attachment #629550 - Flags: feedback- → feedback+
Comment on attachment 629550 [details] [diff] [review]
combined patch for fuzzing (applies to cset 123be6501757) (v.2)

I got three issues here that only occur with this patch:


First (64 bit dbg, options -m -n -a): Assertion failure: false, at methodjit/InvokeHelpers.cpp:915

gczeal(2);
function C(i) { 
  this.m *=  function () { return "'no'\u001D+' error'" <<  this & dbg[i]; }
}
var a = [];
for (var i = 0; i < 5; ((function  () { return 42; } )))
    a[a.length] = new C(i);



Second (64 bit dbg, options -m -n -a): Assertion failure: [barrier verifier] Unmarked edge: <unknown>, at jsgc.cpp:4443

gczeal(4);
evaluate("\
Date.formatFunctions = {count:0};\
Date.prototype.dateFormat = function(format) {\
    var funcName = 'format' + Date.formatFunctions.count++;\
    var code = 'Date.prototype.' + funcName + ' = function(){return ';\
    var ch = '';\
    for (var i = 0; i < format.length; ++i) {\
        ch = format.charAt(i);\
        eval(code.substring(0, code.length - 3) + ';}');\
    }\
};\
var date = new Date('1/1/2007 1:11:11');\
    var shortFormat = date.dateFormat('Y-m-d');\
");



Third (64 bit dbg, options -m -n -a): Assertion failure: (ptrBits & 0x7) == 0, at ../../jsval.h:702

function TestCase(n, d, e, a) {}
function enterFunc (funcName)
function test() {}
try {
test('c{3,4}');
} catch(exc1) {}
function test() {
  enterFunc ('test');
  try  { 
    eval('for each (this in []) { }');
  } catch ( [ [r, g, b]  , l   , v  , e       ]   ) { 
  }  
}
new TestCase( "15.5.2", "String()", "", String() );
test();
Attachment #629550 - Flags: feedback?(choller) → feedback-
(Assignee)

Comment 35

5 years ago
Thanks guys!  2 overzealous asserts, 1 thinko in the mjit throw path, and missing write barriers for the SETALIASEDVAR jit paths (oh hai incremental gc).
(Assignee)

Comment 36

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d657e8cb0201
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ff46668b156
https://hg.mozilla.org/integration/mozilla-inbound/rev/4832054e4e42
https://hg.mozilla.org/integration/mozilla-inbound/rev/75e9c37626e8
https://hg.mozilla.org/integration/mozilla-inbound/rev/b863ef9946b8
(Assignee)

Comment 37

5 years ago
Talking to Bill just now, we realized that the weak pointers in DebugScopes need read barriers for incremental GC:

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe0b1390710f
Target Milestone: mozilla15 → mozilla16
(Assignee)

Updated

5 years ago
Blocks: 478174
https://hg.mozilla.org/mozilla-central/rev/d657e8cb0201
https://hg.mozilla.org/mozilla-central/rev/2ff46668b156
https://hg.mozilla.org/mozilla-central/rev/4832054e4e42
https://hg.mozilla.org/mozilla-central/rev/75e9c37626e8
https://hg.mozilla.org/mozilla-central/rev/b863ef9946b8
https://hg.mozilla.org/mozilla-central/rev/fe0b1390710f
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 39

5 years ago
Yay!!!!
Blocks: 761685

Updated

5 years ago
Depends on: 762105

Updated

5 years ago
Depends on: 762473
Depends on: 705423
(Assignee)

Updated

5 years ago
Depends on: 763950
(Assignee)

Updated

5 years ago
No longer depends on: 763950
(Assignee)

Updated

5 years ago
Depends on: 763950
Depends on: 761864
Depends on: 773108

Updated

5 years ago
Blocks: 762197
Whiteboard: [js:p1:fx15][js:ni] → [js:p1:fx15]
Depends on: 761863
You need to log in before you can comment on or make changes to this bug.