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

[Roman Kennke]

On Fri, 1 Mar 2024 21:22:54 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 68:
>> 
>>> 66:   // Count consecutive ones in forward order, starting from start_idx.  Requires that there is at least one zero
>>> 67:   // between start_idx and index value (_num_bits - 1), inclusive.
>>> 68:   size_t count_leading_ones(ssize_t start_idx) const;
>> 
>> Does the argument have to me `ssize_t`, not `size_t`? Is this because you want it to be signed?
>
> I'm going to add the following comment at the top of shenandoahFreeSet.hpp:
> 
> // The API and internal implementation of ShenandoahSimpleBitMap and ShenandoahRegionPartitions use ssize_t to                      
> // represent index, even though index is "inherently" unsigned.  There are two reasons for this choice:                             
> //  1. We use -1 as a sentinel value to represent empty partitions.  This same value may be used to represent                       
> //     failure to find a previous set bit or previous range of set bits.                                                            
> //  2. Certain loops are written most naturally if the iterator, which may hold the sentinel -1 value, can be                       
> //     declared as signed and the terminating condition can be < 0.

Please use types, method signatures and keep structure similar to bitMap.hpp, which use idx_t, which is a size_t (see e.g. BitMap::count_one_bits()).

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

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