RFR: 8345423: Shenandoah: Parallelize concurrent cleanup [v4]

Xiaolong Peng xpeng at openjdk.org
Wed Dec 4 22:08:39 UTC 2024


On Wed, 4 Dec 2024 18:36:28 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> Xiaolong Peng has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix naming issue
>
> src/hotspot/share/gc/shenandoah/shenandoahGeneration.cpp line 890:
> 
>> 888: size_t ShenandoahGeneration::decrement_affiliated_region_count() {
>> 889:   // Assertions only hold true for Java threads since they call this method under heap lock.
>> 890:   bool const is_java_thread = Thread::current()->is_Java_thread();
> 
> Prefer not to check `Thread::current` to gate assertions. Could we use an `#ifdef ASSERT` block here? Could this be `decrease_affiliated_region_count(1)` instead? or should we have a separate `decrement_under_lock` method?

It will be weird if I only change   `decrement_affiliated_region_count` to `decrement_affiliated_region_count_under_lock` in this file, while all the other `decrement_x` / `decrease_x` /`increment_x` / `increase_x` methods in files follow the same convention.

It is probably better to add new one like `decrement_affiliated_region_count_without_lock` and not change the existing methods' behavior.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22538#discussion_r1870342598


More information about the shenandoah-dev mailing list