JDK-8026985 : Rewrite SystemDictionary::classes_do and Dictionary::classes_do to use KlassClosure
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2013-10-21
  • Updated: 2017-08-25
  • Resolved: 2017-04-12
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10
10 b21Fixed
Related Reports
Relates :  
JDK-8180325 - Use ClassLoaderData::classes_do for CDS classes
Description
Instead of the myriad of overloadings with functional parameters with different arguments.  There's some odd code in jvmtiGetLoadedClasses that could be cleaned up as well.
Comments
After permgen removal, in general ClassLoaderDataGraph::classes_do should be called in preference to SystemDictionary::classes_do. The former function returns all the loaded classes for each class loader which includes loaded, anonymous classes, allocated classes (not yet loaded and may have an error), array classes, and previous versions of classes which still have methods running. If a caller wishes to filter out unwanted classes, this can be done in the closure or function that is passed to CLDG::classes_do. The SystemDictionary::classes_do functions which delegate to Dictionary::classes_do() functions include classes whose loading is initiated by the class loader in the dictionary. So a class can be found more than once: for it's defining class loader and it's initiating class loader. These are filtered out in many of the classes_do functions, but not all. There is one instance that requires reporting classes with their initiating class loaders. Here is information about the callers of these functions. runtime/biasedLocking.cpp: SystemDictionary::classes_do(enable_biased_locking); which? only want InstanceKlass which are historically only stored in SystemDictionary. do these want to walk anonymous classes? Do they use biased locking? probably not. do not want to walk classes before they are in the "loaded" state tested by? Always tested in startup prims/jvmtiGetLoadedClasses.cpp: SystemDictionary::classes_do(&JvmtiGetLoadedClassesClosure::increment_with_loader); prims/jvmtiGetLoadedClasses.cpp: SystemDictionary::classes_do(&JvmtiGetLoadedClassesClosure::add_with_loader); which? For JVMTI API, this walks the loaded classes *including* the entries with initiating loader https://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#GetClassLoaderClasses tested by? nsk.jvmti.testlist:nsk/jvmti/GetClassLoaderClasses/clsldrclss001 nsk.jvmti.testlist:nsk/jvmti/GetClassLoaderClasses/clsldrclss002 prims/jvmtiGetLoadedClasses.cpp: ClassLoaderDataGraph::loaded_classes_do(&closure); which? JVMTI GetLoadedClasses gets all classes loaded, includes anonymous classes https://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#GetLoadedClasses see JDK-8024423 think the problem was that the test case used CFLH which *used* to get anonymous classes then used GetLoadedClasses to verify that it unloaded them, missing the anonymous classes. This probably doesn't want anonymous classes anymore, should file a bug. tested by? nsk.jvmti.testlist:nsk/jvmti/GetLoadedClasses/loadedclss001 nsk.jvmti.testlist:nsk/jvmti/GetLoadedClasses/loadedclss002 was tested by hprof tests but they've been removed with hprof support memory/metaspaceShared.cpp: SystemDictionary::classes_do(collect_classes2); which? This is a special case for a bug that calls the classes_do function with ClassLoaderData argument that returns the initiating loader because the system dictionary may have NULL ClassLoaderData (or so the comment says). So this creates a GrowableArray with duplicate class entries which are filtered out. This would be more correct to walk the CLDG loaded classes, and filter out array and anonymous classes tested by? CDS -Xshare:dump memory/metaspaceShared.cpp: SystemDictionary::classes_do(link_one_shared_class, THREAD); memory/metaspaceShared.cpp: SystemDictionary::classes_do(check_one_shared_class); which? loaded instanceKlasses but not anonymous classes. tested by? CDS -Xshare:dump memory/universe.cpp: SystemDictionary::classes_do(initialize_itable_for_klass, CHECK); which? During universe_post_init, reinitializes the itables for some primordial classes. Are there not any anonymous classes loaded yet? probably not. Uses SystemDictionary::classes_do() API that throws exception tested by? universe_post_init if not UseSharedSpaces, so always oops/klassVtable.cpp: SystemDictionary::classes_do(do_class); which? some printing - klassVtable::print_statistics() - probably should include anonymous classes if it cares tested by? it's not tested. not sure what it's for. gc/cms/concurrentMarkSweepGeneration.cpp: ClassLoaderDataGraph::classes_do(&verify_klass_oops); gc/cms/concurrentMarkSweepGeneration.cpp: ClassLoaderDataGraph::classes_do(&preclean_klass_closure); gc/cms/concurrentMarkSweepGeneration.cpp: ClassLoaderDataGraph::classes_do(&remark_klass_closure); gc/cms/concurrentMarkSweepGeneration.cpp: ClassLoaderDataGraph::classes_do(&remark_klass_closure); gc/shared/cardTableRS.cpp: ClassLoaderDataGraph::classes_do(&closure); gc/shared/cardTableRS.cpp: ClassLoaderDataGraph::classes_do(&closure); which? GC needs to walk all the classes, including anonymous and partially loaded classes. tested by? All GCs prims/whitebox.cpp: ClassLoaderDataGraph::classes_do(&closure); which? WB API that's looking for a class to see if it's alive. CLDG has the answer. tested by? hotspot/test/runtime/ClassUnload prims/jvmtiRedefineClasses.cpp: ClassLoaderDataGraph::classes_do(&clean_weak_method_links); prims/jvmtiRedefineClasses.cpp: ClassLoaderDataGraph::classes_do(&check_class); prims/jvmtiRedefineClasses.cpp: ClassLoaderDataGraph::classes_do(&adjust_cpool_cache_and_vtable); which? Redefinition needs to fix all classes (vtable, cpCache and MethodData) including anonymous classes tested by? StressRedefine for one services/heapDumper.cpp: ClassLoaderDataGraph::classes_do(&do_load_class); services/heapDumper.cpp: ClassLoaderDataGraph::classes_do(&do_class_dump); which? HeapDumper needs to report loaded and anonymous classes because they account for heap usage tested by? vm.heapdump.testlist but not sure how good they are. memory/heapInspection.cpp: ClassLoaderDataGraph::classes_do(&finder); which? I have no idea what this is - it's used in closed commercial product Looks like another place where we have to change if metadata changes Looks like a very expensive operation tested by? tested with closed commercial product
10-04-2017
Can't change SystemDictionary::classes_do to take a closure because each of the functions wants something different (or throws an exception) based on the arguments. ie the one that passes ClassLoaderData also wants classes in the dictionary for initiating loaders (which is probably wrong for the CDS call).
05-04-2017
> > It would help to really understand > - who is walking classes and why > - which subset they want to walk When I made this change, I visited all of the classes_do calls to verify that they were walking the class list that they want to walk. The GC versions want to walk all of the classes to mark/relocate/etc the mirrors. Redefinition has to walk all of the classes to fix methods, actually in the function adjust_cpool_cache_and_vtable (for arrays also in case you redefine java/lang/Object which is allowed). HeapDumping and JFR want to show anonymous classes. The only exceptions were in CDS, which I didn't want to risk changing. // Need to call SystemDictionary::classes_do(void f(Klass*, ClassLoaderData*)) // as we may have some classes with NULL ClassLoaderData* in the dictionary. Other // variants of SystemDictionary::classes_do will skip those classes. SystemDictionary::classes_do(collect_classes2); And jvmtiGetLoadedClasses, which seems to want initiating loader as well: // First, count the classes in the system dictionary 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. SystemDictionary::classes_do(&JvmtiGetLoadedClassesClosure::increment_with_loader); > - who is walking methods and why > - which subset they want to walk > See below. I think walking all the methods in all the classes seems dubious. It seems better to walk the methods only in the classes that you're interested in (which is what is mostly done). Only print_method_data_histogram and print_method_profiling_data call this for some logging. > - when a class gets added to the CLDG, and when removed > A class is added to CLDG when it's allocated, ie in the allocate_instance_klass, allocate_objArrayKlass, and typeArrayKlass::allocate_klass functions.
13-03-2017
// Iterate over all klasses in dictionary // Just the classes from defining class loaders static void classes_do(void f(Klass*)); // Added for initialize_itable_for_klass to handle exceptions static void classes_do(void f(Klass*, TRAPS), TRAPS); // All classes, and their class loaders, including initiating class loaders static void classes_do(void f(Klass*, ClassLoaderData*)); These functions in SystemDictionary that call the same functions in Dictionary have special purpose and aren't easily translated to use KlassClosure without changing KlassClosure to return TRAPS, etc. There is some general confusion in the jvm code whether to call these SystemDictionary::classes_do functions vs. ClassLoaderDataGraph::classes_do functions. The latter will walk all the array classes and anonymous classes which are not in the SystemDictionary. With my changes for this bug, there are 3 instances of calling classes_do with SystemDictionary. The rest should use ClassLoaderDataGraph. My change for this RFE also removes left over functions from Permgen elimination that were thought to be useful for enhanced class redefinition, but are actually wrong for it anyway.
08-03-2017
Maybe all occurrences of Dictionary::classes_do() calls can use ClassLoaderDataGraph::classes_do() instead since most (except one) only want classes in defining class loader.
13-12-2013