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