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