RFR: 8334060: Implementation of Late Barrier Expansion for G1 [v27]

[Roberto Castañeda Lozano]

On Fri, 4 Oct 2024 09:17:47 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/c2/g1BarrierSetC2.cpp line 335:
>> 
>>> 333:       assert(!use_ReduceInitialCardMarks(),
>>> 334:              "post-barriers are only needed for tightly-coupled initialization stores when ReduceInitialCardMarks is disabled");
>>> 335:       access.set_barrier_data(access.barrier_data() ^ G1C2BarrierPre);
>> 
>> I have been looking at this code after integration, and I wonder if `^` is really correct here? Was the intend to remove `G1C2BarrierPre` from the barrier data? What happens if `get_store_barrier` does not actually set it? Do we flip the bit back?
>
> Yes, the intend (and actual effect) is to remove `G1C2BarrierPre` from the barrier data. Using an XOR (`^`) is correct because at that program point `G1C2BarrierPre` is guaranteed to be set. This is because an `access` corresponding to a tightly-coupled initialization store is always of type `C2OptAccess`, hence `!access.is_parse_access()` and `get_store_barrier(access)` trivially returns `G1C2BarrierPre | G1C2BarrierPost`. Having said this, it would be clearly less convoluted to simply clear `G1C2BarrierPre` instead of flipping it. I will file a RFE, thanks.
> 
> As a side note, this complexity is necessary to handle `!ReduceInitialCardMarks`. I keep wondering if the benefit of being able to disable `ReduceInitialCardMarks` [1,2,3] is worth the significant complexity required in the GC-C2 interface to deal with it.
> 
> [1] https://docs.oracle.com/en/java/javase/23/gctuning/garbage-first-garbage-collector-tuning.html
> [2] https://bugs.openjdk.org/browse/JDK-8166899
> [3] https://bugs.openjdk.org/browse/JDK-8167077

Reported here: [JDK-8341525](https://bugs.openjdk.org/browse/JDK-8341525).

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19746#discussion_r1787448241