RFR: 8331609: GenShen: Refactor generational mode allocations [v2]

[William Kemper]

On Mon, 6 May 2024 20:13:00 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> William Kemper has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix zero build, warnings in test
>
> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 1129:
> 
>> 1127:   if (result != nullptr && req.is_old()) {
>> 1128:     old_generation()->configure_plab_for_current_thread(req);
>> 1129:     // TODO: Is this only necessary for 'shared' allocations?
> 
> I believe this is only necessary for shared allocs.  I'm supportive of making this change as part of PR, though it might be safer to do in separate PR.
> 
> (If this is a PLAB allocation, the objects allocated within the PLAB will be independently registered when they are allocated.)

I'm going to test making this change as part of this PR.

> src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp line 229:
> 
>> 227: 
>> 228:   const size_t requested_bytes = req.size() * HeapWordSize;
>> 229:   if (can_promote(requested_bytes)) {
> 
> Even though we don't check whether this req.is_promotion(), we know:
>   if there's room to promote requested_bytes, there is also room to evacuate requested_bytes
> 
> A comment could be helpful here.

Done

> src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.hpp line 125:
> 
>> 123:   }
>> 124: 
>> 125:   // Test if there is enough memory available in the old generation to allocate a new PLAB.
> 
> Comment says this tests for memory to allocate a PLAB, in which case maybe we should rename as can_allocate_PLAB().  However implementation of can_allocate() checks whether req.type() == _alloc_plab.  Seems something's not right here.

Fixed the comment.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/427#discussion_r1593101415
PR Review Comment: https://git.openjdk.org/shenandoah/pull/427#discussion_r1593098077
PR Review Comment: https://git.openjdk.org/shenandoah/pull/427#discussion_r1593100240