# HG changeset patch # User coleenp # Date 1535209821 14400 # Node ID 1f0b605bdc2821d09aaf6c37ee71c23611566d1f # Parent a716460217ed180972b568e28cf332e37e07a3ae 8209821: Make JVMTI GetClassLoaderClasses not walk CLDG Summary: And also added function with KlassClosure to remove the hacks. Reviewed-by: lfoltan, sspitsyn diff -r a716460217ed -r 1f0b605bdc28 src/hotspot/share/classfile/classLoaderData.cpp --- a/src/hotspot/share/classfile/classLoaderData.cpp Sat Aug 25 14:23:21 2018 +0200 +++ b/src/hotspot/share/classfile/classLoaderData.cpp Sat Aug 25 11:10:21 2018 -0400 @@ -1254,15 +1254,6 @@ } } -// Walks all entries in the dictionary including entries initiated by this class loader. -void ClassLoaderDataGraph::dictionary_all_entries_do(void f(InstanceKlass*, ClassLoaderData*)) { - Thread* thread = Thread::current(); - FOR_ALL_DICTIONARY(cld) { - Handle holder(thread, cld->holder_phantom()); - cld->dictionary()->all_entries_do(f); - } -} - void ClassLoaderDataGraph::verify_dictionary() { FOR_ALL_DICTIONARY(cld) { cld->dictionary()->verify(); diff -r a716460217ed -r 1f0b605bdc28 src/hotspot/share/classfile/classLoaderData.hpp --- a/src/hotspot/share/classfile/classLoaderData.hpp Sat Aug 25 14:23:21 2018 +0200 +++ b/src/hotspot/share/classfile/classLoaderData.hpp Sat Aug 25 11:10:21 2018 -0400 @@ -134,9 +134,6 @@ // Added for initialize_itable_for_klass to handle exceptions. static void dictionary_classes_do(void f(InstanceKlass*, TRAPS), TRAPS); - // Iterate all classes and their class loaders, including initiating class loaders. - static void dictionary_all_entries_do(void f(InstanceKlass*, ClassLoaderData*)); - // VM_CounterDecay iteration support static InstanceKlass* try_get_next_class(); diff -r a716460217ed -r 1f0b605bdc28 src/hotspot/share/classfile/dictionary.cpp --- a/src/hotspot/share/classfile/dictionary.cpp Sat Aug 25 14:23:21 2018 +0200 +++ b/src/hotspot/share/classfile/dictionary.cpp Sat Aug 25 11:10:21 2018 -0400 @@ -330,13 +330,13 @@ } // All classes, and their class loaders, including initiating class loaders -void Dictionary::all_entries_do(void f(InstanceKlass*, ClassLoaderData*)) { +void Dictionary::all_entries_do(KlassClosure* closure) { for (int index = 0; index < table_size(); index++) { for (DictionaryEntry* probe = bucket(index); probe != NULL; probe = probe->next()) { InstanceKlass* k = probe->instance_klass(); - f(k, loader_data()); + closure->do_klass(k); } } } diff -r a716460217ed -r 1f0b605bdc28 src/hotspot/share/classfile/dictionary.hpp --- a/src/hotspot/share/classfile/dictionary.hpp Sat Aug 25 14:23:21 2018 +0200 +++ b/src/hotspot/share/classfile/dictionary.hpp Sat Aug 25 11:10:21 2018 -0400 @@ -74,7 +74,7 @@ void classes_do(void f(InstanceKlass*)); void classes_do(void f(InstanceKlass*, TRAPS), TRAPS); - void all_entries_do(void f(InstanceKlass*, ClassLoaderData*)); + void all_entries_do(KlassClosure* closure); void classes_do(MetaspaceClosure* it); void unlink(); diff -r a716460217ed -r 1f0b605bdc28 src/hotspot/share/memory/universe.cpp --- a/src/hotspot/share/memory/universe.cpp Sat Aug 25 14:23:21 2018 +0200 +++ b/src/hotspot/share/memory/universe.cpp Sat Aug 25 11:10:21 2018 -0400 @@ -165,14 +165,15 @@ uint64_t Universe::_narrow_klass_range = (uint64_t(max_juint)+1); void Universe::basic_type_classes_do(void f(Klass*)) { - f(boolArrayKlassObj()); - f(byteArrayKlassObj()); - f(charArrayKlassObj()); - f(intArrayKlassObj()); - f(shortArrayKlassObj()); - f(longArrayKlassObj()); - f(singleArrayKlassObj()); - f(doubleArrayKlassObj()); + for (int i = T_BOOLEAN; i < T_LONG+1; i++) { + f(_typeArrayKlassObjs[i]); + } +} + +void Universe::basic_type_classes_do(KlassClosure *closure) { + for (int i = T_BOOLEAN; i < T_LONG+1; i++) { + closure->do_klass(_typeArrayKlassObjs[i]); + } } void Universe::oops_do(OopClosure* f, bool do_all) { diff -r a716460217ed -r 1f0b605bdc28 src/hotspot/share/memory/universe.hpp --- a/src/hotspot/share/memory/universe.hpp Sat Aug 25 14:23:21 2018 +0200 +++ b/src/hotspot/share/memory/universe.hpp Sat Aug 25 11:10:21 2018 -0400 @@ -486,6 +486,7 @@ // Apply "f" to all klasses for basic types (classes not present in // SystemDictionary). static void basic_type_classes_do(void f(Klass*)); + static void basic_type_classes_do(KlassClosure* closure); static void metaspace_pointers_do(MetaspaceClosure* it); // Debugging diff -r a716460217ed -r 1f0b605bdc28 src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp --- a/src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp Sat Aug 25 14:23:21 2018 +0200 +++ b/src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp Sat Aug 25 11:10:21 2018 -0400 @@ -23,6 +23,7 @@ */ #include "precompiled.hpp" +#include "classfile/dictionary.hpp" #include "classfile/classLoaderData.inline.hpp" #include "classfile/systemDictionary.hpp" #include "gc/shared/collectedHeap.hpp" @@ -39,16 +40,7 @@ Stack _classStack; JvmtiEnv* _env; Thread* _cur_thread; - -public: - LoadedClassesClosure(Thread* thread, JvmtiEnv* env) : _env(env), _cur_thread(thread) { - assert(_cur_thread == Thread::current(), "must be current thread"); - } - - void do_klass(Klass* k) { - // Collect all jclasses - _classStack.push((jclass) _env->jni_reference(Handle(_cur_thread, k->java_mirror()))); - } + bool _dictionary_walk; int extract(jclass* result_list) { // The size of the Stack will be 0 after extract, so get it here @@ -68,198 +60,46 @@ int get_count() { return (int)_classStack.size(); } -}; -// The closure for GetClassLoaderClasses -class JvmtiGetLoadedClassesClosure : public StackObj { - // Since the ClassLoaderDataGraph::dictionary_all_entries_do callback - // doesn't pass a closureData pointer, - // we use a thread-local slot to hold a pointer to - // a stack allocated instance of this structure. - private: - jobject _initiatingLoader; - int _count; - Handle* _list; - int _index; - - private: - // Getting and setting the thread local pointer - static JvmtiGetLoadedClassesClosure* get_this() { - JvmtiGetLoadedClassesClosure* result = NULL; - JavaThread* thread = JavaThread::current(); - result = thread->get_jvmti_get_loaded_classes_closure(); - return result; - } - static void set_this(JvmtiGetLoadedClassesClosure* that) { - JavaThread* thread = JavaThread::current(); - thread->set_jvmti_get_loaded_classes_closure(that); - } - - public: - // Constructor/Destructor - JvmtiGetLoadedClassesClosure() { - JvmtiGetLoadedClassesClosure* that = get_this(); - assert(that == NULL, "JvmtiGetLoadedClassesClosure in use"); - _initiatingLoader = NULL; - _count = 0; - _list = NULL; - _index = 0; - set_this(this); - } - - JvmtiGetLoadedClassesClosure(jobject initiatingLoader) { - JvmtiGetLoadedClassesClosure* that = get_this(); - assert(that == NULL, "JvmtiGetLoadedClassesClosure in use"); - _initiatingLoader = initiatingLoader; - _count = 0; - _list = NULL; - _index = 0; - set_this(this); - } - - ~JvmtiGetLoadedClassesClosure() { - JvmtiGetLoadedClassesClosure* that = get_this(); - assert(that != NULL, "JvmtiGetLoadedClassesClosure not found"); - set_this(NULL); - _initiatingLoader = NULL; - _count = 0; - if (_list != NULL) { - FreeHeap(_list); - _list = NULL; - } - _index = 0; - } - - // Accessors. - jobject get_initiatingLoader() { - return _initiatingLoader; - } - - int get_count() { - return _count; +public: + LoadedClassesClosure(JvmtiEnv* env, bool dictionary_walk) : + _env(env), + _cur_thread(Thread::current()), + _dictionary_walk(dictionary_walk) { } - void set_count(int value) { - _count = value; - } - - Handle* get_list() { - return _list; - } - - void set_list(Handle* value) { - _list = value; - } - - int get_index() { - return _index; - } - - void set_index(int value) { - _index = value; - } - - Handle get_element(int index) { - if ((_list != NULL) && (index < _count)) { - return _list[index]; - } else { - assert(false, "empty get_element"); - return Handle(); - } - } - - void set_element(int index, Handle value) { - if ((_list != NULL) && (index < _count)) { - _list[index] = value; - } else { - assert(false, "bad set_element"); - } - } - - // Other predicates - bool available() { - return (_list != NULL); - } - -#ifdef ASSERT - // For debugging. - void check(int limit) { - for (int i = 0; i < limit; i += 1) { - assert(Universe::heap()->is_in(get_element(i)()), "check fails"); - } - } -#endif - - // Public methods that get called within the scope of the closure - void allocate() { - _list = NEW_C_HEAP_ARRAY(Handle, _count, mtInternal); - assert(_list != NULL, "Out of memory"); - if (_list == NULL) { - _count = 0; - } - } - - void extract(JvmtiEnv *env, jclass* result) { - for (int index = 0; index < _count; index += 1) { - result[index] = (jclass) env->jni_reference(get_element(index)); - } - } - - static void increment_with_loader(InstanceKlass* k, ClassLoaderData* loader_data) { - JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this(); - oop class_loader = loader_data->class_loader(); - if (class_loader == JNIHandles::resolve(that->get_initiatingLoader())) { - for (Klass* l = k; l != NULL; l = l->array_klass_or_null()) { - that->set_count(that->get_count() + 1); + void do_klass(Klass* k) { + // Collect all jclasses + _classStack.push((jclass) _env->jni_reference(Handle(_cur_thread, k->java_mirror()))); + if (_dictionary_walk) { + // Collect array classes this way when walking the dictionary (because array classes are + // not in the dictionary). + for (Klass* l = k->array_klass_or_null(); l != NULL; l = l->array_klass_or_null()) { + _classStack.push((jclass) _env->jni_reference(Handle(_cur_thread, l->java_mirror()))); } } } - static void add_with_loader(InstanceKlass* k, ClassLoaderData* loader_data) { - JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this(); - if (that->available()) { - oop class_loader = loader_data->class_loader(); - if (class_loader == JNIHandles::resolve(that->get_initiatingLoader())) { - Thread *thread = Thread::current(); - for (Klass* l = k; l != NULL; l = l->array_klass_or_null()) { - Handle mirror(thread, l->java_mirror()); - that->set_element(that->get_index(), mirror); - that->set_index(that->get_index() + 1); - } - } - } - } + jvmtiError get_result(JvmtiEnv *env, jint* classCountPtr, jclass** classesPtr) { + // Return results by extracting the collected contents into a list + // allocated via JvmtiEnv + jclass* result_list; + jvmtiError error = env->Allocate(get_count() * sizeof(jclass), + (unsigned char**)&result_list); - // increment the count for the given basic type array class (and any - // multi-dimensional arrays). For example, for [B we check for - // [[B, [[[B, .. and the count is incremented for each one that exists. - static void increment_for_basic_type_arrays(Klass* k) { - JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this(); - assert(that != NULL, "no JvmtiGetLoadedClassesClosure"); - for (Klass* l = k; l != NULL; l = l->array_klass_or_null()) { - that->set_count(that->get_count() + 1); + if (error == JVMTI_ERROR_NONE) { + int count = extract(result_list); + *classCountPtr = count; + *classesPtr = result_list; } - } - - // add the basic type array class and its multi-dimensional array classes to the list - static void add_for_basic_type_arrays(Klass* k) { - JvmtiGetLoadedClassesClosure* that = JvmtiGetLoadedClassesClosure::get_this(); - assert(that != NULL, "no JvmtiGetLoadedClassesClosure"); - assert(that->available(), "no list"); - Thread *thread = Thread::current(); - for (Klass* l = k; l != NULL; l = l->array_klass_or_null()) { - Handle mirror(thread, l->java_mirror()); - that->set_element(that->get_index(), mirror); - that->set_index(that->get_index() + 1); - } + return error; } }; - jvmtiError JvmtiGetLoadedClasses::getLoadedClasses(JvmtiEnv *env, jint* classCountPtr, jclass** classesPtr) { - LoadedClassesClosure closure(Thread::current(), env); + LoadedClassesClosure closure(env, false); { // To get a consistent list of classes we need MultiArray_lock to ensure // array classes aren't created. @@ -270,56 +110,35 @@ ClassLoaderDataGraph::loaded_classes_do(&closure); } - // Return results by extracting the collected contents into a list - // allocated via JvmtiEnv - jclass* result_list; - jvmtiError error = env->Allocate(closure.get_count() * sizeof(jclass), - (unsigned char**)&result_list); - - if (error == JVMTI_ERROR_NONE) { - int count = closure.extract(result_list); - *classCountPtr = count; - *classesPtr = result_list; - } - return error; + return closure.get_result(env, classCountPtr, classesPtr); } jvmtiError JvmtiGetLoadedClasses::getClassLoaderClasses(JvmtiEnv *env, jobject initiatingLoader, jint* classCountPtr, jclass** classesPtr) { - // Since ClassLoaderDataGraph::dictionary_all_entries_do only takes a function pointer - // and doesn't call back with a closure data pointer, - // we can only pass static methods. - JvmtiGetLoadedClassesClosure closure(initiatingLoader); + + LoadedClassesClosure closure(env, true); { // To get a consistent list of classes we need MultiArray_lock to ensure - // array classes aren't created, and SystemDictionary_lock to ensure that - // classes aren't added to the class loader data dictionaries. + // array classes aren't created during this walk. MutexLocker ma(MultiArray_lock); MutexLocker sd(SystemDictionary_lock); - // First, count the classes in the class loader data dictionaries which have this loader recorded - // as an initiating loader. For basic type arrays this information is not recorded - // so GetClassLoaderClasses will return all of the basic type arrays. This is okay - // because the defining loader for basic type arrays is always the boot class loader - // and these classes are "visible" to all loaders. - ClassLoaderDataGraph::dictionary_all_entries_do(&JvmtiGetLoadedClassesClosure::increment_with_loader); - Universe::basic_type_classes_do(&JvmtiGetLoadedClassesClosure::increment_for_basic_type_arrays); - // Next, fill in the classes - closure.allocate(); - ClassLoaderDataGraph::dictionary_all_entries_do(&JvmtiGetLoadedClassesClosure::add_with_loader); - Universe::basic_type_classes_do(&JvmtiGetLoadedClassesClosure::add_for_basic_type_arrays); - // Drop the SystemDictionary_lock, so the results could be wrong from here, - // but we still have a snapshot. + oop loader = JNIHandles::resolve(initiatingLoader); + // All classes loaded from this loader as initiating loader are + // requested, so only need to walk this loader's ClassLoaderData + // dictionary, or the NULL ClassLoaderData dictionary for bootstrap loader. + if (loader != NULL) { + ClassLoaderData* data = java_lang_ClassLoader::loader_data(loader); + // ClassLoader may not be used yet for loading. + if (data != NULL && data->dictionary() != NULL) { + data->dictionary()->all_entries_do(&closure); + } + } else { + ClassLoaderData::the_null_class_loader_data()->dictionary()->all_entries_do(&closure); + } + // Get basic arrays for all loaders. + Universe::basic_type_classes_do(&closure); } - // Post results - jclass* result_list; - jvmtiError err = env->Allocate(closure.get_count() * sizeof(jclass), - (unsigned char**)&result_list); - if (err != JVMTI_ERROR_NONE) { - return err; - } - closure.extract(env, result_list); - *classCountPtr = closure.get_count(); - *classesPtr = result_list; - return JVMTI_ERROR_NONE; + + return closure.get_result(env, classCountPtr, classesPtr); } diff -r a716460217ed -r 1f0b605bdc28 src/hotspot/share/runtime/mutexLocker.cpp --- a/src/hotspot/share/runtime/mutexLocker.cpp Sat Aug 25 14:23:21 2018 +0200 +++ b/src/hotspot/share/runtime/mutexLocker.cpp Sat Aug 25 11:10:21 2018 -0400 @@ -235,7 +235,7 @@ def(InlineCacheBuffer_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); def(VMStatistic_lock , PaddedMutex , leaf, false, Monitor::_safepoint_check_always); def(ExpandHeap_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); // Used during compilation by VM thread - def(JNIHandleBlockFreeList_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_never); // handles are used by VM thread + def(JNIHandleBlockFreeList_lock , PaddedMutex , leaf-1, true, Monitor::_safepoint_check_never); // handles are used by VM thread def(SignatureHandlerLibrary_lock , PaddedMutex , leaf, false, Monitor::_safepoint_check_always); def(SymbolArena_lock , PaddedMutex , leaf+2, true, Monitor::_safepoint_check_never); def(StringTable_lock , PaddedMutex , leaf, true, Monitor::_safepoint_check_always); diff -r a716460217ed -r 1f0b605bdc28 src/hotspot/share/runtime/thread.cpp --- a/src/hotspot/share/runtime/thread.cpp Sat Aug 25 14:23:21 2018 +0200 +++ b/src/hotspot/share/runtime/thread.cpp Sat Aug 25 11:10:21 2018 -0400 @@ -1567,7 +1567,6 @@ _is_method_handle_return = 0; _jvmti_thread_state= NULL; _should_post_on_exceptions_flag = JNI_FALSE; - _jvmti_get_loaded_classes_closure = NULL; _interp_only_mode = 0; _special_runtime_exit_condition = _no_async_condition; _pending_async_exception = NULL; diff -r a716460217ed -r 1f0b605bdc28 src/hotspot/share/runtime/thread.hpp --- a/src/hotspot/share/runtime/thread.hpp Sat Aug 25 14:23:21 2018 +0200 +++ b/src/hotspot/share/runtime/thread.hpp Sat Aug 25 11:10:21 2018 -0400 @@ -63,7 +63,6 @@ class ThreadsSMRSupport; class JvmtiThreadState; -class JvmtiGetLoadedClassesClosure; class ThreadStatistics; class ConcurrentLocksDump; class ParkEvent; @@ -1885,8 +1884,6 @@ // the specified JavaThread is exiting. JvmtiThreadState *jvmti_thread_state() const { return _jvmti_thread_state; } static ByteSize jvmti_thread_state_offset() { return byte_offset_of(JavaThread, _jvmti_thread_state); } - void set_jvmti_get_loaded_classes_closure(JvmtiGetLoadedClassesClosure* value) { _jvmti_get_loaded_classes_closure = value; } - JvmtiGetLoadedClassesClosure* get_jvmti_get_loaded_classes_closure() const { return _jvmti_get_loaded_classes_closure; } // JVMTI PopFrame support // Setting and clearing popframe_condition @@ -1938,7 +1935,6 @@ private: JvmtiThreadState *_jvmti_thread_state; - JvmtiGetLoadedClassesClosure* _jvmti_get_loaded_classes_closure; // Used by the interpreter in fullspeed mode for frame pop, method // entry, method exit and single stepping support. This field is