RFR: 8327000: GenShen: Integrate updated Shenandoah implementation of FreeSet into GenShen [v8]

Kelvin Nilsen kdnilsen at openjdk.org
Thu Jun 20 17:53:31 UTC 2024


On Wed, 19 Jun 2024 01:43:36 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Minor refinements to test programs
>>   
>>   TestAllocIntArrays: comments to explain behavior.
>>   TestOldGrowthTriggers: reduce the number of loop iterations so this test
>>   will not time out on less powerful test platforms.
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 418:
> 
>> 416:   // old collector partition to assure that old collector partition has at least old_reserve.  Update old_region_count
>> 417:   // to represent the total number of regions in the old generation by adding the number of regions moved from the
>> 418:   // mutator partition to the old collector partition.
> 
> How about:
> 
> 
> Ensure that Collector has at least `to_reserve` regions, and OldCollector has at
> least `old_reserve` regions.
> Upon return `old_region_count` holds the number of regions in OldCollector.
> 
> 
> This makes me wonder about the asymmetry of the return value. Also could it be a plain return value from the method rather than var parameter to the method?

I've adjusted this comment as suggested.  old_region_count is both input and output, so I'm leaving as is.  This also more closely mirrors the openjdk/jdk implementation.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/440#discussion_r1647940732


More information about the shenandoah-dev mailing list