945846 - GenerationalGC: Store buffer reentrancy assertion fails in browser builds

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

The work to stop barriers firing minor collection needs extending to the browser.  It looks like there are only two cases, one in ctypes and one in XPConnect.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: fix-ctypes-barriers

This is slightly unpleasant because we have to make a duplicate FieldInfo structure without the Heap<JSObject*>.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: fix-xpconnect-barriers

And here's a patch to fix the case in XPConnect.

Comment 3

[Bobby Holley (:bholley)]

Comment on attachment 8341853 [details] [diff] [review]
fix-xpconnect-barriers

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

I have no idea what this does. Andrew or one of the GC guys can review it.

Comment 4

[Andrew McCreight [:mccr8]]

I vote for Terrence...

Comment 5

[Terrence Cole [:terrence]]

Comment on attachment 8341851 [details] [diff] [review]
fix-ctypes-barriers

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

Great! Sorry about the delay; I thought I hit submit yesterday. Firefox has been lagging horribly for me lately, so I'm going to blame that. r=me

::: js/src/ctypes/CTypes.h
@@ +224,5 @@
> +  JSObject*           mType;    // CType of the field
> +  size_t              mIndex;   // index of the field in the struct (first is 0)
> +  size_t              mOffset;  // offset of the field in the struct, in bytes
> +};
> +JS_STATIC_ASSERT(sizeof(UnbarrieredFieldInfo) == sizeof(FieldInfo));

We are using the C++11 static_assert now. Check gc/Heap.h for examples.

Comment 6

[Terrence Cole [:terrence]]

Comment on attachment 8341853 [details] [diff] [review]
fix-xpconnect-barriers

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

r=me

::: js/xpconnect/src/XPCMaps.h
@@ +694,5 @@
>       * This function is called during minor GCs for each key in the HashMap that
>       * has been moved.
>       */
>      static void KeyMarkCallback(JSTracer *trc, void *k, void *d) {
> +        typedef js::HashMap<JSObject *, JSObject *, js::PointerHasher<JSObject *, 3>,

It would be nice to have a comment here explaining this for the casual reader.

Comment 7

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/27dd92819372
https://hg.mozilla.org/integration/mozilla-inbound/rev/66bd8afba725

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/27dd92819372
https://hg.mozilla.org/mozilla-central/rev/66bd8afba725