On Thu, 2 May 2024 17:04:44 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 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. ;) Ah, hm. Indeed! Separate PR? There is some light cleanup in G1 that can be associated with it. This PR would keep with just a mechanical rename. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19064#discussion_r1588015870
