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
--- 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<ModuleClassPathList*>* 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
--- 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:
--- 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);
--- 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;
};
--- 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") \
--- 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()) {
--- 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) },
--- 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") \
\
--- 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;
--- 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();
--- 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");
}
}
--- 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