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

[Guoxiong Li]

On Tue, 21 Jan 2025 14:43:57 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Simple refactoring of removing the use of `virtual` method and use concrete subclasses when needed.
>> 
>> Test: tier1-5
>
> 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

Several nits/questions.

I am not familiar with the Shenandoah GC, but it is not much modified in this patch. It may be good for a Shenandoah maintainer/developer to review/comfirm this PR.

src/hotspot/share/gc/epsilon/epsilonMonitoringSupport.hpp line 31:

> 29: 
> 30: class EpsilonGenerationCounters;
> 31: class GenerationCounters;

The pre-declaration `GenerationCounters` seems be not needed now. Same in `shenandoahMonitoringSupport.hpp`.

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?

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.

src/hotspot/share/gc/shared/generationCounters.hpp line 54:

> 52:   GenerationCounters(const char* name, int ordinal, int spaces,
> 53:                    size_t min_capacity, size_t max_capacity,
> 54:                    size_t curr_capacity);

Some parameters or arguments formats, like this one, can be adjusted/aligned. It seems an IDE automatical alignment. So, it is not a big problem.

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

Changes requested by gli (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23209#pullrequestreview-2583810474
PR Review Comment: https://git.openjdk.org/jdk/pull/23209#discussion_r1935554161
PR Review Comment: https://git.openjdk.org/jdk/pull/23209#discussion_r1935650179
PR Review Comment: https://git.openjdk.org/jdk/pull/23209#discussion_r1935668622
PR Review Comment: https://git.openjdk.org/jdk/pull/23209#discussion_r1935679850