RFR: 8342214: GenShen: Reduce code duplication in shFreeSet with iterator abstraction

[William Kemper]

On Tue, 15 Oct 2024 20:24:27 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Introduce `ShenandoahLeftRightIterator` and `ShenandoahRightLeftIterator` classes to encapsulate allocation bias to one side of the heap or the other.
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 58:
> 
>> 56:   idx_t _end;
>> 57:   ShenandoahRegionPartitions* _partitions;
>> 58:   ShenandoahFreeSetPartitionId _partition;
> 
> I'd call this a `_pid`, short for free set partition id. The type mismatch of `_partitions` (plural) and `_partition` (singular) makes the current form of naming a bit awkward, and the single-letter difference rife for confusion when reading code (although the type-system saves one here).

`_pid` makes me think of `pid` (process ID). I'll go with `_partition_id`.

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 296:
> 
>> 294:   // Return the address of memory allocated, setting in_new_region to true iff the allocation is taken
>> 295:   // from a region that was previously empty.  Return nullptr if memory could not be allocated.
>> 296:   inline HeapWord* allocate_from_partition_with_affiliation(ShenandoahAffiliation affiliation,
> 
> The "from_partition" in the name of the method sounds a bit mysterious since the first argument of the old method is now gone. How about just `allocate_with_affiliation`, and let the method signature distinguish between the method at line 347 -- or if you prefer rename the latter to `allocate_with_affiliation_using_iterator()`?

I'll take out `from_partition`.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/512#discussion_r1803800707
PR Review Comment: https://git.openjdk.org/shenandoah/pull/512#discussion_r1803801414