1867220 - Weak map marking barriers can be removed if we change the ephemeron edges

Status: RESOLVED FIXED

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

Description

[Jon Coppeard (:jonco)]

While working on bug 1865611 I found that we could simplify weak map marking by removing the incremental marking barriers if we changed the implicit edges (ephemerons) we create for weakmap entries.

Currently when there is a weak map key with a delegate we create an ephemeron edge from the delegate to the key and from the delegate to the value. This means that if the association between the key and the delegate is removed we need a barrier to ensure the value gets marked if the key gets marked (the key originally contains a strong reference to the delegate).

Instead, if we create an edge from the delegate to the key and from the key to the value it seems we don't require this barrier. Marking the key will mark the value. Marking the delegate will mark the value because it marks the key first. If the association between the key and the delegate is removed we will conservatively still mark key and value if the delegate gets marked, as the barrier does now.

I may well be missing something here but I tested this and it and it seems to work.

Comment 1

[Jon Coppeard (:jonco)]

(In reply to Jon Coppeard (:jonco) from comment #0)

If the association between the key and the delegate is removed we will conservatively still mark key and value if the delegate gets marked

In fact we will always mark the key and value because the delegate will get marked by the standard pre barrier when the key to delegate pointer is overwritten, and this will end up marking the key and value via the ephemerons. This is the same effect as the current barrier.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1867220 - Part 1: Remove redundant delegate to value ephemeron edge for weakmap entries r?sfink

This changes weak map marking to create edges from the delegate to the key and
from the key to the value instead of from the delegate to the key and from the
delegate to the value.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1867220 - Part 2: Simplify weak map marking barriers r?sfink

The barrier for removing a delegate from a weak map key is removed. The
delegate will get marked via the standard pre barrier when it is removed. The
key must be live and so will get marked also. The value will get marked via the
ephemeron edges from both the above.

The barrier for addinga delegate to a weak map key is removed. The key and
delegate must be live so will get marked. The value will get marked via the
ephemeron edge from the key.

Depends on D194971

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1867220 - Part 3: Simpify GCMarker::markImplicitEdges r?sfink

This removes the helper method and uses a constexpr if for.

Also the color of the thing we're marking must be the same as the current mark
color so we can use that.

Depends on D194972

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 1867220 - Part 4: Remove empty edge vectors from the ephemerons table r?sfink

We remove elements from the ephemerons vector if they cannot trigger any
further marking (their targets are already marked black). If the vector becomes
empty we can also remove it from the map to make future lookups more efficient.

I had to add a remove() method to OrderedHashMap for this which took the
pointer previously returned by get(). This isn't very elegant as it has to cast
to get back the internal Data pointer.

Depends on D194973

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d53f51849c4
Part 1: Remove redundant delegate to value ephemeron edge for weakmap entries r=sfink
https://hg.mozilla.org/integration/autoland/rev/335affd358af
Part 2: Simplify weak map marking barriers r=sfink
https://hg.mozilla.org/integration/autoland/rev/e9012c25bbf6
Part 3: Simpify GCMarker::markImplicitEdges r=sfink
https://hg.mozilla.org/integration/autoland/rev/a4f88bf8fc68
Part 4: Remove empty edge vectors from the ephemerons table r=sfink

Comment 7

[Aron Cseh]

https://hg.mozilla.org/mozilla-central/rev/7d53f51849c4
https://hg.mozilla.org/mozilla-central/rev/335affd358af
https://hg.mozilla.org/mozilla-central/rev/e9012c25bbf6
https://hg.mozilla.org/mozilla-central/rev/a4f88bf8fc68