diff --git a/src/hotspot/share/oops/instanceKlass.cpp b/src/hotspot/share/oops/instanceKlass.cpp index d7a4c653726..7432a607d3b 100644 --- a/src/hotspot/share/oops/instanceKlass.cpp +++ b/src/hotspot/share/oops/instanceKlass.cpp @@ -2148,15 +2148,8 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) { // the cache can't grow so we can just get the current values get_jmethod_id_length_value(jmeths, idnum, &length, &id); } else { - // cache can grow so we have to be more careful - if (Threads::number_of_threads() == 0 || - SafepointSynchronize::is_at_safepoint()) { - // we're single threaded or at a safepoint - no locking needed - get_jmethod_id_length_value(jmeths, idnum, &length, &id); - } else { - MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); - get_jmethod_id_length_value(jmeths, idnum, &length, &id); - } + MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); + get_jmethod_id_length_value(jmeths, idnum, &length, &id); } } // implied else: @@ -2166,8 +2159,8 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) { length <= idnum || // cache is too short id == NULL) { // cache doesn't contain entry - // This function can be called by the VMThread so we have to do all - // things that might block on a safepoint before grabbing the lock. + // This function can be called by the VMThread or GC worker threads so we + // have to do all things that might block on a safepoint before grabbing the lock. // Otherwise, we can deadlock with the VMThread or have a cache // consistency issue. These vars keep track of what we might have // to free after the lock is dropped. @@ -2186,25 +2179,20 @@ jmethodID InstanceKlass::get_jmethod_id(const methodHandle& method_h) { } // allocate a new jmethodID that might be used - 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 - Method* current_method = method_with_idnum((int)idnum); - assert(current_method != NULL, "old and but not obsolete, so should exist"); - new_id = Method::make_jmethod_id(class_loader_data(), current_method); - } else { - // It is the current version of the method or an obsolete method, - // use the version passed in - new_id = Method::make_jmethod_id(class_loader_data(), method_h()); - } - - if (Threads::number_of_threads() == 0 || - SafepointSynchronize::is_at_safepoint()) { - // we're single threaded or at a safepoint - no locking needed - id = get_jmethod_id_fetch_or_update(idnum, new_id, new_jmeths, - &to_dealloc_id, &to_dealloc_jmeths); - } else { + { MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); + 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 + Method* current_method = method_with_idnum((int)idnum); + assert(current_method != NULL, "old and but not obsolete, so should exist"); + new_id = Method::make_jmethod_id(class_loader_data(), current_method); + } else { + // It is the current version of the method or an obsolete method, + // use the version passed in + new_id = Method::make_jmethod_id(class_loader_data(), method_h()); + } + id = get_jmethod_id_fetch_or_update(idnum, new_id, new_jmeths, &to_dealloc_id, &to_dealloc_jmeths); } @@ -2254,9 +2242,7 @@ jmethodID InstanceKlass::get_jmethod_id_fetch_or_update( assert(new_id != NULL, "sanity check"); assert(to_dealloc_id_p != NULL, "sanity check"); assert(to_dealloc_jmeths_p != NULL, "sanity check"); - assert(Threads::number_of_threads() == 0 || - SafepointSynchronize::is_at_safepoint() || - JmethodIdCreation_lock->owned_by_self(), "sanity check"); + assert(JmethodIdCreation_lock->owned_by_self(), "sanity check"); // reacquire the cache - we are locked, single threaded or at a safepoint jmethodID* jmeths = methods_jmethod_ids_acquire(); diff --git a/src/hotspot/share/oops/method.cpp b/src/hotspot/share/oops/method.cpp index 39ba8f395bb..c8b9409ddaa 100644 --- a/src/hotspot/share/oops/method.cpp +++ b/src/hotspot/share/oops/method.cpp @@ -2165,50 +2165,29 @@ JNIMethodBlockNode::JNIMethodBlockNode(int num_methods) : _top(0), _next(NULL) { } } -void Method::ensure_jmethod_ids(ClassLoaderData* loader_data, int capacity) { - ClassLoaderData* cld = loader_data; - if (!SafepointSynchronize::is_at_safepoint()) { - // Have to add jmethod_ids() to class loader data thread-safely. - // Also have to add the method to the list safely, which the lock - // protects as well. - MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); - if (cld->jmethod_ids() == NULL) { - cld->set_jmethod_ids(new JNIMethodBlock(capacity)); - } else { - cld->jmethod_ids()->ensure_methods(capacity); - } +void Method::ensure_jmethod_ids(ClassLoaderData* cld, int capacity) { + // Have to add jmethod_ids() to class loader data thread-safely. + // Also have to add the method to the list safely, which the lock + // protects as well. + MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); + if (cld->jmethod_ids() == NULL) { + cld->set_jmethod_ids(new JNIMethodBlock(capacity)); } else { - // At safepoint, we are single threaded and can set this. - if (cld->jmethod_ids() == NULL) { - cld->set_jmethod_ids(new JNIMethodBlock(capacity)); - } else { - cld->jmethod_ids()->ensure_methods(capacity); - } + cld->jmethod_ids()->ensure_methods(capacity); } } // Add a method id to the jmethod_ids -jmethodID Method::make_jmethod_id(ClassLoaderData* loader_data, Method* m) { - ClassLoaderData* cld = loader_data; - - if (!SafepointSynchronize::is_at_safepoint()) { - // Have to add jmethod_ids() to class loader data thread-safely. - // Also have to add the method to the list safely, which the lock - // protects as well. - MutexLocker ml(JmethodIdCreation_lock, Mutex::_no_safepoint_check_flag); - if (cld->jmethod_ids() == NULL) { - cld->set_jmethod_ids(new JNIMethodBlock()); - } - // jmethodID is a pointer to Method* - return (jmethodID)cld->jmethod_ids()->add_method(m); - } else { - // At safepoint, we are single threaded and can set this. - if (cld->jmethod_ids() == NULL) { - cld->set_jmethod_ids(new JNIMethodBlock()); - } - // jmethodID is a pointer to Method* - return (jmethodID)cld->jmethod_ids()->add_method(m); +jmethodID Method::make_jmethod_id(ClassLoaderData* cld, Method* m) { + // Have to add jmethod_ids() to class loader data thread-safely. + // Also have to add the method to the list safely, which the lock + // protects as well. + assert(JmethodIdCreation_lock->owned_by_self(), "sanity check"); + if (cld->jmethod_ids() == NULL) { + cld->set_jmethod_ids(new JNIMethodBlock()); } + // jmethodID is a pointer to Method* + return (jmethodID)cld->jmethod_ids()->add_method(m); } jmethodID Method::jmethod_id() { @@ -2218,8 +2197,7 @@ jmethodID Method::jmethod_id() { // Mark a jmethodID as free. This is called when there is a data race in // InstanceKlass while creating the jmethodID cache. -void Method::destroy_jmethod_id(ClassLoaderData* loader_data, jmethodID m) { - ClassLoaderData* cld = loader_data; +void Method::destroy_jmethod_id(ClassLoaderData* cld, jmethodID m) { Method** ptr = (Method**)m; assert(cld->jmethod_ids() != NULL, "should have method handles"); cld->jmethod_ids()->destroy_method(ptr); diff --git a/src/hotspot/share/oops/method.hpp b/src/hotspot/share/oops/method.hpp index 1fc28c86df3..4a5df8134cc 100644 --- a/src/hotspot/share/oops/method.hpp +++ b/src/hotspot/share/oops/method.hpp @@ -774,13 +774,13 @@ public: // however, can be GC'ed away if the class is unloaded or if the method is // made obsolete or deleted -- in these cases, the jmethodID // refers to NULL (as is the case for any weak reference). - static jmethodID make_jmethod_id(ClassLoaderData* loader_data, Method* mh); - static void destroy_jmethod_id(ClassLoaderData* loader_data, jmethodID mid); + static jmethodID make_jmethod_id(ClassLoaderData* cld, Method* mh); + static void destroy_jmethod_id(ClassLoaderData* cld, jmethodID mid); // Ensure there is enough capacity in the internal tracking data // structures to hold the number of jmethodIDs you plan to generate. // This saves substantial time doing allocations. - static void ensure_jmethod_ids(ClassLoaderData* loader_data, int capacity); + static void ensure_jmethod_ids(ClassLoaderData* cld, int capacity); // Use resolve_jmethod_id() in situations where the caller is expected // to provide a valid jmethodID; the only sanity checks are in asserts;