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