RFR: 8348400: GenShen: assert(ShenandoahHeap::heap()->is_full_gc_in_progress() || (used_regions_size() <= _max_capacity)) failed: Cannot use more than capacity # [v2]

[William Kemper]

On Tue, 25 Mar 2025 19:53:35 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> William Kemper has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Revert "Do not enforce size constraints on generations"
>>   
>>   This reverts commit 11ff0677449fa6749df8830f4a03f1c7861ba314.
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 1293:
> 
>> 1291: 
>> 1292:   ShenandoahGenerationalHeap* gen_heap = ShenandoahGenerationalHeap::heap();
>> 1293:   const size_t region_capacity = alloc_capacity(r);
> 
> A general note on terminology. We have generally used "capacity" to mean the total space, including that which has been allocated, and "used" for the space that has been allocated and isn't available to allocate. I'd use "free" here and avoid the extra arithmetic.
> 
> I notice that the method actually uses "used", rather than "free".
> 
> I think the interface for _partitions `move_from_...` is unnecessarily fat. Since we send the region idx to the `move_from_...` method, why not let that method get the amount free, rather than passing it as an additional parameter?
> 
> I see that we essentially use this value only at line 1300 to correct the evacuation reserve figure. (Side question: Why don't we do that when we do the swap after line 1327?)

I see your point, but there are cases where the business logic depends on the allocation capacity before adding the region to the freeset. In those cases, we'd compute allocation capacity twice. We could have an overload to compute and forward allocation capacity for the other cases?

That's a good question. We should probably subtract the capacity of the unusable trash region from the reserve, and add the capacity of the usable region back in.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23998#discussion_r2012933965