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

William Kemper wkemper at openjdk.org
Thu Oct 2 23:16:50 UTC 2025


On Tue, 30 Sep 2025 21:55:46 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> This PR eliminates redundant bookkeeping that had been carried out by both ShenandoahGeneration and ShenandoahFreeSet.  In the new code, we keep a single tally of relevant information within ShenandoahFreeSet.
>> Queries serviced by ShenandoahGeneration are now delegated to ShenandoahFreeSet.
>> 
>> This change eliminates rare and troublesome assertion failures that were often raised when the ShenandoahFreeSet tallies did not match the ShenandoahGeneration tallies.  These assertion failures resulted because the two sets of books are updated at different times, using different synchronization mechanisms.
>> 
>> The other benefit of this change is that we have less synchronization overhead because we only have to maintain a single set of books.
>
> 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()

A few comment from our review session. Will post more later.

src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp line 953:

> 951: }
> 952: 
> 953: size_t ShenandoahGeneration::max_capacity() const {

We never instantiate `ShenandoahGeneration` even non-generational mode gets `ShenandoahGlobalGeneration`. You could make these methods pure virtual. The implementation here should never be called.

src/hotspot/share/gc/shenandoah/shenandoahGlobalGeneration.cpp line 40:

> 38: 
> 39: size_t ShenandoahGlobalGeneration::max_capacity() const {
> 40: #ifdef ASSERT

Could we just have this assertion one time in the constructor?

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

Changes requested by wkemper (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26867#pullrequestreview-3217907983
PR Review Comment: https://git.openjdk.org/jdk/pull/26867#discussion_r2400363917
PR Review Comment: https://git.openjdk.org/jdk/pull/26867#discussion_r2400358514


More information about the hotspot-gc-dev mailing list