1734801 - Investigating making permanent GC things always black

Status: RESOLVED FIXED

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

Description

[Jon Coppeard (:jonco)]

It would be great if we could arrange for permanent GC things (that may be shared between runtimes) to always be black and never have their mark bits cleared. That way, we could remove the checks for permanent GC things that often occur next to a check for whether the thing is marked, since we have to do the mark check anyway.

We already did this for permanent atoms in bug 1669669. We'd need to do the same for well-known symbols. I'm not sure whether other permanent things in the atoms zone (e.g. JIT stubs) can be shared.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1734801 - Part 1: Freeze well-known symbols on engine initialization r?sfink

In the same way as we freeze permanent atoms on startup we can freeze
well-known sybols too. This has the following advantages

• we don't have to mark them
• we don't have to sweep them
• we can now assume that permanent and potentially shared things are always marked

Depends on D129287

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1734801 - Part 2: Remove checks for things owned by other runtimes when marking r?sfink

Anything owned by another runtime will always be marked black so we can fold
this check into the existing check for whether a thing is marked.

Depends on D129288

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1734801 - Part 3: Make zone GC state atomic to make it safe to observe from other runtimes r?sfink

Some places check a thing's zone's GC state before checking its mark bits. To
make this safe the GC state needs to be atomic. The outcome of these checks is
independent of the zone GC state observed.

Depends on D129289

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1734801 - Part 4: Remove checks for things owned by other runtimes in IsMarked, IsAboutToBeFinalized and SweepingTracer r?sfink

These checks are replaced by assertions that the thing is black if it's owned
by another runtime, and hence we will return the right result.

Depends on D129290

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cea53a626528
Part 1: Freeze well-known symbols on engine initialization r=sfink
https://hg.mozilla.org/integration/autoland/rev/dcc7bbf8b0da
Part 2: Remove checks for things owned by other runtimes when marking r=sfink
https://hg.mozilla.org/integration/autoland/rev/068545b6f978
Part 3: Make zone GC state atomic to make it safe to observe from other runtimes r=sfink
https://hg.mozilla.org/integration/autoland/rev/0d7c0ebe0e3c
Part 4: Remove checks for things owned by other runtimes in IsMarked, IsAboutToBeFinalized and SweepingTracer r=sfink

Comment 6

[Cristian Tuns]

Backed out for causing multiple failures.

• Backout link
• Push with failures
• Mochitest Failure Log
• Reftest Failure Log

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4df5a0d025f9
Part 1: Freeze well-known symbols on engine initialization r=sfink
https://hg.mozilla.org/integration/autoland/rev/85f439d1507d
Part 2: Remove checks for things owned by other runtimes when marking r=sfink
https://hg.mozilla.org/integration/autoland/rev/5ccc1dafb61e
Part 3: Make zone GC state atomic to make it safe to observe from other runtimes r=sfink
https://hg.mozilla.org/integration/autoland/rev/cd76a8234de6
Part 4: Remove checks for things owned by other runtimes in IsMarked, IsAboutToBeFinalized and SweepingTracer r=sfink

Comment 8

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/4df5a0d025f9
https://hg.mozilla.org/mozilla-central/rev/85f439d1507d
https://hg.mozilla.org/mozilla-central/rev/5ccc1dafb61e
https://hg.mozilla.org/mozilla-central/rev/cd76a8234de6