RFR: 8324649: Shenandoah: refactor implementation of free set [v39]

[Kelvin Nilsen]

On Wed, 10 Apr 2024 11:16:11 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove debugging instrumentation
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 42:
> 
>> 40:     case Mutator: return "Mutator";
>> 41:     case Collector: return "Collector";
>> 42:     default: return "Unrecognized";
> 
> I believe using an 'enum class' for ShenandoahFreeSetPartitionId means you don't need to have a default.

Compiler still wants the default clause.  Otherwise, "control reaches end of non-void function".  You'd think the compiler could be less strict, but will leave as is, unless we can figure out a way to get around this.

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 49:
> 
>> 47:   assert((start_idx >= 0) && (start_idx < _num_bits), "precondition");
>> 48:   size_t array_idx = start_idx >> LogBitsPerWord;
>> 49:   uintx element_bits = _bitmap[array_idx];
> 
> Same here, try to stick with what is done in bitMap.hpp (typedef uintptr_t bm_word_t, you could just as well typedef uintx bm_word_t, if you prefer).

This is one of the "philosophical" and "tactical" differences between ShenandoahSimpleBitMap and BitMap.  ShenandoahSimpleBitMap allows searches to go in the backward direction, and uses Sentinel value -1 to indicate NotFound, and to represent "NoLimitOnBackwardSearch".  BitMap only searches in the forward direction.  Thus, it doesn't need signed values.

I could introduce a typedef to make future translation simpler if you think that would be useful.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1566568060
PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1566570315