RFR: 8358586: ZGC: Combine ZAllocator and ZObjectAllocator [v2]

[Axel Boldt-Christmas]

On Wed, 11 Jun 2025 09:46:21 GMT, Joel Sikström <jsikstro at openjdk.org> wrote:

>> Hello,
>> 
>> The main purpose of this RFE is to merge the ZAllocator class into ZObjectAllocator. After [JDK-8353184](https://bugs.openjdk.org/browse/JDK-8353184), ZAllocator has essentially become a mirror of ZObjectAllocator, with a few tweaks for different behavior for eden or relocation allocations. The goal is to make the code a bit easier to follow and generalise the different paths for eden and relocation allocations to a shared path.
>> 
>> The storage of the static allocators for each ZPageAge has been moved from ZHeap into ZObjectAllocator, and initilization is done via ZInitialize (calling into `ZObjectAllocator::initialize()`) instead of in the ZHeap constructor.
>> 
>> Instead of storing eden and relocation allocators separately, they are now in a single array, which makes iterating over the allocators more straightforward (when retiring pages for example). I have opted to keep the getter for the eden allocator (`ZObjectAllocator::eden()`), but it can be swapped to `ZObjectAllocator::allocator(ZPageAge::eden)` if we prefer that instead.
>> 
>> `ZObjectAllocator::{alloc_object, alloc_object_for_relocation}` have been merged into a single `ZObjectAllocator::alloc_object()`, with a check to add a non-blocking flag or not. The null-check for eden allocation has been moved to its single caller in zCollectedHeap.cpp. This makes the API in ZObjectAllocator more general and we don't have to track methods for both eden and relocation allocations.
>> 
>> `_relocation_allocators` has been moved from ZAllocator to ZObjectAllocator and renamed to `NumRelocationAllocators` to be more consistent with the naming style in ZGC.
>> 
>> Testing:
>> * Oracle's tier 1-5
>
> Joel Sikström has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - -Wconversion fixes
>  - Intermediate alloc_object in ZHeap

The change looks good. But I have a few thoughts if we want to take this a step further.

This change starts adding a static interface on `ZObjectAllocator` which is used for `retire_pages`. I wonder if we can take this all the way and make `ZObjectAllocator` a static only interface. And keep the Allocator implementation opaque / private. 

So instead of first pulling out the allocator and calling the corresponding function, we go through the static interface. So we get something like
```c++
  static void initialize();
  static void retire_pages(ZPageAgeRange range);
  static zaddress alloc_object(size_t size, ZPageAge age);
  static void undo_alloc_object(zaddress addr, size_t size, ZPageAge age);
  static size_t remaining_in_eden();

src/hotspot/share/gc/z/zObjectAllocator.hpp line 44:

> 42:   static constexpr uint NumRelocationAllocators = NumAllocators - 1;
> 43: 
> 44:   typedef Deferred<ValueObjArray<ZObjectAllocator, NumAllocators>> Allocators;

Nit, I prefer `using` over `typedef`.
Suggestion:

  using Allocators = Deferred<ValueObjArray<ZObjectAllocator, NumAllocators>>;

src/hotspot/share/gc/z/zObjectAllocator.hpp line 85:

> 83:   static ZObjectAllocator* eden();
> 84: 
> 85:   static void retire_pages(ZPageAgeRange range);

We tend to avoid mixing static and non static interfaces.

Maybe adding a `ZObjectAllocators` to wrap the static interface is preferable.

src/hotspot/share/gc/z/zObjectAllocator.inline.hpp line 43:

> 41:   guarantee(size <= ZHeap::heap()->max_tlab_size(), "TLAB too large");
> 42:   return alloc_object(size);
> 43: }

This could probably be on `ZHeap` instead and only have `alloc_object` on `ZObjectAllocator`

src/hotspot/share/gc/z/zObjectAllocator.inline.hpp line 47:

> 45: inline void ZObjectAllocator::retire_pages(ZPageAgeRange range) {
> 46:   for (ZPageAge age : range) {
> 47:     _allocators->at((int)untype(age))->retire_pages();

Suggestion:

    allocator(age)->retire_pages();

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

PR Review: https://git.openjdk.org/jdk/pull/25693#pullrequestreview-2920054625
PR Review Comment: https://git.openjdk.org/jdk/pull/25693#discussion_r2142045793
PR Review Comment: https://git.openjdk.org/jdk/pull/25693#discussion_r2142109305
PR Review Comment: https://git.openjdk.org/jdk/pull/25693#discussion_r2142153886
PR Review Comment: https://git.openjdk.org/jdk/pull/25693#discussion_r2142089670