1800263 - Avoid setting compartment's 'hasMarkedCells' flag outside root marking

Status: RESOLVED FIXED

(Core :: JavaScript: GC, task, P3)

Description

[Jon Coppeard (:jonco)]

To support ensuring we collect zones that we suspect are dead we set a flag on compartments when we certain GC things in that compartment (currently objects and scripts).

We only need to do this when marking roots because most children will be in the same compartment and we handle CCWs separately anyway. However currently we do this for all marking.

This is a slight issue for parallel marking because writing this flag can race where there is more than one thread marking. It is also sub-optimal since most marking time is spent outside of root marking.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1800263 - Part 1: Tidup, make MarkingState a private enum inside GCMarker r?sfink

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1800263 - Part 2: Add MarkingTracer as a member of GCMarker r?sfink

Previously GCMarker was a JSTracer. This separates out the impelmentation of
the tracer interface into another class.

The idea here is that we will change the tracer class used in different phases
of the GC. This will allow us to skip setting this compartment flag outside
root marker, and later allow us to select a different way of accessing the mark
bits for parallel marking.

Depends on D162393

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1800263 - Part 3: Add a root marking mode with its own tracer r?sfink

This adds a new marking mode for root marking and templates MarkingTracer on a
options bitmask. The GCMarker's tracer becomes a variant and we change the
constructed variant when the mode changes.

The templating allows us to make parts of the marking path conditional using if
constexpr on the marking options.

Depends on D162394

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1800263 - Part 4: Push marking options through markAndTraverse to avoid setting the flag for things we've already marked r?sfink

As a further optimisation we can move the flag update into
GCMarker::markAndTraverse so we don't set the flag for thing we've already
marked.

Depends on D162395

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 1800263 - Part 5: Tidyup GCMarker interface and make methods private where possible r?sfink

This class has become pretty complicated. I've tried to reorder the methods in
a way that groups related methods together and to make methods private where
possible.

Depends on D162396

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1bca4e782410
Part 1: Tidup, make MarkingState a private enum inside GCMarker r=sfink
https://hg.mozilla.org/integration/autoland/rev/969822745d98
Part 2: Add MarkingTracer as a member of GCMarker r=sfink
https://hg.mozilla.org/integration/autoland/rev/ba6cf596552d
Part 3: Add a root marking mode with its own tracer r=sfink
https://hg.mozilla.org/integration/autoland/rev/beeb51f05f78
Part 4: Push marking options through markAndTraverse to avoid setting the flag for things we've already marked r=sfink
https://hg.mozilla.org/integration/autoland/rev/eee37f3bfaba
Part 5: Tidyup GCMarker interface and make methods private where possible r=sfink

Comment 7

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/1bca4e782410
https://hg.mozilla.org/mozilla-central/rev/969822745d98
https://hg.mozilla.org/mozilla-central/rev/ba6cf596552d
https://hg.mozilla.org/mozilla-central/rev/beeb51f05f78
https://hg.mozilla.org/mozilla-central/rev/eee37f3bfaba