RFR: 8327097: GenShen: Align PLAB sizes down rather than up [v6]

[Y. Srinivas Ramakrishna]

On Wed, 20 Mar 2024 23:35:52 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add comments to clarify behavior of retire_plab
>
> src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp line 52:
> 
>> 50:   // from alignment_in_words to determine padding required to next alignment point.
>> 51: 
>> 52:   HeapWord* aligned_obj = (HeapWord*) align_up(orig_top, alignment_in_bytes);
> 
> I think this method's declaration `allocate_aligned(...)` in the `.hpp` uses `alignment_in_words` as the name of the second parm. Similarly the comment in the header. Those should be corrected too to say `alignment_in_bytes`.

We do know that this is alignment must be a mutiple of HeapWordSize (as demonstrated by the integer division you do further above). I'd just assert for the sake of peace of mind from an API perspective that this is always so by:

assert(is_aligned(alignment_in_bytes, HeapWordSize), "Expected heap word alignment");

> src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp line 54:
> 
>> 52:   HeapWord* aligned_obj = (HeapWord*) align_up(orig_top, alignment_in_bytes);
>> 53:   size_t pad_words = aligned_obj - orig_top;
>> 54:   if (pad_words > 0) {
> 
> Can probably just && and fold together the two if tests.

if (pad_words > 0 && pad_words < ShenandoahHeap::min_fill_size()) {
   // shift allocation right by another unit of alignment to accommodate pad for filler
   pad_words += alignment_in_words;
   aligned_obj += alignment_in_words;
}

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

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