On Thu, 2 May 2024 16:56:11 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:
>> `CollectedHeap::is_gc_active()` is confusing, since its name implies _any_ >> GC phase is running, while it actually only covers the STW GCs. It would be >> good to rename it for clarity. The freed-up name, `is_gc_active` could then >> be repurposed to track any (concurrent or STW) GC phase running. That would >> be useful to resolve >> [JDK-8331572](https://bugs.openjdk.org/browse/JDK-8331572). >> >> Doing this rename separately guarantees we have caught and renamed all >> current uses. >> >> Additional testing: >> - [ ] Linux AArch64 server fastdebug, `all` > > src/hotspot/share/gc/parallel/psParallelCompact.cpp line 1493: > >> 1491: PCAddThreadRootsMarkingTaskClosure(uint worker_id) : >> _worker_id(worker_id) { } >> 1492: void do_thread(Thread* thread) { >> 1493: assert(ParallelScavengeHeap::heap()->is_stw_gc_active(), "called >> outside gc"); > > Should this be updated to "called outside gc pause" as you did in > `G1CollectedHeap::pin_object`? The same comment goes for the other > occurrences below. I deliberately stopped myself from doing this for Parallel GC code, where every GC is STW GC :) I can change to "GC pause" if you want. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19064#discussion_r1587999105
