2025288 - Refactor barriered pointer wrappers to simplify repetitive code

Status: RESOLVED FIXED

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

Description

[Jon Coppeard (:jonco)]

gc/Barrier.h provides wrapper classes to add barriers to GC thing pointers. There are several barriers supported (read, pre-write and post-write) we supply a wrapper class for many (most?) of the possible combinations. This results in a lot of repetitive code.

This bug aims to fix that by refactoring most of the implementation into a single class that is parameterized with a bitfield of options and using compile time conditionals to trigger the appropriate barriers.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 2025288 - Part 1: Abstract access to BarrieredBase::value r?sfink

This makes the |value| that stores the wrapped pointer in BarrieredBase
private, and provides unbarriered accessors for sublasses to use.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 2025288 - Part 2: Remove BarrieredSetPair r?sfink

This removes an optimization for setting two HeapPtrs at the same time. It
skips a GC state check on the zone. It's only used in one place and I'd be
surprised if this made a measurable difference to anything.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 2025288 - Part 3: Add a class to implement barriered pointer wrappers r?sfink

This adds the BarrieredPtrImpl class template which contains logic for all barriers
and conditionally selects which barriers to apply at compile time.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 2025288 - Part 4: Reimplement most barriers in terms of BarrieredPtrImpl class r?sfink

This ended up adding the gray things check to more paths hence the addition of
AutoTouchingGrayThings in RemapDeadWarpper.

This also made it so that we need to use BarrieredBase rather than
WriteBarriered as the base pointer type in some tracing methods.

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 2025288 - Part 5: Remove WriteBarriered base class that only has a single use r?sfink

This is now only used for HeapSlot, so we can merge the two.

Comment 6

[Jon Coppeard (:jonco)]

Attachment: Bug 2025288 - Part 6: Define all comparison ops in one place r?sfink

This also uses a macro to reduce repetitive code.

Comment 7

[Jon Coppeard (:jonco)]

Attachment: Bug 2025288 - Part 7: Use macros to define hashing related specializations r?sfink

Comment 8

[Jon Coppeard (:jonco)]

Attachment: Bug 2025288 - Part 8: Use macros to define RemoveBarrier specializations r?sfink

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/354dcbee0147
https://hg.mozilla.org/integration/autoland/rev/3a9c4fe19229
Part 1: Abstract access to BarrieredBase::value r=sfink
https://github.com/mozilla-firefox/firefox/commit/d5e620c97174
https://hg.mozilla.org/integration/autoland/rev/9e5969b4e161
Part 2: Remove BarrieredSetPair r=sfink
https://github.com/mozilla-firefox/firefox/commit/bde590b9a69b
https://hg.mozilla.org/integration/autoland/rev/8568c4319af8
Part 3: Add a class to implement barriered pointer wrappers r=sfink
https://github.com/mozilla-firefox/firefox/commit/36a99b599d39
https://hg.mozilla.org/integration/autoland/rev/e1fe8c52bfea
Part 4: Reimplement most barriers in terms of BarrieredPtrImpl class r=sfink
https://github.com/mozilla-firefox/firefox/commit/52becbbc6765
https://hg.mozilla.org/integration/autoland/rev/b11039655f6b
Part 5: Remove WriteBarriered base class that only has a single use r=sfink
https://github.com/mozilla-firefox/firefox/commit/bbde1da07cce
https://hg.mozilla.org/integration/autoland/rev/1e42a8bdc545
Part 6: Define all comparison ops in one place r=sfink
https://github.com/mozilla-firefox/firefox/commit/0b49be4c4a50
https://hg.mozilla.org/integration/autoland/rev/3a0930d84a7e
Part 7: Use macros to define hashing related specializations r=sfink
https://github.com/mozilla-firefox/firefox/commit/61a7fee6fb50
https://hg.mozilla.org/integration/autoland/rev/3618de534588
Part 8: Use macros to define RemoveBarrier specializations r=sfink
https://github.com/mozilla-firefox/firefox/commit/ee00bd5fc0e5
https://hg.mozilla.org/integration/autoland/rev/cf03e38ca715
apply code formatting via Lando

Comment 10

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/3a9c4fe19229
https://hg.mozilla.org/mozilla-central/rev/9e5969b4e161
https://hg.mozilla.org/mozilla-central/rev/8568c4319af8
https://hg.mozilla.org/mozilla-central/rev/e1fe8c52bfea
https://hg.mozilla.org/mozilla-central/rev/b11039655f6b
https://hg.mozilla.org/mozilla-central/rev/1e42a8bdc545
https://hg.mozilla.org/mozilla-central/rev/3a0930d84a7e
https://hg.mozilla.org/mozilla-central/rev/3618de534588
https://hg.mozilla.org/mozilla-central/rev/cf03e38ca715