RFR: 8285984: G1: Use standard idiom for inlined payload in G1MonotonicArena::Segment [v2]

Axel Boldt-Christmas aboldtch at openjdk.org
Fri Sep 12 13:42:17 UTC 2025


On Fri, 12 Sep 2025 13:37:36 GMT, Leo Korinth <lkorinth at openjdk.org> wrote:

>> This change remove the old padding (extremely big, especially on x86-64), and replaces it with a guaranteed 8 byte alignment. It also removes the `_bottom` field and instead adds a `payload` method. I statically assert that Segment is 8 byte aligned. I then runtime assert that Segment is constructed on 8 byte aligned memory. I also assert that the Slot alignment is less than or equal to the Segment alignment.
>> 
>> I am running tier 1-3 at the moment. Do you think I should run some specific performance test on this?
>
> Leo Korinth has updated the pull request incrementally with one additional commit since the last revision:
> 
>   may fail on 32 bit machines, added explicit alignas

This looks good. Just wonder if we want some comment w.r.t. the (Max)Alignment variable explaining its purpose.

src/hotspot/share/gc/g1/g1MonotonicArena.hpp line 113:

> 111: };
> 112: 
> 113: static constexpr uint SegmentAlignment = 8;

Maybe this should be called `SegmentMaxAlignment`, or `SegmentPayloadMaxAlignment`.

And have a comment describing that we align the Segment class to this alignment to guarantee that the payload is aligned to all powers of 2 less than or equal to this value.

src/hotspot/share/gc/g1/g1MonotonicArena.hpp line 239:

> 237:     assert(_max_num_slots > 0, "Must be");
> 238:     assert(_slot_alignment > 0, "Must be");
> 239:     assert(_slot_alignment <= SegmentAlignment, "Must be");

Not sure if it should be clarified that this assert is sufficient because we only ever use allow alignments which are a power of two.

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

Marked as reviewed by aboldtch (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27258#pullrequestreview-3216748107
PR Review Comment: https://git.openjdk.org/jdk/pull/27258#discussion_r2344290116
PR Review Comment: https://git.openjdk.org/jdk/pull/27258#discussion_r2344282983


More information about the hotspot-gc-dev mailing list