On Fri, 26 Jun 2026 04:33:36 GMT, Ivan Walulya <[email protected]> wrote:

> Hi,
> 
> Please review this change to rename G1 region count fields to use num_regions 
> or num_*_regions consistently where the value represents a number regions. 
> Keep length terminology where the collection explicitly refers to the length 
> of a list. 
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

Looks good. I have a few nits that you might want to consider.

src/hotspot/share/gc/g1/g1AllocRegion.inline.hpp line 35:

> 33:   do {                                                                   \
> 34:     assert((p), "[%s] %s c: %u r: " PTR_FORMAT,                          \
> 35:            _name, (message), _num_regions_used, 
> p2i(_alloc_region.load_relaxed())   \

This messes with the alignment of the backslashes.

src/hotspot/share/gc/g1/g1HeapRegionSet.hpp line 34:

> 32:   do {                                     \
> 33:     assert((p), "[%s] %s ln: %u",          \
> 34:            name(), message, num_regions());\

I think injecting a row of extra spaces would be good her and below.

src/hotspot/share/gc/g1/g1RegionsOnNodes.cpp line 29:

> 27: #include "gc/g1/g1RegionsOnNodes.hpp"
> 28: 
> 29: G1RegionsOnNodes::G1RegionsOnNodes() : _regions_per_node(nullptr), 
> _numa(G1NUMA::numa()) {

Should this be `_num_regions_per_node` to stay consistent with the naming?

src/hotspot/share/gc/g1/vmStructs_g1.hpp line 71:

> 69:   nonstatic_field(G1MonitoringSupport, _old_gen_used,             size_t) 
>     \
> 70:                                                                           
>     \
> 71:   nonstatic_field(G1HeapRegionSetBase, _num_regions,       uint)          
>     \

The alignment was slightly adjusted here, but it doesn't match the block before 
or the block after, and still there are some extra spaces before the `uint`. 
Maybe some slight tweaking here would be nice.

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

Marked as reviewed by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/31688#pullrequestreview-4577734535
PR Review Comment: https://git.openjdk.org/jdk/pull/31688#discussion_r3479964289
PR Review Comment: https://git.openjdk.org/jdk/pull/31688#discussion_r3479975116
PR Review Comment: https://git.openjdk.org/jdk/pull/31688#discussion_r3479982601
PR Review Comment: https://git.openjdk.org/jdk/pull/31688#discussion_r3479991448

Reply via email to