changeset 55991:186646672fb4 cont

Cleanup for keepalive handles.
author rbackman
date Fri, 19 Jul 2019 11:10:02 +0200
parents e52583e674ae
children 423f4ecbf972 3d74bc8cc177
files src/hotspot/share/runtime/continuation.cpp src/hotspot/share/runtime/continuation.hpp src/hotspot/share/runtime/safepoint.cpp src/hotspot/share/runtime/safepoint.hpp src/hotspot/share/runtime/thread.cpp src/hotspot/share/runtime/thread.hpp
diffstat 6 files changed, 52 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/runtime/continuation.cpp	Thu Jul 18 17:45:09 2019 +0100
+++ b/src/hotspot/share/runtime/continuation.cpp	Fri Jul 19 11:10:02 2019 +0200
@@ -1370,16 +1370,18 @@
     }
 
     if (shadow != NULL) {
+      //log_info(jvmcont)("trying to clear stale shadow for %p", _method);
       if (cm->clear_shadow(shadow)) {
-        // TODO: We are currently leaking handles here. We can't just delete it straight away because someone else might be looking at it.
-        // The right thing to do would be to put it on a list and clear them at safepoint or similiar.
+        //log_info(jvmcont)("shadow cleared for %p", _method);
+        thread->keepalive_cleanup()->append(shadow);
+        // put on a list for cleanup in a safepoint
       }
     }
 
     nmethod* nm = cm->as_nmethod_or_null();
     if (nm != NULL) {
       _nr_oops = nm->nr_oops();
-      //log_info(jvmcont)("need shadow for %d oops", _nr_oops());
+      //log_info(jvmcont)("need shadow for %d oops", _nr_oops);
       _required = true;
       _parent = parent;
     }
@@ -4088,6 +4090,35 @@
   ConfigResolve::resolve();
 }
 
+class KeepaliveCleanupClosure : public ThreadClosure {
+private:
+  int _count;
+public:
+  KeepaliveCleanupClosure() : _count(0) {}
+
+  int count() const { return _count; }
+
+  virtual void do_thread(Thread* thread) {
+    JavaThread* jthread = (JavaThread*) thread;
+    GrowableArray<jweak>* cleanup_list = jthread->keepalive_cleanup();
+    int len = cleanup_list->length();
+    _count += len;
+    for (int i = 0; i < len; ++i) {
+      jweak ref = cleanup_list->at(i);
+      JNIHandles::destroy_weak_global(ref);
+    }
+
+    cleanup_list->clear();
+    assert(cleanup_list->length() == 0, "should be clean");
+  }
+};
+
+void Continuations::cleanup_keepalives() {
+  KeepaliveCleanupClosure closure;
+  Threads::java_threads_do(&closure);
+  //log_info(jvmcont)("cleanup %d refs", closure.count());
+}
+
 volatile intptr_t Continuations::_exploded_miss = 0;
 volatile intptr_t Continuations::_exploded_hit = 0;
 volatile intptr_t Continuations::_nmethod_miss = 0;
--- a/src/hotspot/share/runtime/continuation.hpp	Thu Jul 18 17:45:09 2019 +0100
+++ b/src/hotspot/share/runtime/continuation.hpp	Fri Jul 19 11:10:02 2019 +0200
@@ -56,6 +56,8 @@
   static void print_statistics();
   static void init();
 
+  static void cleanup_keepalives();
+
   static int flags() { return _flags; }
 };
 
--- a/src/hotspot/share/runtime/safepoint.cpp	Thu Jul 18 17:45:09 2019 +0100
+++ b/src/hotspot/share/runtime/safepoint.cpp	Fri Jul 19 11:10:02 2019 +0200
@@ -47,6 +47,7 @@
 #include "oops/symbol.hpp"
 #include "runtime/atomic.hpp"
 #include "runtime/compilationPolicy.hpp"
+#include "runtime/continuation.hpp"
 #include "runtime/deoptimization.hpp"
 #include "runtime/frame.inline.hpp"
 #include "runtime/handles.inline.hpp"
@@ -631,6 +632,15 @@
       }
     }
 
+    if (_subtasks.try_claim_task(SafepointSynchronize::SAFEPOINT_CLEANUP_KEEPALIVES)) {
+      const char* name = "cleaning keepalive jweak handles";
+      EventSafepointCleanupTask event;
+      TraceTime timer(name, TRACETIME_LOG(Info, safepoint, cleanup));
+      Continuations::cleanup_keepalives();
+
+      post_safepoint_cleanup_task_event(event, safepoint_id, name);
+    }
+
     _subtasks.all_tasks_completed(_num_workers);
   }
 };
--- a/src/hotspot/share/runtime/safepoint.hpp	Thu Jul 18 17:45:09 2019 +0100
+++ b/src/hotspot/share/runtime/safepoint.hpp	Fri Jul 19 11:10:02 2019 +0200
@@ -69,6 +69,7 @@
     SAFEPOINT_CLEANUP_STRING_TABLE_REHASH,
     SAFEPOINT_CLEANUP_CLD_PURGE,
     SAFEPOINT_CLEANUP_SYSTEM_DICTIONARY_RESIZE,
+    SAFEPOINT_CLEANUP_KEEPALIVES,
     // Leave this one last.
     SAFEPOINT_CLEANUP_NUM_TASKS
   };
--- a/src/hotspot/share/runtime/thread.cpp	Thu Jul 18 17:45:09 2019 +0100
+++ b/src/hotspot/share/runtime/thread.cpp	Fri Jul 19 11:10:02 2019 +0200
@@ -1682,6 +1682,7 @@
   set_vframe_array_head(NULL);
   set_vframe_array_last(NULL);
   set_deferred_locals(NULL);
+  set_keepalive_cleanup(new (ResourceObj::C_HEAP, mtInternal) GrowableArray<jweak>(16, true));
   set_deopt_mark(NULL);
   set_deopt_compiled_method(NULL);
   clear_must_deopt_id();
--- a/src/hotspot/share/runtime/thread.hpp	Thu Jul 18 17:45:09 2019 +0100
+++ b/src/hotspot/share/runtime/thread.hpp	Fri Jul 19 11:10:02 2019 +0200
@@ -1029,6 +1029,7 @@
   // This holds the pointer to array (yeah like there might be more than one) of
   // description of compiled vframes that have locals that need to be updated.
   GrowableArray<jvmtiDeferredLocalVariableSet*>* _deferred_locals_updates;
+  GrowableArray<jweak>* _keepalive_cleanup;
 
   // Handshake value for fixing 6243940. We need a place for the i2c
   // adapter to store the callee Method*. This value is NEVER live
@@ -1522,6 +1523,9 @@
   GrowableArray<jvmtiDeferredLocalVariableSet*>* deferred_locals() const { return _deferred_locals_updates; }
   void set_deferred_locals(GrowableArray<jvmtiDeferredLocalVariableSet *>* vf) { _deferred_locals_updates = vf; }
 
+  void set_keepalive_cleanup(GrowableArray<jweak>* lst) { _keepalive_cleanup = lst; }
+  GrowableArray<jweak>* keepalive_cleanup() const { return _keepalive_cleanup; }
+
   // These only really exist to make debugging deopt problems simpler
 
   void set_vframe_array_last(vframeArray* value) { _vframe_array_last = value; }