# HG changeset patch # User hseigel # Date 1519229932 18000 # Node ID dc68aeea4840db0d1adae30d58645cedf0af63b4 # Parent bc92debe57e45b8c91e970a81e86d0eecea6191b 8184289: Obsolete -XX:+UnsyncloadClass and -XX:+MustCallLoadClassInternal options Summary: Add comments, fix a small issue with the boot loader, and add an assert. Reviewed-by: coleenp, alanb, acorn, dholmes diff -r bc92debe57e4 -r dc68aeea4840 src/hotspot/share/classfile/classLoader.cpp --- a/src/hotspot/share/classfile/classLoader.cpp Thu Feb 15 23:45:15 2018 +0100 +++ b/src/hotspot/share/classfile/classLoader.cpp Wed Feb 21 11:18:52 2018 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -137,7 +137,6 @@ PerfCounter* ClassLoader::_sync_JVMDefineClassLockFreeCounter = NULL; PerfCounter* ClassLoader::_sync_JNIDefineClassLockFreeCounter = NULL; PerfCounter* ClassLoader::_unsafe_defineClassCallCounter = NULL; -PerfCounter* ClassLoader::_isUnsyncloadClass = NULL; PerfCounter* ClassLoader::_load_instance_class_failCounter = NULL; GrowableArray* ClassLoader::_patch_mod_entries = NULL; @@ -1642,9 +1641,6 @@ // of the bug fix of 6365597. They are mainly focused on finding out // the behavior of system & user-defined classloader lock, whether // ClassLoader.loadClass/findClass is being called synchronized or not. - // Also two additional counters are created to see whether 'UnsyncloadClass' - // flag is being set or not and how many times load_instance_class call - // fails with linkageError etc. NEWPERFEVENTCOUNTER(_sync_systemLoaderLockContentionRate, SUN_CLS, "systemLoaderLockContentionRate"); NEWPERFEVENTCOUNTER(_sync_nonSystemLoaderLockContentionRate, SUN_CLS, @@ -1660,14 +1656,8 @@ NEWPERFEVENTCOUNTER(_unsafe_defineClassCallCounter, SUN_CLS, "unsafeDefineClassCalls"); - NEWPERFEVENTCOUNTER(_isUnsyncloadClass, SUN_CLS, "isUnsyncloadClassSet"); NEWPERFEVENTCOUNTER(_load_instance_class_failCounter, SUN_CLS, "loadInstanceClassFailRate"); - - // increment the isUnsyncloadClass counter if UnsyncloadClass is set. - if (UnsyncloadClass) { - _isUnsyncloadClass->inc(); - } } // lookup zip library entry points diff -r bc92debe57e4 -r dc68aeea4840 src/hotspot/share/classfile/classLoader.hpp --- a/src/hotspot/share/classfile/classLoader.hpp Thu Feb 15 23:45:15 2018 +0100 +++ b/src/hotspot/share/classfile/classLoader.hpp Wed Feb 21 11:18:52 2018 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -207,7 +207,6 @@ static PerfCounter* _sync_JNIDefineClassLockFreeCounter; static PerfCounter* _unsafe_defineClassCallCounter; - static PerfCounter* _isUnsyncloadClass; static PerfCounter* _load_instance_class_failCounter; // The boot class path consists of 3 ordered pieces: diff -r bc92debe57e4 -r dc68aeea4840 src/hotspot/share/classfile/systemDictionary.cpp --- a/src/hotspot/share/classfile/systemDictionary.cpp Thu Feb 15 23:45:15 2018 +0100 +++ b/src/hotspot/share/classfile/systemDictionary.cpp Wed Feb 21 11:18:52 2018 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -106,7 +106,6 @@ oop SystemDictionary::_java_system_loader = NULL; oop SystemDictionary::_java_platform_loader = NULL; -bool SystemDictionary::_has_loadClassInternal = false; bool SystemDictionary::_has_checkPackageAccess = false; // lazily initialized klass variables @@ -159,7 +158,7 @@ // Parallel class loading check bool SystemDictionary::is_parallelCapable(Handle class_loader) { - if (UnsyncloadClass || class_loader.is_null()) return true; + if (class_loader.is_null()) return true; if (AlwaysLockClassLoader) return false; return java_lang_ClassLoader::parallelCapable(class_loader()); } @@ -503,8 +502,7 @@ // // We only get here if // 1) custom classLoader, i.e. not bootstrap classloader -// 2) UnsyncloadClass not set -// 3) custom classLoader has broken the class loader objectLock +// 2) custom classLoader has broken the class loader objectLock // so another thread got here in parallel // // lockObject must be held. @@ -594,7 +592,6 @@ } else { placeholder = placeholders()->get_entry(p_index, p_hash, name, loader_data); if (placeholder && placeholder->super_load_in_progress() ){ - // Before UnsyncloadClass: // We only get here if the application has released the // classloader lock when another thread was in the middle of loading a // superclass/superinterface for this class, and now @@ -687,9 +684,9 @@ // defining the class in parallel by accident. // This lock must be acquired here so the waiter will find // any successful result in the SystemDictionary and not attempt - // the define - // ParallelCapable Classloaders and the bootstrap classloader, - // or all classloaders with UnsyncloadClass do not acquire lock here + // the define. + // ParallelCapable Classloaders and the bootstrap classloader + // do not acquire lock here. bool DoObjectLock = true; if (is_parallelCapable(class_loader)) { DoObjectLock = false; @@ -765,14 +762,11 @@ // and that lock is still held when calling classloader's loadClass. // For these classloaders, we ensure that the first requestor // completes the load and other requestors wait for completion. - // case 3. UnsyncloadClass - don't use objectLocker - // With this flag, we allow parallel classloading of a - // class/classloader pair - // case4. Bootstrap classloader - don't own objectLocker + // case 3. Bootstrap classloader - don't own objectLocker // This classloader supports parallelism at the classloader level, // but only allows a single load of a class/classloader pair. // No performance benefit and no deadlock issues. - // case 5. parallelCapable user level classloaders - without objectLocker + // case 4. parallelCapable user level classloaders - without objectLocker // Allow parallel classloading of a class/classloader pair { @@ -788,7 +782,7 @@ // case 1: traditional: should never see load_in_progress. while (!class_has_been_loaded && oldprobe && oldprobe->instance_load_in_progress()) { - // case 4: bootstrap classloader: prevent futile classloading, + // case 3: bootstrap classloader: prevent futile classloading, // wait on first requestor if (class_loader.is_null()) { SystemDictionary_lock->wait(); @@ -811,7 +805,7 @@ } } // All cases: add LOAD_INSTANCE holding SystemDictionary_lock - // case 3: UnsyncloadClass || case 5: parallelCapable: allow competing threads to try + // case 4: parallelCapable: allow competing threads to try // LOAD_INSTANCE in parallel if (!throw_circularity_error && !class_has_been_loaded) { @@ -844,28 +838,6 @@ // Do actual loading k = load_instance_class(name, class_loader, THREAD); - // For UnsyncloadClass only - // If they got a linkageError, check if a parallel class load succeeded. - // If it did, then for bytecode resolution the specification requires - // that we return the same result we did for the other thread, i.e. the - // successfully loaded InstanceKlass - // Should not get here for classloaders that support parallelism - // with the new cleaner mechanism, even with AllowParallelDefineClass - // Bootstrap goes through here to allow for an extra guarantee check - if (UnsyncloadClass || (class_loader.is_null())) { - if (k == NULL && HAS_PENDING_EXCEPTION - && PENDING_EXCEPTION->is_a(SystemDictionary::LinkageError_klass())) { - MutexLocker mu(SystemDictionary_lock, THREAD); - InstanceKlass* check = find_class(d_hash, name, dictionary); - if (check != NULL) { - // Klass is already loaded, so just use it - k = check; - CLEAR_PENDING_EXCEPTION; - guarantee((!class_loader.is_null()), "dup definition for bootstrap loader?"); - } - } - } - // If everything was OK (no exceptions, no null return value), and // class_loader is NOT the defining loader, do a little more bookkeeping. if (!HAS_PENDING_EXCEPTION && k != NULL && @@ -1097,7 +1069,7 @@ HandleMark hm(THREAD); // Classloaders that support parallelism, e.g. bootstrap classloader, - // or all classloaders with UnsyncloadClass do not acquire lock here + // do not acquire lock here bool DoObjectLock = true; if (is_parallelCapable(class_loader)) { DoObjectLock = false; @@ -1556,40 +1528,17 @@ InstanceKlass* spec_klass = SystemDictionary::ClassLoader_klass(); - // Call public unsynchronized loadClass(String) directly for all class loaders - // for parallelCapable class loaders. JDK >=7, loadClass(String, boolean) will + // Call public unsynchronized loadClass(String) directly for all class loaders. + // For parallelCapable class loaders, JDK >=7, loadClass(String, boolean) will // acquire a class-name based lock rather than the class loader object lock. - // JDK < 7 already acquire the class loader lock in loadClass(String, boolean), - // so the call to loadClassInternal() was not required. - // - // UnsyncloadClass flag means both call loadClass(String) and do - // not acquire the class loader lock even for class loaders that are - // not parallelCapable. This was a risky transitional - // flag for diagnostic purposes only. It is risky to call - // custom class loaders without synchronization. - // WARNING If a custom class loader does NOT synchronizer findClass, or callers of - // findClass, the UnsyncloadClass flag risks unexpected timing bugs in the field. - // Do NOT assume this will be supported in future releases. - // - // Added MustCallLoadClassInternal in case we discover in the field - // a customer that counts on this call - if (MustCallLoadClassInternal && has_loadClassInternal()) { - JavaCalls::call_special(&result, - class_loader, - spec_klass, - vmSymbols::loadClassInternal_name(), - vmSymbols::string_class_signature(), - string, - CHECK_NULL); - } else { - JavaCalls::call_virtual(&result, - class_loader, - spec_klass, - vmSymbols::loadClass_name(), - vmSymbols::string_class_signature(), - string, - CHECK_NULL); - } + // JDK < 7 already acquire the class loader lock in loadClass(String, boolean). + JavaCalls::call_virtual(&result, + class_loader, + spec_klass, + vmSymbols::loadClass_name(), + vmSymbols::string_class_signature(), + string, + CHECK_NULL); assert(result.get_type() == T_OBJECT, "just checking"); oop obj = (oop) result.get_jobject(); @@ -1718,7 +1667,7 @@ { MutexLocker mu(SystemDictionary_lock, THREAD); // First check if class already defined - if (UnsyncloadClass || (is_parallelDefine(class_loader))) { + if (is_parallelDefine(class_loader)) { InstanceKlass* check = find_class(d_hash, name_h, dictionary); if (check != NULL) { return check; @@ -1737,7 +1686,7 @@ // Only special cases allow parallel defines and can use other thread's results // Other cases fall through, and may run into duplicate defines // caught by finding an entry in the SystemDictionary - if ((UnsyncloadClass || is_parallelDefine(class_loader)) && (probe->instance_klass() != NULL)) { + if (is_parallelDefine(class_loader) && (probe->instance_klass() != NULL)) { placeholders()->find_and_remove(p_index, p_hash, name_h, loader_data, PlaceholderTable::DEFINE_CLASS, THREAD); SystemDictionary_lock->notify_all(); #ifdef ASSERT @@ -2174,10 +2123,6 @@ //_box_klasses[T_OBJECT] = WK_KLASS(object_klass); //_box_klasses[T_ARRAY] = WK_KLASS(object_klass); - { // Compute whether we should use loadClass or loadClassInternal when loading classes. - Method* method = InstanceKlass::cast(ClassLoader_klass())->find_method(vmSymbols::loadClassInternal_name(), vmSymbols::string_class_signature()); - _has_loadClassInternal = (method != NULL); - } { // Compute whether we should use checkPackageAccess or NOT Method* method = InstanceKlass::cast(ClassLoader_klass())->find_method(vmSymbols::checkPackageAccess_name(), vmSymbols::class_protectiondomain_signature()); _has_checkPackageAccess = (method != NULL); diff -r bc92debe57e4 -r dc68aeea4840 src/hotspot/share/classfile/systemDictionary.hpp --- a/src/hotspot/share/classfile/systemDictionary.hpp Thu Feb 15 23:45:15 2018 +0100 +++ b/src/hotspot/share/classfile/systemDictionary.hpp Wed Feb 21 11:18:52 2018 -0500 @@ -467,9 +467,6 @@ static void load_abstract_ownable_synchronizer_klass(TRAPS); protected: - // Tells whether ClassLoader.loadClassInternal is present - static bool has_loadClassInternal() { return _has_loadClassInternal; } - // Returns the class loader data to be used when looking up/updating the // system dictionary. static ClassLoaderData *class_loader_data(Handle class_loader) { @@ -746,7 +743,6 @@ static oop _java_system_loader; static oop _java_platform_loader; - static bool _has_loadClassInternal; static bool _has_checkPackageAccess; }; diff -r bc92debe57e4 -r dc68aeea4840 src/hotspot/share/classfile/vmSymbols.hpp --- a/src/hotspot/share/classfile/vmSymbols.hpp Thu Feb 15 23:45:15 2018 +0100 +++ b/src/hotspot/share/classfile/vmSymbols.hpp Wed Feb 21 11:18:52 2018 -0500 @@ -361,7 +361,6 @@ template(run_finalizers_on_exit_name, "runFinalizersOnExit") \ template(dispatchUncaughtException_name, "dispatchUncaughtException") \ template(loadClass_name, "loadClass") \ - template(loadClassInternal_name, "loadClassInternal") \ template(get_name, "get") \ template(put_name, "put") \ template(type_name, "type") \ diff -r bc92debe57e4 -r dc68aeea4840 src/hotspot/share/prims/jvm.cpp --- a/src/hotspot/share/prims/jvm.cpp Thu Feb 15 23:45:15 2018 +0100 +++ b/src/hotspot/share/prims/jvm.cpp Wed Feb 21 11:18:52 2018 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -166,9 +166,8 @@ } } else if (last_caller != NULL && last_caller->method_holder()->name() == - vmSymbols::java_lang_ClassLoader() && - (last_caller->name() == vmSymbols::loadClassInternal_name() || - last_caller->name() == vmSymbols::loadClass_name())) { + vmSymbols::java_lang_ClassLoader() && + last_caller->name() == vmSymbols::loadClass_name()) { found_it = true; } else if (!vfst.at_end()) { if (vfst.method()->is_native()) { diff -r bc92debe57e4 -r dc68aeea4840 src/hotspot/share/runtime/arguments.cpp --- a/src/hotspot/share/runtime/arguments.cpp Thu Feb 15 23:45:15 2018 +0100 +++ b/src/hotspot/share/runtime/arguments.cpp Wed Feb 21 11:18:52 2018 -0500 @@ -520,8 +520,8 @@ // --- Deprecated alias flags (see also aliased_jvm_flags) - sorted by obsolete_in then expired_in: { "DefaultMaxRAMFraction", JDK_Version::jdk(8), JDK_Version::undefined(), JDK_Version::undefined() }, { "CreateMinidumpOnCrash", JDK_Version::jdk(9), JDK_Version::undefined(), JDK_Version::undefined() }, - { "MustCallLoadClassInternal", JDK_Version::jdk(10), JDK_Version::undefined(), JDK_Version::undefined() }, - { "UnsyncloadClass", JDK_Version::jdk(10), JDK_Version::undefined(), JDK_Version::undefined() }, + { "MustCallLoadClassInternal", JDK_Version::jdk(10), JDK_Version::jdk(11), JDK_Version::jdk(12) }, + { "UnsyncloadClass", JDK_Version::jdk(10), JDK_Version::jdk(11), JDK_Version::jdk(12) }, // -------------- Obsolete Flags - sorted by expired_in -------------- { "ConvertSleepToYield", JDK_Version::jdk(9), JDK_Version::jdk(10), JDK_Version::jdk(11) }, diff -r bc92debe57e4 -r dc68aeea4840 src/hotspot/share/runtime/globals.hpp --- a/src/hotspot/share/runtime/globals.hpp Thu Feb 15 23:45:15 2018 +0100 +++ b/src/hotspot/share/runtime/globals.hpp Wed Feb 21 11:18:52 2018 -0500 @@ -1142,11 +1142,6 @@ diagnostic(bool, DynamicallyResizeSystemDictionaries, true, \ "Dynamically resize system dictionaries as needed") \ \ - diagnostic(bool, UnsyncloadClass, false, \ - "Unstable: VM calls loadClass unsynchronized. Custom " \ - "class loader must call VM synchronized for findClass " \ - "and defineClass.") \ - \ product(bool, AlwaysLockClassLoader, false, \ "Require the VM to acquire the class loader lock before calling " \ "loadClass() even for class loaders registering " \ @@ -1156,9 +1151,6 @@ "Allow parallel defineClass requests for class loaders " \ "registering as parallel capable") \ \ - product(bool, MustCallLoadClassInternal, false, \ - "Call loadClassInternal() rather than loadClass()") \ - \ product_pd(bool, DontYieldALot, \ "Throw away obvious excess yield calls") \ \ diff -r bc92debe57e4 -r dc68aeea4840 src/hotspot/share/runtime/synchronizer.hpp --- a/src/hotspot/share/runtime/synchronizer.hpp Thu Feb 15 23:45:15 2018 +0100 +++ b/src/hotspot/share/runtime/synchronizer.hpp Wed Feb 21 11:18:52 2018 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -184,8 +184,6 @@ // have to pass through, and we must also be able to deal with // asynchronous exceptions. The caller is responsible for checking // the threads pending exception if needed. -// doLock was added to support classloading with UnsyncloadClass which -// requires flag based choice of locking the classloader lock. class ObjectLocker : public StackObj { private: Thread* _thread; diff -r bc92debe57e4 -r dc68aeea4840 src/java.base/share/classes/java/lang/ClassLoader.java --- a/src/java.base/share/classes/java/lang/ClassLoader.java Thu Feb 15 23:45:15 2018 +0100 +++ b/src/java.base/share/classes/java/lang/ClassLoader.java Wed Feb 21 11:18:52 2018 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -650,21 +650,6 @@ return lock; } - // This method is invoked by the virtual machine to load a class. - private Class loadClassInternal(String name) - throws ClassNotFoundException - { - // For backward compatibility, explicitly lock on 'this' when - // the current class loader is not parallel capable. - if (parallelLockMap == null) { - synchronized (this) { - return loadClass(name); - } - } else { - return loadClass(name); - } - } - // Invoked by the VM after loading class with this loader. private void checkPackageAccess(Class cls, ProtectionDomain pd) { final SecurityManager sm = System.getSecurityManager(); diff -r bc92debe57e4 -r dc68aeea4840 test/hotspot/jtreg/runtime/CommandLine/VMDeprecatedOptions.java --- a/test/hotspot/jtreg/runtime/CommandLine/VMDeprecatedOptions.java Thu Feb 15 23:45:15 2018 +0100 +++ b/test/hotspot/jtreg/runtime/CommandLine/VMDeprecatedOptions.java Wed Feb 21 11:18:52 2018 -0500 @@ -41,7 +41,6 @@ public static final String[][] DEPRECATED_OPTIONS = { // deprecated non-alias flags: {"MaxGCMinorPauseMillis", "1032"}, - {"MustCallLoadClassInternal", "false"}, {"MaxRAMFraction", "8"}, {"MinRAMFraction", "2"}, {"InitialRAMFraction", "64"}, @@ -106,7 +105,6 @@ public static void main(String[] args) throws Throwable { testDeprecated(DEPRECATED_OPTIONS); // Make sure that each deprecated option is mentioned in the output. - testDeprecatedDiagnostic("UnsyncloadClass", "false"); testDeprecatedDiagnostic("IgnoreUnverifiableClassesDuringDump", "false"); } } diff -r bc92debe57e4 -r dc68aeea4840 test/hotspot/jtreg/runtime/Metaspace/DefineClass.java --- a/test/hotspot/jtreg/runtime/Metaspace/DefineClass.java Thu Feb 15 23:45:15 2018 +0100 +++ b/test/hotspot/jtreg/runtime/Metaspace/DefineClass.java Wed Feb 21 11:18:52 2018 -0500 @@ -1,4 +1,5 @@ /* + * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2017 SAP SE. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -29,17 +30,9 @@ * @library /test/lib * @run main/othervm test.DefineClass defineClass * @run main/othervm test.DefineClass defineSystemClass - * @run main/othervm -XX:+UnlockDiagnosticVMOptions - -XX:+UnsyncloadClass -XX:+AllowParallelDefineClass - test.DefineClass defineClassParallel - * @run main/othervm -XX:+UnlockDiagnosticVMOptions - -XX:+UnsyncloadClass -XX:-AllowParallelDefineClass + * @run main/othervm -XX:+AllowParallelDefineClass test.DefineClass defineClassParallel - * @run main/othervm -XX:+UnlockDiagnosticVMOptions - -XX:-UnsyncloadClass -XX:+AllowParallelDefineClass - test.DefineClass defineClassParallel - * @run main/othervm -XX:+UnlockDiagnosticVMOptions - -XX:-UnsyncloadClass -XX:-AllowParallelDefineClass + * @run main/othervm -XX:-AllowParallelDefineClass test.DefineClass defineClassParallel * @run main/othervm -Djdk.attach.allowAttachSelf test.DefineClass redefineClass * @run main/othervm -Djdk.attach.allowAttachSelf test.DefineClass redefineClassWithError @@ -126,7 +119,7 @@ } catch (LinkageError jle) { // Expected with a parallel capable class loader and - // -XX:+UnsyncloadClass or -XX:+AllowParallelDefineClass + // -XX:+AllowParallelDefineClass pcl.incrementLinkageErrors(); } @@ -320,7 +313,7 @@ } System.out.print("Counted " + pcl.getLinkageErrors() + " LinkageErrors "); System.out.println(pcl.getLinkageErrors() == 0 ? - "" : "(use -XX:+UnsyncloadClass and/or -XX:+AllowParallelDefineClass to avoid this)"); + "" : "(use -XX:+AllowParallelDefineClass to avoid this)"); System.gc(); System.out.println("System.gc()"); // After System.gc() we expect to remain with two instances: one is the initial version which is