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 shenandoah-dev
mailing list