RFR: 8331573: Rename CollectedHeap::is_gc_active to be explicitly about STW GCs

[Stefan Karlsson]

On Thu, 2 May 2024 14:40:35 GMT, Aleksey Shipilev <shade at 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`

Seems like a good change.

src/hotspot/share/gc/parallel/psParallelCompact.cpp line 1270:

> 1268: 
> 1269:   ParallelScavengeHeap* heap = ParallelScavengeHeap::heap();
> 1270:   assert(!heap->is_stw_gc_active(), "not reentrant");

While reading this I see that all these "not reentrant" asserts seems redundant given that we already do these checks in `IsSTWGCActiveMark`. Brownies points if you get rid of them. ;)

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.

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

Changes requested by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19064#pullrequestreview-2036315901
PR Review Comment: https://git.openjdk.org/jdk/pull/19064#discussion_r1587988542
PR Review Comment: https://git.openjdk.org/jdk/pull/19064#discussion_r1587974562