1447385 - Store whole cell buffer contents outside the nursery

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement, P2)

Description

[Jon Coppeard (:jonco)]

As mentioned in bug 1447074, the whole cell store buffer is a possible culprit for bit flip crashes.  tcampbell made the suggestion of storing this data outside the nursery so that access to stale data couldn't overwrite JS objects.  We should give this its own LIFO allocated storage.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1447385-whole-cell-sb-lifo

Patch to use a LifoAlloc to store the whole cell store buffer data.

We used to store this in the nursery which meant we very rarely triggered minor GC on account of this filling up.  With its own buffer this will happen more often so I made the threshold much larger than that for the generic buffer.

I gave octane a quick run and this doesn't trigger that many more minor GCs, but it's possible that this will regress something.

Comment 2

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8960699 [details] [diff] [review]
bug1447385-whole-cell-sb-lifo

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

::: js/src/gc/StoreBuffer.h
@@ +186,5 @@
> +
> +      private:
> +        ArenaCellSet* allocateCellSet(Arena* arena);
> +
> +        WholeCellBuffer& operator=(const WholeCellBuffer& other) = delete;

I'm always fuzzy on these things, but if you're going to delete operator=, shouldn't you delete the copy constructor too?

@@ +522,5 @@
>      // Construct the empty sentinel object.
>      ArenaCellSet();
>  
>    public:
> +    explicit ArenaCellSet(Arena* arena, ArenaCellSet* next);

explicit is unnecessary here, not that it matters.

Comment 3

[Jon Coppeard (:jonco)]

(In reply to Steve Fink [:sfink] [:s:] from comment #2)

> I'm always fuzzy on these things, but if you're going to delete operator=,
> shouldn't you delete the copy constructor too?

Good point.  I'll fix the others in that file too.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea6bc3f6d52a
Store whole cell store buffer data outside the nursery r=sfink

Comment 5

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/ea6bc3f6d52a