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

[Kelvin Nilsen]

On Fri, 10 May 2024 14:18:19 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Another attempt at improving clarity of comments
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 144:
> 
>> 142:   assert (which_partition < NumPartitions, "selected free partition must be valid");
>> 143:   idx_t idx = _rightmosts[int(which_partition)];
>> 144:   // _membership[which_partition].is_set(idx) may not be true if we are shrinking the interval
> 
> Same here.

Thanks.  I've rewritten both of those comments to clarify what I was thinking...  Hope this is more clear now.

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 334:
> 
>> 332: const char* ShenandoahRegionPartitions::partition_membership_name(idx_t idx) const {
>> 333:   ShenandoahFreeSetPartitionId result = membership(idx);
>> 334:   return partition_name(result);
> 
> Could make it even more succinct by doing `return partition_name(membership(idx));`

Nice improvement.  Thanks. Changed.

> src/hotspot/share/gc/shenandoah/shenandoahSimpleBitMap.hpp line 3:
> 
>> 1: /*
>> 2:  * Copyright (c) 2016, 2019, Red Hat, Inc. All rights reserved.
>> 3:  * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
> 
> Same copyright issue.

Fixed.

> src/hotspot/share/gc/shenandoah/shenandoahSimpleBitMap.hpp line 92:
> 
>> 90:   }
>> 91: 
>> 92:   inline idx_t alignment() const {
> 
> I guess this could even be constexpr.

Thanks.  Fixed.

> src/hotspot/share/gc/shenandoah/shenandoahSimpleBitMap.inline.hpp line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2016, 2019, Red Hat, Inc. All rights reserved.
> 
> Same copyright issue.

Fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1597820331
PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1597820649
PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1597826251
PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1597826232
PR Review Comment: https://git.openjdk.org/jdk/pull/17561#discussion_r1597826480