8331911: Reconsider locking for recently disarmed nmethods

Reviewed-by: shade, eosterlund

src/hotspot/share/code/nmethod.cpp

@@ -844,10 +844,8 @@ void nmethod::run_nmethod_entry_barrier() {
     // By calling this nmethod entry barrier, it plays along and acts
     // like any other nmethod found on the stack of a thread (fewer surprises).
     nmethod* nm = this;
-    if (bs_nm->is_armed(nm)) {
-      bool alive = bs_nm->nmethod_entry_barrier(nm);
-      assert(alive, "should be alive");
-    }
+    bool alive = bs_nm->nmethod_entry_barrier(nm);
+    assert(alive, "should be alive");
   }
 }
 

src/hotspot/share/gc/shared/barrierSetNMethod.cpp

@@ -100,6 +100,12 @@ bool BarrierSetNMethod::nmethod_entry_barrier(nmethod* nm) {
     virtual void do_oop(narrowOop* p) { ShouldNotReachHere(); }
   };
 
+  if (!is_armed(nm)) {
+    // Some other thread got here first and healed the oops
+    // and disarmed the nmethod. No need to continue.
+    return true;
+  }
+
   // If the nmethod is the only thing pointing to the oops, and we are using a
   // SATB GC, then it is important that this code marks them live.
   // Also, with concurrent GC, it is possible that frames in continuation stack
@@ -172,6 +178,9 @@ int BarrierSetNMethod::nmethod_stub_entry_barrier(address* return_address_ptr) {
   nmethod* nm = cb->as_nmethod();
   BarrierSetNMethod* bs_nm = BarrierSet::barrier_set()->barrier_set_nmethod();
 
+  // Check for disarmed method here to avoid going into DeoptimizeNMethodBarriersALot code
+  // too often. nmethod_entry_barrier checks for disarmed status itself,
+  // but we have no visibility into whether the barrier acted or not.
   if (!bs_nm->is_armed(nm)) {
     return 0;
   }

src/hotspot/share/gc/x/xBarrierSetNMethod.cpp

@@ -32,12 +32,18 @@
 #include "runtime/threadWXSetters.inline.hpp"
 
 bool XBarrierSetNMethod::nmethod_entry_barrier(nmethod* nm) {
+  if (!is_armed(nm)) {
+    // Some other thread got here first and healed the oops
+    // and disarmed the nmethod. No need to continue.
+    return true;
+  }
+
   XLocker<XReentrantLock> locker(XNMethod::lock_for_nmethod(nm));
   log_trace(nmethod, barrier)("Entered critical zone for %p", nm);
 
   if (!is_armed(nm)) {
-    // Some other thread got here first and healed the oops
-    // and disarmed the nmethod.
+    // Some other thread managed to complete while we were
+    // waiting for lock. No need to continue.
     return true;
   }
 

src/hotspot/share/gc/z/zBarrierSetNMethod.cpp

@@ -37,6 +37,13 @@
 #include "runtime/threadWXSetters.inline.hpp"
 
 bool ZBarrierSetNMethod::nmethod_entry_barrier(nmethod* nm) {
+  if (!is_armed(nm)) {
+    log_develop_trace(gc, nmethod)("nmethod: " PTR_FORMAT " visited by entry (disarmed before lock)", p2i(nm));
+    // Some other thread got here first and healed the oops
+    // and disarmed the nmethod. No need to continue.
+    return true;
+  }
+
   ZLocker<ZReentrantLock> locker(ZNMethod::lock_for_nmethod(nm));
   log_trace(nmethod, barrier)("Entered critical zone for %p", nm);
 
@@ -44,8 +51,8 @@ bool ZBarrierSetNMethod::nmethod_entry_barrier(nmethod* nm) {
 
   if (!is_armed(nm)) {
     log_develop_trace(gc, nmethod)("nmethod: " PTR_FORMAT " visited by entry (disarmed)", p2i(nm));
-    // Some other thread got here first and healed the oops
-    // and disarmed the nmethod.
+    // Some other thread managed to complete while we were
+    // waiting for lock. No need to continue.
     return true;
   }