RFR: 8327097: GenShen: Align PLAB sizes down rather than up

Y. Srinivas Ramakrishna ysr at openjdk.org
Fri Mar 8 00:53:14 UTC 2024


On Fri, 8 Mar 2024 00:30:55 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 1114:
>> 
>>> 1112:   future_size = MIN2(future_size, PLAB::max_size());
>>> 1113:   future_size = MAX2(future_size, PLAB::min_size());
>>> 1114:   future_size = align_down(future_size, CardTable::card_size_in_words());
>> 
>> Can one use cardsize-aligned min and max sizes (taking whetever PLAB gives us, may be by using a ShenandoahPLAB subclass). We then first align future_size up wrt card size, then floor/clamp it with the already cardsize-aligned min and max?
>> 
>> In particular if cur_size starts off a multiple of cardsize then any doubling keeps it a multiple of cardsize, and you can more simply perhaps just assert that it's always  a cardsize-multiple.
>
> We also need a comment somewhere stating why we need these to be card-aligned.
> 
> I wonder also if this is entirely correct in itself, unless the shared allocation itself also always leaves us with a card-aligned top from which these PLABs are allocated.
> 
> In particular, it would be good to assert that `allocate_new_plab()` always gives us back a card-aligned start address.
> 
> Would be good to also use `align_up` or `align_down` in `allocate_new_plab()` as well. Indeed, it might be a good idea to do so in any code in Shenandoah where we are unnecessarily rolling our own custom alignment code.

There's also a couple of assertion checks in `ShenandoahFreeSet::allocate_aligned_plab()` that use the modulus operation, which should probably use `is_aligned()` instead.

Would be good to clean those up as well while we are taking care of this.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/401#discussion_r1517028913


More information about the shenandoah-dev mailing list