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

Kelvin Nilsen kdnilsen at openjdk.org
Fri Mar 22 17:46:36 UTC 2024


On Wed, 20 Mar 2024 07:28:54 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:
>> 
>>   Fix up a few other places where PLAB min size or current size gets set
>>   
>>   This may explain how unaligned sizes were being detected by existing
>>   assertions.
>
> src/hotspot/share/gc/shenandoah/shenandoahArguments.cpp line 222:
> 
>> 220:   if (strcmp(ShenandoahGCMode, "generational") == 0) {
>> 221:     ShenandoahGenerationalHeap* heap = new ShenandoahGenerationalHeap(new ShenandoahCollectorPolicy());
>> 222:     CollectedHeap* result = heap;
> 
> Wonder if it might read cleaner if we reversed the if-condition and returned the ShenandoahHeap from inside the if, then did the more cumbersome GenShen thing after.
> 
> Also, did you need to save in CollectedHeap* result to return it?
> Simpler may be to do a
> 
> 
>       ShenandoahGenerationalHeap* const gen_heap = new ...; 
>       ... set min and max to card-aligned values ...
>       return gen_heap;
> 
> Does that cause a compiler error?

Thanks.  Making this change.

> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 1165:
> 
>> 1163: #endif // ASSERT
>> 1164:     }
>> 1165:     assert (actual_size % CardTable::card_size_in_words() == 0,
> 
> Can use `is_aligned(actual_size, CardTable::...)` inside assert, avoiding `%`.

Done.  Thanks.

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

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


More information about the shenandoah-dev mailing list