RFR: 8365880: Shenandoah: Unify memory usage accounting in ShenandoahFreeSet [v12]

[Kelvin Nilsen]

On Thu, 2 Oct 2025 23:48:43 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with five additional commits since the last revision:
>> 
>>  - small refactoring
>>  - OO refinements to used_regions_size()
>>  - fix broken assert from previous commit
>>  - OO refinements to free_unaffiliated_regions()
>>  - OO refinements to max_capacity()
>
> src/hotspot/share/gc/shenandoah/shenandoahMemoryPool.cpp line 65:
> 
>> 63:   // to make sense under the race. See JDK-8207200.
>> 64:   committed = MAX2(used, committed);
>> 65:   assert(used <= committed, "used: %zu, committed: %zu", used, committed);
> 
> Shouldn't it always be the case that `initial <= used <= committed <= max`?

It should be that way.  Yes.  But we have been known to violate these conditions, especially when unsigned arithmetic underflows (i.e. small_unsigned_value - large_unsigned_value yields really big unsigned_value)

> src/hotspot/share/gc/shenandoah/shenandoahSimpleBitMap.cpp line 42:
> 
>> 40: }
>> 41: 
>> 42: size_t ShenandoahSimpleBitMap::count_leading_ones(index_type start_idx) const {
> 
> Did we figure out why we want to rename this type? It adds a lot of noise to this PR and I'd hate to do something like this because of a C++ technicality.

I've reverted this change.  In a different PR, we might decide to address this in a different way.  We have multiple typedefs for idx_t even within the shenandoah development directory..

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26867#discussion_r2412009478
PR Review Comment: https://git.openjdk.org/jdk/pull/26867#discussion_r2412011761