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

[Stefan Karlsson]

On Thu, 2 May 2024 17:23:21 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> 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.

Sounds like a good idea.

>> 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.

Ah, I see. I wouldn't mind if it were changed to include "pause", but I'm also OK with you leaving it as is.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19064#discussion_r1588019866
PR Review Comment: https://git.openjdk.org/jdk/pull/19064#discussion_r1588018382