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

Richard Reingruber rrich at openjdk.org
Thu Jan 26 20:48:20 UTC 2023


On Thu, 26 Jan 2023 20:29:25 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.
>
> Richard Reingruber has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Remark: only finish codecache cycle if marking was finished
>  - Conservative fix: arm nmethods unconditionally at marking start

I've pushed an "extended" conservative fix that does not increment `CodeCache::_gc_epoch` when g1 concurrent marking is undone or aborted. But it always arms nmethods before concurrent marking starts (3074787aa1c4c4986832b489367d1ae35bf2b24f).

I noticed that the code cache cycle was finished in `G1ConcurrentMark::remark()` even if marking was not finished because of marking stack overflow. Marking is restarted in this case (after root region scan). Upon the second `remark()` attempt an assertion should fire when calling `CodeCache::on_gc_marking_cycle_finish()` because it was finished before. In the release build this could make nmethods with frames in continuations look as if they didn't have any frames. I fix this too (da2162f6ceb5a25e9b7ea7b99fda1e3c9b755364).

I'm testing if this version still fixes https://bugs.openjdk.org/browse/JDK-8299956

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

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


More information about the hotspot-gc-dev mailing list