RFR: 8357053: ZGC: Improved utility for ZPageAge [v2]

[Stefan Karlsson]

On Fri, 16 May 2025 07:11:38 GMT, Joel Sikström <jsikstro at openjdk.org> wrote:

>> Hello,
>> 
>> This RFE improves utility for converting to/from, iterating over and defining structures that are indexed using the `ZPageAge` type.
>> 
>> Converting to/from ZPageAge and its underlying type (uint8_t, often just uint) is currently done via using static_cast. This works fine because sane values are converted in all use cases. However, to make conversion safer (and also more readable), I propose we add a `to_zpageage` and a corresponding `untype` that checks that the conversion is valid. Such conversion methods should be used instead of calling `static_cast<uint/ZPageAge>`.
>> 
>> We currently define a value called `ZPageAgeMax`, which is defined as `static_cast<uint>(ZPageAge::old)`. The majority of places that use this value actualy use `ZPageAgeMax + 1`, which is equivalent to the number of ages. Instead, I propose we define and use a value that represents the number of possible ages, called `ZPageAgeCount`.
>> 
>> Lastly, to make iterating over ages more accessible, I propose we create an intreface of enum iterators of ZPageAge. This will also create a foundation for generating values that require a ZPageAge in the future. Since the end of the enum iterators are exclusive, I've opted to use the following value as end for the iterators:
>> 
>> constexpr ZPageAge ZPageAgeLastPlusOne = static_cast<ZPageAge>(ZPageAgeCount);
>> 
>> 
>> I see us using either this or a sentinel/dummy value at the end of the enum class, but I prefer having a value similar to `ZPageAgeLastPlusOne` over a dummy value.
>> 
>> Testing:
>> * Oracle's tier 1-4
>> * GHA
>
> Joel Sikström has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Copyright years
>  - Simplify untype(ZPageAge age)

A couple of questions before fully reviewing this?

src/hotspot/share/gc/z/zAllocator.inline.hpp line 38:

> 36: 
> 37: inline ZAllocatorForRelocation* ZAllocator::relocation(ZPageAge page_age) {
> 38:   return _relocation[untype(page_age) - 1];

I wonder if we should have our own defined ZPageAge operators for `+` and `-` (or maybe inc/dec) and we could add verification that we don't fall out of the range?
Suggestion:

  return _relocation[untype(page_age - 1)];

src/hotspot/share/gc/z/zPageAge.hpp line 55:

> 53:                        static_cast<uint>(ZPageAge::eden),
> 54:                        ZPageAgeCount);
> 55: 

Could this be using the other define to set this up?
Suggestion:

ENUMERATOR_VALUE_RANGE(ZPageAge,
                       ZPageAge::eden,
                       ZPageAge::old);

Or does that not work?

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

PR Review: https://git.openjdk.org/jdk/pull/25251#pullrequestreview-2863980273
PR Review Comment: https://git.openjdk.org/jdk/pull/25251#discussion_r2104314371
PR Review Comment: https://git.openjdk.org/jdk/pull/25251#discussion_r2104312132