Comments from Jorn

JornVernee · Nov 23, 2023 · 5d34ddb · 5d34ddb
1 parent d12fa90
commit 5d34ddb
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 47 deletions.
diff --git a/src/hotspot/share/prims/scopedMemoryAccess.cpp b/src/hotspot/share/prims/scopedMemoryAccess.cpp
@@ -35,38 +35,6 @@
 #include "runtime/sharedRuntime.hpp"
 #include "runtime/vframe.inline.hpp"
 
-class CloseScopedMemoryFindOopClosure : public OopClosure {
-  oop _deopt;
-  bool _found;
-
-public:
-  CloseScopedMemoryFindOopClosure(jobject deopt) :
-      _deopt(JNIHandles::resolve(deopt)),
-      _found(false) {}
-
-  template <typename T>
-  void do_oop_work(T* p) {
-    if (_found) {
-      return;
-    }
-    if (RawAccess<>::oop_load(p) == _deopt) {
-      _found = true;
-    }
-  }
-
-  virtual void do_oop(oop* p) {
-    do_oop_work(p);
-  }
-
-  virtual void do_oop(narrowOop* p) {
-    do_oop_work(p);
-  }
-
-  bool found() {
-    return _found;
-  }
-};
-
 static bool is_in_scoped_access(JavaThread* jt, oop session) {
   const int max_critical_stack_depth = 10;
   int depth = 0;
@@ -147,17 +115,10 @@ class CloseScopedMemoryClosure : public HandshakeClosure {
     }
 
     ResourceMark rm;
-    if (_session != nullptr && last_frame.is_compiled_frame() && last_frame.can_be_deoptimized()) {
-      CloseScopedMemoryFindOopClosure cl(_session);
-      CompiledMethod* cm = last_frame.cb()->as_compiled_method();
-
-      /* FIXME: this doesn't work if reachability fences are violated by C2
-      last_frame.oops_do(&cl, nullptr, &register_map);
-      if (cl.found()) {
-           //Found the deopt oop in a compiled method; deoptimize.
-           Deoptimization::deoptimize(jt, last_frame);
-      }
-      so... we unconditionally deoptimize, for now: */
+    if (last_frame.is_compiled_frame() && last_frame.can_be_deoptimized()) {
+      // FIXME: we would like to conditionally deoptimize only if the corresponding
+      // _session is reachable from the frame, but reachabilityFence doesn't currently
+      // work the way it should. Therefore we deopt unconditionally for now.
       Deoptimization::deoptimize(jt, last_frame);
     }
 

diff --git a/src/java.base/share/classes/jdk/internal/foreign/MemorySessionImpl.java b/src/java.base/share/classes/jdk/internal/foreign/MemorySessionImpl.java
@@ -60,7 +60,7 @@ public abstract sealed class MemorySessionImpl
     static final VarHandle STATE;
     static final int MAX_FORKS = Integer.MAX_VALUE;
 
-    public static final ScopedMemoryAccess.ScopedAccessError ALREADY_CLOSED = new ScopedMemoryAccess.ScopedAccessError(MemorySessionImpl::alreadyClosed);
+    static final ScopedMemoryAccess.ScopedAccessError ALREADY_CLOSED = new ScopedMemoryAccess.ScopedAccessError(MemorySessionImpl::alreadyClosed);
     static final ScopedMemoryAccess.ScopedAccessError WRONG_THREAD = new ScopedMemoryAccess.ScopedAccessError(MemorySessionImpl::wrongThread);
     // This is the session of all zero-length memory segments
     public static final MemorySessionImpl GLOBAL_SESSION = new GlobalSession();

diff --git a/src/java.base/share/classes/jdk/internal/foreign/SharedSession.java b/src/java.base/share/classes/jdk/internal/foreign/SharedSession.java
@@ -83,7 +83,7 @@ void justClose() {
         } else if (prevState != OPEN) {
             throw alreadyAcquired(prevState);
         }
-        SCOPED_MEMORY_ACCESS.closeScope(this);
+        SCOPED_MEMORY_ACCESS.closeScope(this, ALREADY_CLOSED);
     }
 
     /**

diff --git a/src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template b/src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template
@@ -83,8 +83,8 @@ public class ScopedMemoryAccess {
         registerNatives();
     }
 
-    public void closeScope(MemorySessionImpl session) {
-        closeScope0(session, MemorySessionImpl.ALREADY_CLOSED);
+    public void closeScope(MemorySessionImpl session, ScopedAccessError error) {
+        closeScope0(session, error);
     }
 
     native void closeScope0(MemorySessionImpl session, ScopedAccessError error);