From 094539c4e86020043cdb15e7bb58c33dac20ea89 Mon Sep 17 00:00:00 2001 From: dcubed Date: Wed, 12 Mar 2008 18:09:34 -0700 Subject: [PATCH] 6453355: 4/4 new No_Safepoint_Verifier uses fail during GC Summary: (for Serguei) Clean up use of No_Safepoint_Verifier in JVM TI Reviewed-by: dcubed --- src/share/vm/oops/instanceKlass.cpp | 69 ++++++++++++++++------------- src/share/vm/oops/instanceKlass.hpp | 2 + src/share/vm/runtime/thread.cpp | 8 ++-- 3 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/share/vm/oops/instanceKlass.cpp b/src/share/vm/oops/instanceKlass.cpp index ea85a6c63..74b60274f 100644 --- a/src/share/vm/oops/instanceKlass.cpp +++ b/src/share/vm/oops/instanceKlass.cpp @@ -950,7 +950,6 @@ jmethodID instanceKlass::jmethod_id_for_impl(instanceKlassHandle ik_h, methodHan // These allocations will have to be freed if they are unused. // Allocate a new array of methods. - jmethodID* to_dealloc_jmeths = NULL; jmethodID* new_jmeths = NULL; if (length <= idnum) { // A new array will be needed (unless some other thread beats us to it) @@ -961,7 +960,6 @@ jmethodID instanceKlass::jmethod_id_for_impl(instanceKlassHandle ik_h, methodHan } // Allocate a new method ID. - jmethodID to_dealloc_id = NULL; jmethodID new_id = NULL; if (method_h->is_old() && !method_h->is_obsolete()) { // The method passed in is old (but not obsolete), we need to use the current version @@ -975,40 +973,51 @@ jmethodID instanceKlass::jmethod_id_for_impl(instanceKlassHandle ik_h, methodHan new_id = JNIHandles::make_jmethod_id(method_h); } - { + if (Threads::number_of_threads() == 0 || SafepointSynchronize::is_at_safepoint()) { + // No need and unsafe to lock the JmethodIdCreation_lock at safepoint. + id = get_jmethod_id(ik_h, idnum, new_id, new_jmeths); + } else { MutexLocker ml(JmethodIdCreation_lock); + id = get_jmethod_id(ik_h, idnum, new_id, new_jmeths); + } + } + return id; +} - // We must not go to a safepoint while holding this lock. - debug_only(No_Safepoint_Verifier nosafepoints;) - // Retry lookup after we got the lock - jmeths = ik_h->methods_jmethod_ids_acquire(); - if (jmeths == NULL || (length = (size_t)jmeths[0]) <= idnum) { - if (jmeths != NULL) { - // We have grown the array: copy the existing entries, and delete the old array - for (size_t index = 0; index < length; index++) { - new_jmeths[index+1] = jmeths[index+1]; - } - to_dealloc_jmeths = jmeths; // using the new jmeths, deallocate the old one - } - ik_h->release_set_methods_jmethod_ids(jmeths = new_jmeths); - } else { - id = jmeths[idnum+1]; - to_dealloc_jmeths = new_jmeths; // using the old jmeths, deallocate the new one - } - if (id == NULL) { - id = new_id; - jmeths[idnum+1] = id; // install the new method ID - } else { - to_dealloc_id = new_id; // the new id wasn't used, mark it for deallocation +jmethodID instanceKlass::get_jmethod_id(instanceKlassHandle ik_h, size_t idnum, + jmethodID new_id, jmethodID* new_jmeths) { + // Retry lookup after we got the lock or ensured we are at safepoint + jmethodID* jmeths = ik_h->methods_jmethod_ids_acquire(); + jmethodID id = NULL; + jmethodID to_dealloc_id = NULL; + jmethodID* to_dealloc_jmeths = NULL; + size_t length; + + if (jmeths == NULL || (length = (size_t)jmeths[0]) <= idnum) { + if (jmeths != NULL) { + // We have grown the array: copy the existing entries, and delete the old array + for (size_t index = 0; index < length; index++) { + new_jmeths[index+1] = jmeths[index+1]; } + to_dealloc_jmeths = jmeths; // using the new jmeths, deallocate the old one } + ik_h->release_set_methods_jmethod_ids(jmeths = new_jmeths); + } else { + id = jmeths[idnum+1]; + to_dealloc_jmeths = new_jmeths; // using the old jmeths, deallocate the new one + } + if (id == NULL) { + id = new_id; + jmeths[idnum+1] = id; // install the new method ID + } else { + to_dealloc_id = new_id; // the new id wasn't used, mark it for deallocation + } - // Free up unneeded or no longer needed resources - FreeHeap(to_dealloc_jmeths); - if (to_dealloc_id != NULL) { - JNIHandles::destroy_jmethod_id(to_dealloc_id); - } + // Free up unneeded or no longer needed resources + FreeHeap(to_dealloc_jmeths); + if (to_dealloc_id != NULL) { + JNIHandles::destroy_jmethod_id(to_dealloc_id); } return id; } diff --git a/src/share/vm/oops/instanceKlass.hpp b/src/share/vm/oops/instanceKlass.hpp index 478e8ecc1..285291dce 100644 --- a/src/share/vm/oops/instanceKlass.hpp +++ b/src/share/vm/oops/instanceKlass.hpp @@ -432,6 +432,8 @@ class instanceKlass: public Klass { _enclosing_method_method_index = method_index; } // jmethodID support + static jmethodID get_jmethod_id(instanceKlassHandle ik_h, size_t idnum, + jmethodID new_id, jmethodID* new_jmeths); static jmethodID jmethod_id_for_impl(instanceKlassHandle ik_h, methodHandle method_h); jmethodID jmethod_id_or_null(methodOop method); diff --git a/src/share/vm/runtime/thread.cpp b/src/share/vm/runtime/thread.cpp index 607772a02..3667e576d 100644 --- a/src/share/vm/runtime/thread.cpp +++ b/src/share/vm/runtime/thread.cpp @@ -1317,10 +1317,6 @@ JavaThread::~JavaThread() { ThreadSafepointState::destroy(this); if (_thread_profiler != NULL) delete _thread_profiler; if (_thread_stat != NULL) delete _thread_stat; - - if (jvmti_thread_state() != NULL) { - JvmtiExport::cleanup_thread(this); - } } @@ -1571,6 +1567,10 @@ void JavaThread::exit(bool destroy_vm, ExitType exit_type) { tlab().make_parsable(true); // retire TLAB } + if (jvmti_thread_state() != NULL) { + JvmtiExport::cleanup_thread(this); + } + // Remove from list of active threads list, and notify VM thread if we are the last non-daemon thread Threads::remove(this); } -- GitLab