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

[Axel Boldt-Christmas]

On Mon, 23 Jun 2025 13:08:18 GMT, Joel Sikström <jsikstro at openjdk.org> wrote:

>> Hello,
>> 
>> After [JDK-8353184](https://bugs.openjdk.org/browse/JDK-8353184), ZAllocator has essentially become a mirror of ZObjectAllocator, questioning the relevance of ZAllocator. The purpose of this RFE is to combine ZAllocator and ZObjectAllocator into a single class ZObjectAllocator.
>> 
>> The solution I propose moves the new ZObjectAllocator as a data member in ZHeap, and is accessed solely from ZHeap. ZObjectAllocator stores a number of allocators, one for each age. Instead of storing eden and relocation allocators separately, all allocators are now in a single array, which makes it more straightforward to iterate over all allocators (when retiring pages for example). Since the underlying allocator (called PerAge) needs to be constructed with different ages, we can't use the default constructor/initialization, so I have a solution in-place for dealing with this.
>> 
>> Undoing an object allocation has been moved in its entirety (not calling into ZObjectAllocator) to ZHeap, where it fits much better.
>> 
>> Testing:
>> * Oracle's tier 1-5
>
> Joel Sikström has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8358586_zallocator_cleanup
>  - Rework ZObjectAllocator
>  - Private member variables in ZObjectAlloactorImpl
>  - Move implementation to .cpp and static-only interface in .hpp
>  - Consistent template friend style
>  - Friend class ValueObjBlock
>  - AllStatic interface with ZObjectAllocators
>  - -Wconversion fixes
>  - Intermediate alloc_object in ZHeap
>  - 8358586: ZGC: Combine ZAllocator and ZObjectAllocator

Looks good. I like the new approach with keeping the state as part of ZCollectedHeap better.

Just a small suggestion.

src/hotspot/share/gc/z/zDeferredConstructed.hpp line 33:

> 31: 
> 32: template<typename T>
> 33: class ZDeferredConstructed {

We should move the definitions to zDeferredConstructed.inline.hpp. It is an exceptional case that we have them in the header instead of the inline header file. 

_(We usually only do this when the compiler needs a definition like when creating destructors for types containing none trivial destructors, we would probably also allow it if we would want constexpr creation of variables in header files)_

src/hotspot/share/gc/z/zDeferredConstructed.hpp line 38:

> 36:   };
> 37: 
> 38:   DEBUG_ONLY(bool _initialized);

Suggestion:

  DEBUG_ONLY(bool _initialized;)

src/hotspot/share/gc/z/zDeferredConstructed.hpp line 83:

> 81:   void initialize(Ts&&... args) {
> 82:     assert(!_initialized, "Double initialization forbidden");
> 83:     DEBUG_ONLY(_initialized = true);

Suggestion:

    DEBUG_ONLY(_initialized = true;)

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

Changes requested by aboldtch (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25693#pullrequestreview-2956582351
PR Review Comment: https://git.openjdk.org/jdk/pull/25693#discussion_r2165795004
PR Review Comment: https://git.openjdk.org/jdk/pull/25693#discussion_r2165780778
PR Review Comment: https://git.openjdk.org/jdk/pull/25693#discussion_r2165781100