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

Kelvin Nilsen kdnilsen at openjdk.org
Tue Apr 16 00:48:20 UTC 2024


On Wed, 10 Apr 2024 11:36:45 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 186:
> 
>> 184: }
>> 185: 
>> 186: ssize_t ShenandoahSimpleBitMap::find_prev_consecutive_bits(
> 
> It looks weird to me to have an opening ( and then all the args on the next line. I think in HotSpot, it's usually done like:
> 
> ssize_t ShenandoahSimpleBitMap::find_prev_consecutive_bits(const size_t num_bits,
>                                                            ssize_t last_idx,
>                                                            const ssize_t boundary_idx) const {

Fixed

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.inline.hpp line 31:
> 
>> 29: #include "gc/shenandoah/shenandoahFreeSet.hpp"
>> 30: 
>> 31: inline ssize_t ShenandoahSimpleBitMap::find_next_set_bit(ssize_t start_idx, ssize_t boundary_idx) const {
> 
> bitMap.hpp calls it get_next_one_offset(), maybe keep names, args (and impl?) the same as there, to make future merge easier? I guess that is true for all methods that also exist in bitMap.hpp. If you do, then say in comment that the whole block is taken from bitMap.* to make review and later merge easier.

I'll work on this tomorrow.  Thanks.

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

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


More information about the hotspot-gc-dev mailing list