RFR: Add generations to freeset [v5]

Kelvin Nilsen kdnilsen at openjdk.org
Tue Apr 18 23:07:44 UTC 2023


On Thu, 13 Apr 2023 16:13:55 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:
>> 
>>   Remove debug instrumentation
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 37:
> 
>> 35:   CHeapBitMap _mutator_free_bitmap;
>> 36:   CHeapBitMap _collector_free_bitmap;
>> 37:   // We keep the _old_collector regions separate from the young collector regions.  This allows us to pack the old regions
> 
> Looks like this comment block needs to move down to line 44.

I've added some more comments to make this more clear, I hope.

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 77:
> 
>> 75:   inline void clear_mutator_free(size_t idx);
>> 76:   inline void clear_collector_free(size_t idx);
>> 77:   inline void clear_old_collector_free(size_t idx);
> 
> Re an earlier comment, I suggest use of mathematical set nomenclature here for ease of reading, i.e. `set_x_free` -> `add_to_x`, `clear_x_free` -> `remove_from_x`, etc. with suitable assertion checks in those methods (before removal and after addition) that verify global steady-state invariants (under lock, I assume).

I agree with the ideal of improved abstraction, as discussed in other context.  I'm inclined to keep the existing API as is for now, with a todo statement describing possible future refactoring to improve abstraction.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1170646990
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1170648009


More information about the shenandoah-dev mailing list