RFR: 8300915: G1: incomplete SATB because nmethod entry barriers don't get armed

[Richard Reingruber]

On Wed, 25 Jan 2023 13:32:27 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

> The change makes sure all nmethod entry barriers are armed when a G1 concurrent marking cycle starts by doing it unconditionally in `G1ConcurrentMark::pre_concurrent_start()`. The barrier is needed to add oop constants to the SATB.
> 
> This alone would be a conservative/minimal fix but I didn't like that `CodeCache::is_gc_marking_cycle_active()` can return true after a marking cycle start is undone. I.e. `G1ConcurrentMarkThread::_state == Idle && CodeCache::is_gc_marking_cycle_active()` is possible.
> Edit: It felt like an inconsistency but maybe its actually ok not to finish the CC marking cycle when the G1 marking is undone. Please comment...
> The changes in `G1ConcurrentMark::post_concurrent_undo_start()` were made to avoid it.
> 
> 
> Testing
> 
> * Rebased this PR onto [JDK-8297487](https://bugs.openjdk.org/browse/JDK-8297487), replaced related assertions with guarantees, and then executed test/langtools:tier1 in a loop for 12h without seeing the issues reported in [JDK-8299956](https://bugs.openjdk.org/browse/JDK-8299956).
> 
> * CI testing at SAP: This includes most JCK and JTREG tiers 1-4, also in Xcomp mode, on the standard platforms and also on ppc64le.
> 
> * GHA: build failures because download of apache ant failed. `serviceability/jvmti/vthread/SuspendResumeAll/SuspendResumeAll.java` had a timeout on windows-x64. Should be unrelated. It succeeded in our CI testing.

src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 3377:

> 3375:     // will be conservative and use the last code marking cycle. Code
> 3376:     // caches marked between the two concurrent marks will live a bit
> 3377:     // longer than needed.

I removed the comment because I found it a little missleading. If "concurrent marking" is G1 cm then we never reach here with an ongoing concurrent marking. Reaching here G1 cm was either finished normally, undone or aborted. But maybe "concurrent marking" means marking of nmethods?

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

PR: https://git.openjdk.org/jdk/pull/12194