RFR: 8299614: Shenandoah: STW mark should keep nmethod/oops referenced from stack chunk alive [v2]

[Aleksey Shipilev]

On Mon, 11 Sep 2023 22:08:53 GMT, William Kemper <wkemper at openjdk.org> wrote:

> I'm not suggesting we keep `ShenandoahNMethodBarrier` flag, but for the sake of my own understanding: the passive mode does not need the nmethod barriers, correct?

Passive needs nmethod barriers, because STW mark in passive mode needs to visit stackChunks, which in current code requires nmethod barriers to be armed to get them to visit the stackChunk's nmethods. Read through `ShenandoahMarkRefsSuperClosure::do_nmethod`. The reproducer (see bug) fails with passive too.

It is an unfortunate dependency, which we can untie by making `ShenandoahMarkRefsSuperClosure::do_nmethod` scan the nmethods directly when nmethod barriers are disabled, like this:


diff --git a/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp
index 1c8daba3d24..fe00fe72220 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp
@@ -71,6 +71,7 @@ BoolObjectClosure* ShenandoahIsAliveSelector::is_alive_closure() {
 
 void ShenandoahOopClosureBase::do_nmethod(nmethod* nm) {
   nm->run_nmethod_entry_barrier();
+  nm->follow_nmethod(this);
 }
 
 ShenandoahKeepAliveClosure::ShenandoahKeepAliveClosure() :
diff --git a/src/hotspot/share/gc/shenandoah/shenandoahOopClosures.hpp b/src/hotspot/share/gc/shenandoah/shenandoahOopClosures.hpp
index 11c70f2726a..44f06a26684 100644
--- a/src/hotspot/share/gc/shenandoah/shenandoahOopClosures.hpp
+++ b/src/hotspot/share/gc/shenandoah/shenandoahOopClosures.hpp
@@ -63,6 +63,7 @@ class ShenandoahMarkRefsSuperClosure : public MetadataVisitingOopIterateClosure
   virtual void do_nmethod(nmethod* nm) {
     assert(!is_weak(), "Can't handle weak marking of nmethods");
     nm->run_nmethod_entry_barrier();
+    nm->follow_nmethod(this);
   }
 };


But IMO that is riskier than just delegating to nmethod barriers, like concurrent GC does. ...and it would probably do double work when nmethod barriers are used. 

Further, `ShenandoahNMethodBarrier` is kinda misnomer, because most of the `Shenandoah*Barrier` flags are dealing with emitting the mutator-side barriers. But `ShenandoahNMethodBarrier`, as evident from this PR, only guards arming the already emitted barriers in some conditions. So, counter to other `Shenandoah*Barrier` flags, the nmethod barriers are still emitted in the generated code by interpreter and compilers even with `-ShenandoahNMethodBarrier`. So, this PR resolves this oddity as well.

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

PR Comment: https://git.openjdk.org/jdk/pull/15669#issuecomment-1715052134