RFR: Add generations to freeset [v20]

Kelvin Nilsen kdnilsen at openjdk.org
Sat Apr 29 13:49:54 UTC 2023


On Sat, 29 Apr 2023 00:47:06 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:
>> 
>>   Fix white space
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 32:
> 
>> 30: #include "gc/shenandoah/shenandoahHeap.hpp"
>> 31: 
>> 32: enum MemoryReserve : uint8_t {
> 
> `FreeMemoryType` or `MemoryType` instead of `MemoryReserve` for the enum type name ?

That's probably better.

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 32:
> 
>> 30: #include "gc/shenandoah/shenandoahHeap.hpp"
>> 31: 
>> 32: enum MemoryReserve : uint8_t {
> 
> Would it tidy up matters to move `ShenandoahSetsOfFree` into its own source file(s)? I realize it's only used by `ShenandoahFreeSet`, but I think it'll keep it all much cleaner.

My preference for expedience is to keep as is.  But if you feel strongly, I'll separate it out.

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 87:
> 
>> 85:   inline bool in_free_set(size_t idx, MemoryReserve which_set) const;
>> 86: 
>> 87:   // Each of the following four methods returns _max to indicate absence of requested region.
> 
> Are these expected invariants for returned values for queries on a static set `which_set`:
> 
> -  `0 <= leftmost <= leftmost_empty <= rightmost_empty <= rightmost <= _max - 1`, whenever `which_set` is not empty
> - `_max`, whenever `which_set` is empty
> 
> ?

Thanks for this question.  The comment is old and wrong.  I'm updating the comment.

Your proposed wording is correct except that the only left_most returns _max whenever which_set is empty.  right_most returns 0 when which_set is empty.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1181081125
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1181081305
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1181080380


More information about the shenandoah-dev mailing list