1986862 - Refactor FinalizationRegistry implementation by removing global record set

Status: RESOLVED FIXED

(Core :: JavaScript: GC, task)

Description

[Jon Coppeard (:jonco)]

Following on from recent work in bug 1984091 we can simplify the FinalizationRegistry implementation by removing the set of finalization records associated with each global.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1986862 - Refactor finalization registries to remove global data r?sfink

This simplifies the finalization registry implementation by removing
FinalizationRegistryGlobalData, a data structure that currently holds a set of
all registered FinalizationRecordObjects for each global. This is replaced with
a vector held by the finalization registy itself.

This means that the records are held alive by the registry they are registered
with. This simplifies sweeping because we don't need to check for unregistered
records as we now remove them when they are unregistered.

I updated the diagram of how this all works, which previously didn't include
the global data. I also updated the comments here which I missed in bug
1984091.

This has also restore the original behaviour of test gc/bug-1825936.js which
now no longer catches an error thrown in a separate global after nuking CCWs. I
don't exactly understand this but since this was the original behaviour before
the recent changes I assume it's OK.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/af2576e4e50c
https://hg.mozilla.org/integration/autoland/rev/c75dd11ecb3a
Refactor finalization registries to remove global data r=sfink

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/812f04c365aa
https://hg.mozilla.org/integration/autoland/rev/af5d86afcfd7
Revert "Bug 1986862 - Refactor finalization registries to remove global data r=sfink" for causing dt assertion failures @ gray marking

Comment 4

[Sandor Molnar[:smolnar]]

Backed out for causing dt assertion failures @ gray marking

Backout link: https://hg.mozilla.org/integration/autoland/rev/af5d86afcfd76a7c5fcea26b63777850f9b8367b

Push with failures

Failure log -> Assertion failure: !tc->isMarkedGray(), at /builds/worker/checkouts/gecko/js/src/gc/GC.cpp

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/34f6ff62ef05
https://hg.mozilla.org/integration/autoland/rev/2fcef31c7581
Refactor finalization registries to remove global data r=sfink

Comment 6

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/2fcef31c7581