RFR: Add generations to freeset [v8]

Kelvin Nilsen kdnilsen at openjdk.org
Thu Apr 20 19:11:54 UTC 2023


On Thu, 20 Apr 2023 18:03:35 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 207 commits:
>> 
>>  - Merge remote-tracking branch 'origin' into add-generations-to-freeset
>>  - Respond to reviewer feedback
>>    
>>    Various improvements suggested by reviewers.  Mostly improved comments
>>    and some minor refactoring.
>>  - Add TODO comment for exapnsion of old-gen
>>  - Remove debug instrumentation
>>  - Merge master
>>  - Fix calculation of minimum fill size
>>    
>>    We were incorrectly using byte size rather than word size.
>>  - Fix error in ShenandoahFreeSet usage accounting
>>    
>>    We were incorrectly increasing used for plab padding.  That is
>>    old_collector memory and should not affect mutator usage.  This commit
>>    also includes some refactoring, additional assertions, and additional
>>    verification of consistent free-space accounting.
>>  - Fix typo in a comment
>>  - Fix white space
>>  - Merge remote-tracking branch 'GitFarmBranch/add-generations-to-freeset' into add-generations-to-freeset
>>  - ... and 197 more: https://git.openjdk.org/shenandoah/compare/016bf071...7319eeeb
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 125:
> 
>> 123: 
>> 124: 
>> 125: HeapWord* ShenandoahFreeSet::allocate_old_with_affiliation(ShenandoahAffiliation affiliation,
> 
> Will `affiliation` always be `old` here? Should we remove this parameter?

Affiliation might be FREE.  First, we try to allocate from within an old-collector-region that is_old().  If that fails, we'll try to allocate from an old_collector_region that is not afilliated.

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 540:
> 
>> 538:     } else if (r->free() < PLAB::min_size() * HeapWordSize) {
>> 539:       // Permanently retire this region if there's room for a fill object
>> 540:       size_t waste = r->free();
> 
> Are we writing a filler object here to keep this region out of the old collector freeset when it's rebuilt?

yes.  Putting the region back into the old_collector set at rebuild time just makes work for us, because then we'll have to rediscover that it needs to be retired, and will need to retire it, and will need to adjust the bounds again.

> src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp line 312:
> 
>> 310: 
>> 311:     // Old regions are under marking, still need SATB barriers.
>> 312:     OLD_MARKING_BITPOS = 5,
> 
> Are these trailing commas intentional?

my mistake.  I had another constant in this list, and then removed it.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1172983688
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1172982613
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1172984835


More information about the shenandoah-dev mailing list