RFR: 8331609: GenShen: Refactor generational mode allocations

[Kelvin Nilsen]

On Thu, 2 May 2024 23:21:46 GMT, William Kemper <wkemper at openjdk.org> wrote:

> * The code for resizing and retrying young allocation requests according to what's available in the young generation has been removed. This code became redundant after the free set was rewritten and began providing the same functionality.
> * An unused field and its attendant methods were removed from `ShenandoahThreadLocalData`.
> * The code for managing `plab` expenditures and configuring other plab behavior has been moved from `ShenandoahHeap` into `ShenandoahOldGeneration`.

Changes requested by kdnilsen (Committer).

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.)

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.

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp line 237:

> 235: 
> 236:   if (req.type() == ShenandoahAllocRequest::_alloc_plab) {
> 237:     // The promotion reserve is exhausted. Check if we still have room for evacuations.

We don't KNOW promotion reserve is entirely exhausted, just that what remains in promotion reserve is less than requested bytes.

src/hotspot/share/gc/shenandoah/shenandoahOldGeneration.cpp line 243:

> 241:   }
> 242: 
> 243:   // This is a shared allocation promotion request. However, we do not have room for any

How do we know this is NOT a shared allocation evacuation request?  Is there a precondition on the method call?

Implementation below looks we're not sure this is a promotion.  We still need to check.

I think what we're saying here is:
  if req.is_promotion();   return false because we already checked above, and found that can_promote(requested_bytes) to be false.
  
  So what if req is not promotion?  wouldn't we want to confirm that get_evacuation_reserve() > requested_bytes?

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.

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

PR Review: https://git.openjdk.org/shenandoah/pull/427#pullrequestreview-2041548822
PR Review Comment: https://git.openjdk.org/shenandoah/pull/427#discussion_r1591505837
PR Review Comment: https://git.openjdk.org/shenandoah/pull/427#discussion_r1591477363
PR Review Comment: https://git.openjdk.org/shenandoah/pull/427#discussion_r1591524664
PR Review Comment: https://git.openjdk.org/shenandoah/pull/427#discussion_r1591535274
PR Review Comment: https://git.openjdk.org/shenandoah/pull/427#discussion_r1591481761