In the interpreter, using initializedLength is fine because setElement checks the capacity every time. The JITs, however, assume that the array capacity will not change between the initarray and initializing setelem opcodes. This seems like a reasonable assumption, so we should just not shrink below the existing capacity. This also fixes a small issue where we always choose the thingKind based on the initializedLength without regard to hasDynamicElements. This would lead us to over-allocating: e.g. starting with FINALIZE_OBJECT4_BACKGROUND in the nursery, filled in 6 elements (allocating a new backing store to replace the inline elements), tenuring with FINALIZE_OBJECT8_BACKGROUND, then re-using the dynamic elements. This would leave the 8 inline elements in the heap totally unused.

Comment on attachment 750540 [details] [diff] [review] v0 Review of attachment 750540 [details] [diff] [review]: ----------------------------------------------------------------- > In the interpreter, using initializedLength is fine because setElement checks the capacity > every time. The JITs, however, assume that the array capacity will not change between the > initarray and initializing setelem opcodes. This seems like a reasonable assumption, so > we should just not shrink below the existing capacity. That sucks, but I guess I agree. > This also fixes a small issue where we always choose the thingKind based on the > initializedLength without regard to hasDynamicElements. This would lead us to > over-allocating: e.g. starting with FINALIZE_OBJECT4_BACKGROUND in the nursery, > filled in 6 elements (allocating a new backing store to replace the inline > elements), tenuring with FINALIZE_OBJECT8_BACKGROUND, then re-using the dynamic > elements. This would leave the 8 inline elements in the heap totally unused. It seems like we ought to fix this. If we have enough fixed slots, we should use them. That could be a follow-up, though. ::: js/src/gc/Nursery.cpp @@ +228,5 @@ > + /* Use minimal size object if we have external elements. */ > + if (!obj->hasFixedElements()) > + return FINALIZE_OBJECT0_BACKGROUND; > + > + size_t nelements = obj->getElementsHeader()->capacity; Just use obj->getDenseCapacity() and then you won't need to make this a member of Nursery.

Green try run at: https://tbpl.mozilla.org/?tree=Try&rev=17dcf510b7bb https://hg.mozilla.org/integration/mozilla-inbound/rev/89a472c35979

Reluctantly backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/b54ce66659aa - one of the four things in that push, despite having been green on try, picked up causing failures in Android 2.2 reftest-1, reftest-2, and reftest-4 by the time it landed. Since I always blame the need to clobber when it's Android, I clobbered and retriggered on your push, and they still failed.

I'm going to reland this now because this patch only affects ggc enabled builds: https://hg.mozilla.org/integration/mozilla-inbound/rev/a8ec45bbe41a I trychose the wrong tests apparently, but here is a green try anyway: https://tbpl.mozilla.org/?tree=Try&rev=a704f20883bf