RFR: 8348171: Refactor GenerationCounters and its subclasses [v2]

[Albert Mingkun Yang]

On Thu, 30 Jan 2025 13:53:13 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> Albert Mingkun Yang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
>> 
>>  - merge
>>  - gen-counter
>
> src/hotspot/share/gc/parallel/psYoungGen.cpp line 95:
> 
>> 93:   // Generation Counters - generation 0, 3 subspaces
>> 94:   _gen_counters = new GenerationCounters("new", 0, 3, min_gen_size(),
>> 95:                                          max_gen_size(), virtual_space()->committed_size());
> 
> Question:
> I notice the `virtual_space()->committed_size()` here is not the same as `_virtual_space.committed_size()` of Serial GC. Because one is a pointer and another is just an pure object instead of a pointer. Why are they different?  Is this difference occasional or one convention I don't know before?

I don't think there is a fundamental reason that they have to be different. We can probably uniform them, but the benefit seems small, IMO.

> src/hotspot/share/gc/shared/generationCounters.cpp line 72:
> 
>> 70: void GenerationCounters::update_all(size_t curr_capacity) {
>> 71:   _current_size->set_value(curr_capacity);
>> 72: }
> 
> I don't think the `update_all` is a good method name because it only modifies the current capacity(committed size). But this method had been used since OpenJDK opened source, so we don't know its commit history before opening source. Maybe the method name `update_capacity` in the class `ZGenerationCounters` is a better name we can adopt here.

I agree the current name can be improved to sth like `update_capacity`, but it also seems that such renaming can/should be done in its own PR, so I kept it as is.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23209#discussion_r1935731114
PR Review Comment: https://git.openjdk.org/jdk/pull/23209#discussion_r1935737525