gc(); var recursiveFunctions = [{ text: "(function(){if(a){}g()})" }]; (function testAllRecursiveFunctions() { for (var i = 0; i < recursiveFunctions.length; ++i) { var a = recursiveFunctions[i]; eval(a.text.replace(/@/g, "")) } })(); gcslice(2868); Function("v={c:[{x:[[]],N:{x:[{}[d]]}}]}=minorgc(true)")() crashes js debug shell (tested with a threadsafe deterministic 64-bit debug build) on m-c changeset c193fdeb4932 without any CLI arguments at Assertion failure: thing->isAligned(), at gc/Marking.cpp when the shell is compiled with --enable-gcgenerational --enable-exact-rooting.

(function(){}) gc(); var recursiveFunctions = [{ text: "(function(){if(a){}g()})" }]; (function testAllRecursiveFunctions() { for (var i = 0; i < recursiveFunctions.length; ++i) { var a = recursiveFunctions[i]; eval(a.text.replace(/@/g, "")) } })(); gcslice(2868); Function("v={c:[{x:[[]],N:{x:[{}[d]]}}]}=minorgc(true)")() Assertion failure: MapTypeToTraceKind<T>::kind == GetGCThingTraceKind(thing), at gc/Marking.cpp when testcase is passed in as a CLI argument.

function f(){} gc(); var recursiveFunctions = [{ text: "(function(){if(a){}g()})" }]; (function testAllRecursiveFunctions() { for (var i = 0; i < recursiveFunctions.length; ++i) { var a = recursiveFunctions[i]; eval(a.text.replace(/@/g, "")) } })(); gcslice(2868); Function("v={c:[{x:[[]],N:{x:[{}[d]]}}]}=minorgc(true)")() Assertion failure: !InFreeList(thing->arenaHeader(), thing), at gc/Marking.cpp

The problem exposed here is that the minor GC is happening between sweep slices. In this case, a JSFunction's LazyScript has been swept, but the JSFunction is background finalized, so hasn't been swept yet. We don't run the background finalizer until fully after the incremental GC, not between slices as I thought. The obvious solution here is to only mark in the minor GC if we are not between sweep slices or the thing is marked. However, this make some other ggc tests fail. I did not do a full investigation, but we end up with a tenured array in the mark stack and one or more of its slots in the nursery. It should have been relocated, presumably, by the minorGC we run at the start of each GC slice. Apparently, we need to reify even dead objects. Bill and I discussed several potential solutions, none of which are particularly awesome. I thought of one other solution a few seconds after Bill walked out the door though, which I think is somewhat more agreeable. Bill, the exact assertion that is firing is: /* * Try to assert that the thing is allocated. This is complicated by the * fact that allocated things may still contain the poison pattern if that * part has not been overwritten, and that the free span list head in the * ArenaHeader may not be synced with the real one in ArenaLists. */ JS_ASSERT_IF(IsThingPoisoned(thing) && rt->isHeapBusy(), !InFreeList(thing->arenaHeader(), thing)); This is the last statement in CheckMarkedThing and the last statement before we enter our custom mark function. Our minor GC marker will immediately exit without observing the object's broken contents because this is not a nursery thing. There are a handful of checks in CheckMarkedThing that may observe |thing|'s contents, but only as a side-effect. Secondly, we only have to worry about edges from background finalized things to foreground finalized things. This is a very small set: I think it may actually be just this one edge. Thus, I think the right solution here is to just skip these checks when in a minor GC and in a gc state that may have an incoherent heap. Bill, to answer your question from earlier about how this test overflowed the store buffer in the first place: the test calls minorgc(true), which explicitly sets the overflowed bit on the store buffer.

Comment on attachment 771716 [details] [diff] [review] v0 Sorry, I don't think this is a good solution. It makes our invariants way more complex and it's likely to blow up in our faces later. Here are the options: 1. Eliminate fallback marking and crash if we run out of dynamically-allocated store buffer space. This seems fine to me, particularly given that we're only in this situation because the test case explicitly asked for it. 2. I realized that we should be able to run background finalization before the minor GC without too much trouble. I think you should be able to do something like this: /* Set up list of zones for sweeping of background things. */ JS_ASSERT(!rt->gcSweepingZones); for (GCZonesIter zone(rt); !zone.done(); zone.next()) { zone->gcNextGraphNode = rt->gcSweepingZones; rt->gcSweepingZones = zone; } /* Immediately sweep anything scheduled for background finalization. */ SweepBackgroundThings(rt, false); 3. Figure out why checking the mark bits didn't work.

(In reply to Bill McCloskey (:billm) from comment #4) > Comment on attachment 771716 [details] [diff] [review] > > 3. Figure out why checking the mark bits didn't work. IIRC, checking the mark bits did work. However, I'm convinced we should go the other route now in any case. Our perf problem on pdfjs is two-fold. First, we're blowing through store buffer space and constantly re-running the filter on already-filtered items. Simply removing one filter improves our perf from 48 to 4800. Secondly, we're blowing out the generic buffer and doing ~50 40-50ms fallback minor GCs. Some observations: (1) We're always going to run into a performance cliff of this magnitude if we don't switch to a growable store buffer. (2) The algorithmic performance of the store buffer is more important than the bit-shuffling performance to a shocking degree. (3) We can mitigate the OOM situation significantly by keeping a ballast and designing our data structure in such a way that incrementally growing does not need to reallocate the full buffer. I discovered the pdfjs bits by accident when doing cleanup in preparation for rewriting the store buffer's storage. I'll clean those up and attach them separately since they're a clear win in either case and are somewhat prerequisite to fixing this bug for real.

- This checks inRememberedSet on put and removes compactNotInSet. - This required moving some things into the header so that we can avoid going out-of-line for put. - This also fixes a bug where we set about-to-overflow /before/ deduping. This leads us to do a minor GC way too early, which leads us to not growing the store buffer, which ends up totally destroying our performance on earley boyer. - This also adds |explicit| to all of the internal classes. - This also moves two assertions in CheckMarkedThing above the use of those pointers. Sorry I omnibussed so much stuff into this patch. I didn't split it up as I went and tearing it all apart now would be a pain.

Comment on attachment 773385 [details] [diff] [review] v0: Eagerly apply inRememberedSet. Looks like this doesn't work for RelocatablePtr, which only popped up with deeper testing. Will fix and re-upload.

I started seeing other crashes after I rebased, but it was all bug 892287 and bug 891773. With those patches applied, this passes jit-tests --tbpl.

This implements a growable store buffer and uses it to allocate backing storage for GenericBuffer. The new StoreBuffer::Storage class is a singly linked list of StoreBuffer::Storage::Block. The default block size corresponds to typical windows allocation granularity, but this is a totally arbitrary choice. Append is trivial and has O(1) behavior. Iteration of the buffer is encapsulated by StoreBuffer::Storage::Range. Because GenericBuffer does not store a single, static type and because MonoTypeBuffer needs to be able to update the buffer in-place, Range has a somewhat verbose interface. In practice, it seems to work well, however. This passes all jsapi and jit tests with the block size in the patch and with a block size of 0x30, which heavily stresses the grow/truncate logic. It raises pdfjs perf from 48 to ~8000 and seems to stop the intermittent font-not-found errors there. I've tried to keep the patches in this series minimal to ease reviewing. If you want me to split up the prerequisite cleanup/eagerization patch so you can get to this faster, just let me know.

This makes MonoTypeBuffer use StoreBuffer::Storage for its backing buffer. I haven't tested the performance yet, since there is still a bunch of code to rip out of Nursery::collect, but this seems to eliminate the intermittent timeouts I get on GGC builds on Environment-find-06.

Now that there is not a fallback GC, the bool parameter to minorgc does setAboutToOverflow instead of setOverflowed.

This adds the tests from this bug, which now pass on top of this stack.

The first round of cleanup: 7 files changed, 14 insertions(+), 117 deletions(-)

We don't use this table concurrently, so share it.

Before ====== Richards: 13372 DeltaBlue: 15478 Crypto: 15492 RayTrace: 28757 EarleyBoyer: 11779 RegExp: 2079 Splay: 4961 NavierStokes: 20459 PdfJS: fail Mandreel: 4062 Gameboy: 17946 CodeLoad: 14823 Box2D: 5311 ---- Score (version 8): 10231 (w/out pdfjs) After ===== Richards: 13581 DeltaBlue: 15465 Crypto: 16738 RayTrace: 25604 EarleyBoyer: 11199 RegExp: 2367 Splay: 4645 NavierStokes: 20182 PdfJS: 7707 Mandreel: 4056 Gameboy: 21032 CodeLoad: 14625 Box2D: 5306 ---- Score (version 8): 10102 d8 == Richards: 13567 DeltaBlue: 17330 Crypto: 16223 RayTrace: 16132 EarleyBoyer: 26226 RegExp: 3898 Splay: 5289 NavierStokes: 17885 PdfJS: 13018 Mandreel: 13986 Gameboy: 13636 CodeLoad: 14719 Box2D: 13116 ---- Score (version 8): 12940 Tip === Richards: 14901 DeltaBlue: 12994 Crypto: 16577 RayTrace: 15730 EarleyBoyer: 14742 RegExp: 2695 Splay: 12427 NavierStokes: 20270 PdfJS: 7972 Mandreel: 4130 Gameboy: 21557 CodeLoad: 15291 Box2D: 5591 ---- Score (version 8): 10900 The main takeaway is that with pdfjs passing, this should get us into the right ballpark. Other than that, there is a small regression across the board and a big regression on Raytrace. This is a laptop, however, and my numbers have lots of noise. I'll look at the generated code next to ensure that clang isn't doing something boneheaded for the new |put| implementation.

Comment on attachment 776527 [details] [diff] [review] Part 5: remove overflow state; v0 At lunch David convinced me that we should keep this available as an option.

Comment on attachment 775046 [details] [diff] [review] Part 0: eagerly check remembered set membership; v2 Review of attachment 775046 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/StoreBuffer.h @@ +149,2 @@ > if (isFull()) { > compact(); I don't think this extra compact() is going to do any good now that you have one above it. @@ +316,5 @@ > bool operator!=(const SlotEdge &other) const { > return object != other.object || offset != other.offset || kind != other.kind; > } > > + HeapSlot *slotLocation() const; I'm kinda worried that this isn't inline. Can you see if it affects performance? @@ +407,5 @@ > RelocValueBufferSize + RelocCellBufferSize + > GenericBufferSize; > > public: > + explicit StoreBuffer(JSRuntime *rt, const Nursery &n) Can you call it nursery instead of n?

This passes the tests in this bug with the new if-statement in the heap tracing.

This better matches the new purpose and use of this flag.

You were right about the uninlining: it gives ~100pt drop. Inlining the inRememberedSet computation is a bit painful for slots, but it does get us back what we lost and ~50-100pts more, probably because of the isObject checks. This is a diff on top of Part0, after re-inlined SlotEdge.

Replace our custom allocator with LifoAlloc. This only saves ~50 lines, but LifoAlloc already exists, is well tested, and contains a couple of nice-to-have features when I didn't implement for Storage.

Comment on attachment 776479 [details] [diff] [review] Part 1: Implement a growable store buffer and rebase generic buffer; v0 Review of attachment 776479 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/StoreBuffer.h @@ +356,5 @@ > > GenericBuffer &operator=(const GenericBuffer& other) MOZ_DELETE; > > + void enable() { enabled_ = true; } > + void disable() { enabled_ = false; } You might want to clear when you disable.

Comment on attachment 776696 [details] [diff] [review] Part 7: Rename aboutToOverflow to needsGC; v0 Review of attachment 776696 [details] [diff] [review]: ----------------------------------------------------------------- I'd be okay renaming this to needsMinorGC, but needsGC is confusing.

Comment on attachment 776696 [details] [diff] [review] Part 7: Rename aboutToOverflow to needsGC; v0 Actually, I changed my mind. I think the current name is more descriptive than the others.

Comment on attachment 776771 [details] [diff] [review] Part 0.1: inline the in-remembered-set computation for slots; v0 Review of attachment 776771 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/StoreBuffer.h @@ +427,5 @@ > if (!edge.inRememberedSet(nursery)) > return; > bufferCell.put(edge); > } > + void putSlot(JSObject *obj, HeapSlot::Kind kind, uint32_t slot, void *target) { I think putSlot should take the target as a Value. Then it could do the isObject test itself. Then you could remove all the other isObject checks. ::: js/src/jsobj.cpp @@ +2080,5 @@ > * Trigger post barriers for fixed slots. JSObject bits are barriered > * below, in common with the other case. > */ > for (size_t i = 0; i < a->numFixedSlots(); ++i) { > + HeapSlot::writeBarrierPost(cx->runtime(), a, HeapSlot::Slot, i, &a->getSlotRef(i)); This isn't the right target. But if you change the target to a Value, you can just pass a->getSlot(i) here.

I'm getting really confused trying to review this stuff. Can you fold parts 1, 2, 5, and 8 into a single patch? Then I'll review that.

Yeah, I probably should have just done this rollup when switching to LifoAlloc.

Thanks Bill, that's much nicer. I'm not sure what I was thinking here.

Comment on attachment 779456 [details] [diff] [review] Part 0.1 - Inline inRememberedSet for putSlot - v1 Review of attachment 779456 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/Barrier-inl.h @@ +369,4 @@ > { > #ifdef JSGC_GENERATIONAL > + if (target.isObject()) > + obj->runtime()->gcStoreBuffer.putSlot(obj, kind, slot, &target.toObject()); Can you just call writeBarrierPost(obj->runtime(), obj, kind, slot, target) ? @@ +378,3 @@ > { > #ifdef JSGC_GENERATIONAL > + if (target.isObject()) It would be great to assert the following: JS_ASSERT_IF(kind == Slot, obj->getSlot(slot) == target); JS_ASSERT_IF(kind == Element, obj->getDenseElement(slot) == target); I think that should work.

Comment on attachment 779416 [details] [diff] [review] Part 1, 2, 5, and 8: rollup Review of attachment 779416 [details] [diff] [review]: ----------------------------------------------------------------- I'm confused about this patch. It makes the store buffer growable, but I don't see where we ever actually grow it. ::: js/src/ds/LifoAlloc.h @@ +87,5 @@ > void setNext(BumpChunk *succ) { next_ = succ; } > > size_t used() const { return bump - bumpBase(); } > > + void *start() const { return AlignPtr(bumpBase()); } I think bumpBase() should already be aligned. Can you just add an assert that it is, and get rid of the AlignPtr here? @@ +349,5 @@ > } > > + // Return true if the LifoAlloc does not currently contain any allocations. > + bool isEmpty() const { > + return !latest; I think you want !latest || !latest->used(). @@ +418,5 @@ > + position_(static_cast<char *>(alloc.first ? alloc.first->start() : NULL)) > + {} > + > + // Return true if there are no more bytes to enumerate. > + bool empty() { return !chunk_ || position_ >= alloc_->latest->mark(); } This doesn't seem right. position_ is within chunk_, which could be located after alloc_->latest in memory. I think you want this: !chunk || (chunk_ == alloc_->latest && position_ >= chunk_->mark()) @@ +436,5 @@ > + // Update the bytes at the current position with a new value. > + template <typename T> > + void updateFront(const T &t) { > + ensureSpaceAndAlignment(sizeof(T)); > + memmove(reinterpret_cast<void *>(position_), &const_cast<T &>(t), sizeof(T)); Are either of these casts needed? memmove takes a const void * for its second param. ::: js/src/gc/Nursery.cpp @@ +530,5 @@ > + * During sweeping, we have to be careful not to touch objects > + * which are not live: they may have already been swept. > + */ > + if (trc->runtime->gcIncrementalState == SWEEP && > + (!cell->isMarked() && !cell->arenaHeader()->allocatedDuringIncremental)) I thought we weren't doing this? Isn't that the whole point of the patch? ::: js/src/gc/StoreBuffer.h @@ +109,5 @@ > MonoTypeBuffer &operator=(const MonoTypeBuffer& other) MOZ_DELETE; > > + void enable() { enabled_ = true; } > + void disable() { enabled_ = false; clear(); } > + void clear() { storage_.used() ? storage_.releaseAll() : storage_.freeAll(); } What's the rationale for this? @@ +114,3 @@ > > + bool isAboutToOverflow() const { > + return !storage_.isEmpty() && storage_.availableInCurrentChunk() < MinAvailableSize; This test has a bad property: it could be that isAboutToOverflow() is true, but then you do some put() operations and it becomes false. I think we should add an sizeIsAbove(n) method to LifoAlloc. Then we could test here if storage_.sizeIsAbove(x), where x is (1<<16)-MinAvailableSize or something. @@ +134,3 @@ > return; > > + T *tp = storage_.newPod<T>(); Why aren't you just using new_? @@ +223,2 @@ > > + T *tp = storage_.newPod<T>(); Again, it seems like you could use new_ here.

(In reply to Bill McCloskey (:billm) from comment #31) > Comment on attachment 779416 [details] [diff] [review] > Part 1, 2, 5, and 8: rollup Thanks for the thorough review! > I'm confused about this patch. It makes the store buffer growable, but I > don't see where we ever actually grow it. It happens implicitly: |new_| calls |alloc| calls |getOrCreateChunk|. > ::: js/src/ds/LifoAlloc.h > @@ +87,5 @@ > > void setNext(BumpChunk *succ) { next_ = succ; } > > > > size_t used() const { return bump - bumpBase(); } > > > > + void *start() const { return AlignPtr(bumpBase()); } > > I think bumpBase() should already be aligned. Can you just add an assert > that it is, and get rid of the AlignPtr here? Good point. I was confused because I did not see an assertion that bumpBase is aligned, so I did not want to assume that it was. After looking harder, however, I think that the constuctor's assertion of |bump == AlignPtr(bump)| does already actually assert this property indirectly, so I'm just going to drop the AlignPtr here. > @@ +349,5 @@ > > } > > > > + // Return true if the LifoAlloc does not currently contain any allocations. > > + bool isEmpty() const { > > + return !latest; > > I think you want !latest || !latest->used(). Good catch! > @@ +418,5 @@ > > + position_(static_cast<char *>(alloc.first ? alloc.first->start() : NULL)) > > + {} > > + > > + // Return true if there are no more bytes to enumerate. > > + bool empty() { return !chunk_ || position_ >= alloc_->latest->mark(); } > > This doesn't seem right. position_ is within chunk_, which could be located > after alloc_->latest in memory. I think you want this: > > !chunk || (chunk_ == alloc_->latest && position_ >= chunk_->mark()) Good catch, again. > @@ +436,5 @@ > > + // Update the bytes at the current position with a new value. > > + template <typename T> > > + void updateFront(const T &t) { > > + ensureSpaceAndAlignment(sizeof(T)); > > + memmove(reinterpret_cast<void *>(position_), &const_cast<T &>(t), sizeof(T)); > > Are either of these casts needed? memmove takes a const void * for its > second param. Huh, seems not. > ::: js/src/gc/Nursery.cpp > @@ +530,5 @@ > > + * During sweeping, we have to be careful not to touch objects > > + * which are not live: they may have already been swept. > > + */ > > + if (trc->runtime->gcIncrementalState == SWEEP && > > + (!cell->isMarked() && !cell->arenaHeader()->allocatedDuringIncremental)) > > I thought we weren't doing this? Isn't that the whole point of the patch? Well, there are a couple intersecting concerns here. There are two ways to solve the specific crash in this bug: the above hunk, or the growable store buffer that makes up the rest of the patch. Since the growable store buffer also fixes our performance cliff -- manifesting heavily on pdf.js right now -- we want this for sure. Since you asked why the above did not work in your initial comment on the new code, it was not clear to me that you wanted to completely remove the fallback marker after all. Thinking about it some more, I think it would probably be possible to design a client script that could allocate the full heap, then make arbitrarily many store buffer entries, exposing users to a DOS. Not the end of the world, but I think it is probably worth keeping the fallback marking around, since it isn't much code and we have a couple of weeks of solid fuzzing against it now. > ::: js/src/gc/StoreBuffer.h > @@ +109,5 @@ > > MonoTypeBuffer &operator=(const MonoTypeBuffer& other) MOZ_DELETE; > > > > + void enable() { enabled_ = true; } > > + void disable() { enabled_ = false; clear(); } > > + void clear() { storage_.used() ? storage_.releaseAll() : storage_.freeAll(); } > > What's the rationale for this? If we have not used this store buffer for awhile (since the last minor GC), then return all of our memory -- including the first block -- and lazily re-acquire if we use it again. On the other hand, if we are using this store buffer actively, keep any memory that we've allocated as we're likely to need it again soon. I considered adding a function like freeAllButFirstBlock, but I wasn't sure if it would be worth the extra code. > @@ +114,3 @@ > > > > + bool isAboutToOverflow() const { > > + return !storage_.isEmpty() && storage_.availableInCurrentChunk() < MinAvailableSize; > > This test has a bad property: it could be that isAboutToOverflow() is true, > but then you do some put() operations and it becomes false. I think we > should add an sizeIsAbove(n) method to LifoAlloc. Then we could test here if > storage_.sizeIsAbove(x), where x is (1<<16)-MinAvailableSize or something. Actually, I don't think this is a bad property. The purpose of triggering a minor GC here is to keep us from needing to allocate another chunk (and risking not getting one). If the situation above does happen, we've already successfully allocated the memory, so we should now put off GCing until the nursery fills, if possible. > @@ +134,3 @@ > > return; > > > > + T *tp = storage_.newPod<T>(); > > Why aren't you just using new_? I managed to not see the JS_DECLARE_NEW_METHODS despite having searched for it and despite it being two lines above my changes. I must have typoed while searching for it or something.

> Thinking about it some more, I think it would probably be possible to design a client script > that could allocate the full heap, then make arbitrarily many store buffer entries, exposing > users to a DOS. Not the end of the world, but I think it is probably worth keeping the > fallback marking around, since it isn't much code and we have a couple of weeks of solid > fuzzing against it now. There are already plenty of ways to DOS the browser in precisely this way. For example, this patch aborts if it can't allocate a big enough hash table for the relocatable store buffer. I'd rather we have one more DOS patch if we can remove the tracing code. Despite the fuzzing it's gotten, it still may have bugs that we don't know about, and it adds complexity. > Actually, I don't think this is a bad property. The purpose of triggering a minor GC here is > to keep us from needing to allocate another chunk (and risking not getting one). If the > situation above does happen, we've already successfully allocated the memory, so we should now > put off GCing until the nursery fills, if possible. Looking at this again, it looks like we call setAboutToOverflow on the StoreBuffer the first time isAboutToOverflow goes to true, so we're going to do a minor GC no matter what. So I guess it doesn't matter what we do here. However, I still think it would be simpler if it worked the way I described. Also, what's the purpose of the isEmpty() check?

(In reply to Bill McCloskey (:billm) from comment #33) > > Thinking about it some more, I think it would probably be possible to design a client script > > that could allocate the full heap, then make arbitrarily many store buffer entries, exposing > > users to a DOS. Not the end of the world, but I think it is probably worth keeping the > > fallback marking around, since it isn't much code and we have a couple of weeks of solid > > fuzzing against it now. > > There are already plenty of ways to DOS the browser in precisely this way. > For example, this patch aborts if it can't allocate a big enough hash table > for the relocatable store buffer. I'd rather we have one more DOS patch if > we can remove the tracing code. Despite the fuzzing it's gotten, it still > may have bugs that we don't know about, and it adds complexity. Fair enough. I'll rip it all out. > > Actually, I don't think this is a bad property. The purpose of triggering a minor GC here is > > to keep us from needing to allocate another chunk (and risking not getting one). If the > > situation above does happen, we've already successfully allocated the memory, so we should now > > put off GCing until the nursery fills, if possible. > > Looking at this again, it looks like we call setAboutToOverflow on the > StoreBuffer the first time isAboutToOverflow goes to true, so we're going to > do a minor GC no matter what. So I guess it doesn't matter what we do here. > However, I still think it would be simpler if it worked the way I described. Great point. Now that this is more paged in, the reason I didn't implement sizeIsAbove before is that LifoAlloc does not track the total size. We would need to walk the chunk list to count the size or add a new field and appropriate machinery. An O(n) list traversal per store buffer insertion could potentially be very bad, but an add in that path would be somewhat bad all the time. Rather than benchmark I went with the more-complicated-but-trivially-performance-neutral solution that currently exists. > Also, what's the purpose of the isEmpty() check? If we haven't allocated into the LifoAlloc yet, the latest chunk will be NULL and availableInCurrentChunk will return 0. This seemed to be better than returning our default chunk size or a sentinel, as the LifoAlloc is free to make a bigger allocation. I could also easily see this method being useful for "can we fit this without allocation?" tests in other contexts.

Comment on attachment 779456 [details] [diff] [review] Part 0.1 - Inline inRememberedSet for putSlot - v1 Part 0 took us from 10000 to 15000 on octane, time to see what the rest of it will do.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a0e01422cb3 https://hg.mozilla.org/integration/mozilla-inbound/rev/eb89f19070ae

https://hg.mozilla.org/mozilla-central/rev/1a0e01422cb3 https://hg.mozilla.org/mozilla-central/rev/eb89f19070ae