RFR: 8324649: Shenandoah: refactor implementation of free set [v4]
Y. Srinivas Ramakrishna
ysr at openjdk.org
Mon Jan 29 00:24:37 UTC 2024
On Sun, 28 Jan 2024 23:06:41 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:
>> Kelvin Nilsen has updated the pull request incrementally with two additional commits since the last revision:
>>
>> - Fix typo in comment
>> - Remove unnecessary include
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 41:
>
>> 39: case Mutator: return "Mutator";
>> 40: case Collector: return "Collector";
>> 41: case NumFreeSets: return "NumFreeSets";
>
> How is "NumFreeSets" a type whose name would be used? I'd remove it from here.
For the same reason, it seems curious to have "NotFree" be a valid "free memory type". If the types are a partitioning of free memory, they would be FreeMemoryTypes (or more correctly FreeRegionTypes). If they are a partitioning of all regions, they should be RegionTypes and can include both Free and NotFree.
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 40:
>
>> 38: };
>> 39:
>> 40: class ShenandoahSetsOfFree {
>
> I wonder if "ShenandoahSetsOfFreeRegions" or "ShenandoahFreeRegionSets" is a better name.
The other possibility is to think of the types defined further above as inducing a partitioning of the [free] regions on the heap, and call it `Shenandoah[Free]RegionPartition`.
I place [free] in the optional bracket because you have a NotFree type, so I am not sure if the partitioning implicitly includes all regions of the heap both free and not free, depending on which the "free" might be kept or dropped.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1468956592
PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1468954374
More information about the hotspot-gc-dev
mailing list