RFR: 8324649: Shenandoah: refactor implementation of free set [v6]

Kelvin Nilsen kdnilsen at openjdk.org
Sat Feb 3 14:24:03 UTC 2024


On Thu, 1 Feb 2024 07:12:58 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> So the documentation here might be:
>> 
>> // Move cset_regions number of regions from being available to the collector to
>> // being available to the mutator.
>> //
>> // Typical usage is at the end of evacuation, when the collector no longer needs
>> // the regions that were reserved for evacuation, and these can now be
>> // made available for mutator allocation.
>> 
>> BTW, why call the number of regions `cset_regions`?
>
> Also, the concept of partition is itself an internal implementation detail that you have carefully encapsulated in this class. There is no point in leaking that out in the naming of the method.
> 
> The method can just be called `move_regions_from_collector_to_mutator(size_t num)` and be done? "Partition" here adds no value and can be confusing leakage of abstraction.

Agree.  Thanks for these improvements.  Done.  (also, have enhanced comment to clarify the intent of cset_regions argument, which represents the number of regions in the collection set.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1477072830


More information about the hotspot-gc-dev mailing list