Hi Peter,
I'd like to understand the scope and impact of the problem before
jumping to a solution.
For specific applications, overriding ObjectInputStream.resolveClass to
handle the class
lookup can handle the case as expeditiously as is necessary.
What application uses cases are impacted and how prevelant are they?
Many, many frameworks and applications create classloaders for their own
purposes and
take advantage of the required behavior.
Thanks, Roger
On 6/4/20 7:08 PM, Peter Kessler OS wrote:
ObjectInputStream.resolveClass(ObjectStreamClass) is specified to find "the first
class loader on the current thread's stack (starting from the currently executing method)
that is neither the platform class loader nor its ancestor." Finding that class
loader is done with a virtual frame walk in JVM_LatestUserDefinedLoader(JNIEnv *env).
The virtual frame walk is expensive. If an application does not define any ClassLoaders,
then the virtual frame walk will always find the system class loader.
If an application does a significant amount of reading from an
ObjectInputStream, the cost of
ObjectInputStream.resolveClass(ObjectInputStreamClass) can be considerable. In
a sample application, that method (and the methods it calls) have been measured
to consume 5+% of the CPU cycles.
The proposal is to note the construction of any user-defined ClassLoader, and
if none has been constructed, avoid the virtual frame walk returning
ClassLoader.getSystemClassLoader(). But if any user-defined ClassLoader has
been constructed, use the virtual frame walk to find the first user-defined
ClassLoader on the thread stack.
User-defined ClassLoaders could be distinguished from non-user-defined
ClassLoaders in several ways. The ClassLoader class hierarchy could be changed
to make non-user-defined ClassLoaders inherit from a marker class. There is
already a jdk.internal.loader.BuiltinClassLoader but it is used for module (and
resource) loading. BuiltinClassLoader is not a super-class of
jdk.internal.reflect.DelegatingClassLoader, which is used to load reflection
classes, and reflection methods are also ignored by the virtual frame walk.
ClassLoaders could have a boolean that distinguished whether they are
user-defined or not. That would require a change to ClassLoader API. There
are other ways to distinguish user-defined ClassLoaders. The proposal is to
keep the decoration of the ClassLoaders out of the instances, so that it is
only visible to the code that needs it.
The proposal is that each non-user-defined ClassLoader have a static block that
registers the class as a non-user-defined ClassLoader. Then in the base
ClassLoader constructor check if the ClassLoader being constructed is a
user-defined ClassLoader and if so set a boolean that acts as the guard on the
virtual frame walk. If additional non-user-defined ClassLoaders are declared
in the future beyond the ones identified in this patch, they can be added to
the registry.
There are currently 4 non-user-defined ClassLoader classes, so the registry is
not difficult to maintain. Nor is the registry difficult to search when
ClassLoaders are constructed. The guard boolean that records whether a
user-defined ClassLoader has been constructed transitions from false to true
and never becomes false again. (An extension to transition the boolean back to
false when a user-defined ClassLoader becomes unreachable is beyond the scope
of this proposal.)
This proposal slows loading of non-user-defined ClassLoaders by the time it
takes to register them. It slows ClassLoader construction by the time it takes
to consult the registry and conditionally set the guard boolean. At each
invocation of ObjectInputStream.resolveClass(ObjectInputStreamClass), the guard
boolean is used to possibly avoid the virtual frame walk.
Tested with `make run-test-tier1` on Linux (CentOS 7) on both aarch64 and
x86_64. I had one failure on each of the runs
ACTION: main -- Failed. Execution failed: `main' threw exception:
java.lang.Error: TESTBUG: the library has not been found in
${Repo}/build/linux-aarch64-server-release/images/test/hotspot/jtreg/native
REASON: User specified action: run main/native GTestWrapper
on both plaforms. But I get the same failure when I run the tier1 tests on
clones of OpenJDK-15+25 on both aarch64 and x86_64.
Running a sample (representative?) application with the Oracle Developer Studio
analyzer shows a performance comparison of
Stack Fragment sorted by metric: Attributed Total CPU Time
baseline.er experiment.er
Attributed Attributed Name
Total CPU Time Total CPU Time
sec. sec.
================================== Callers
3459.210 29.931
java.io.ObjectInputStream.readOrdinaryObject(boolean)
1139.727 3.532
java.io.ObjectInputStream.readArray(boolean)
875.262 9.116
java.io.ObjectInputStream.readNonProxyDesc(boolean)
================================== Stack Fragment
java.io.ObjectInputStream.readClassDesc(boolean)
java.io.ObjectInputStream.readNonProxyDesc(boolean)
java.io.ObjectInputStream.resolveClass(java.io.ObjectStreamClass)
java.io.ObjectInputStream.latestUserDefinedLoader()
4.173 3.953
jdk.internal.misc.VM.latestUserDefinedLoader()
================================== Callees
5470.026 0.
jdk.internal.misc.VM.latestUserDefinedLoader0()
0. 38.627
java.lang.ClassLoader.getSystemClassLoader()
The `hg export -g` below is with respect to OpenJDK-15+25.
Thank you for your review comments. I will need a sponsor to get this change
into the repository.
... peter
# HG changeset patch
# User Peter Kessler <peter.kess...@os.amperecomputing.com>
# Date 1591310467 25200
# Thu Jun 04 15:41:07 2020 -0700
# Node ID 9a39488182c1dfc5bc7bb41d562d7ef16ee657f6
# Parent 90b266a84c06f1b3dc0ed8767856793e8c1c357e
Improve the performance of ObjectInputStream.resolveClass
diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java
b/src/java.base/share/classes/java/lang/ClassLoader.java
--- a/src/java.base/share/classes/java/lang/ClassLoader.java
+++ b/src/java.base/share/classes/java/lang/ClassLoader.java
@@ -385,6 +385,8 @@
}
this.package2certs = new ConcurrentHashMap<>();
this.nameAndId = nameAndId(this);
+ /* Note the construction of a ClassLoader. */
+ VM.noteClassLoaderConstruction(this.getClass());
}
/**
diff --git a/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
b/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
--- a/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
+++ b/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
@@ -117,6 +117,11 @@
protected Class<?> loadClassOrNull(String cn, boolean resolve) {
return JLA.findBootstrapClassOrNull(this, cn);
}
+
+ static {
+ /* Register this ClassLoader as not a user-defined ClassLoader. */
+ VM.registerNotUserDefinedClassLoader(BootClassLoader.class);
+ }
};
/**
@@ -127,6 +132,8 @@
static {
if (!ClassLoader.registerAsParallelCapable())
throw new InternalError();
+ /* Register this ClassLoader as not a user-defined ClassLoader. */
+ VM.registerNotUserDefinedClassLoader(PlatformClassLoader.class);
}
PlatformClassLoader(BootClassLoader parent) {
@@ -142,6 +149,8 @@
static {
if (!ClassLoader.registerAsParallelCapable())
throw new InternalError();
+ /* Register this ClassLoader as not a user-defined ClassLoader. */
+ VM.registerNotUserDefinedClassLoader(AppClassLoader.class);
}
final URLClassPath ucp;
diff --git a/src/java.base/share/classes/jdk/internal/misc/VM.java
b/src/java.base/share/classes/jdk/internal/misc/VM.java
--- a/src/java.base/share/classes/jdk/internal/misc/VM.java
+++ b/src/java.base/share/classes/jdk/internal/misc/VM.java
@@ -304,12 +304,44 @@
private static final int JVMTI_THREAD_STATE_WAITING_INDEFINITELY = 0x0010;
private static final int JVMTI_THREAD_STATE_WAITING_WITH_TIMEOUT = 0x0020;
+ /** A registry of not user-defined ClassLoaders. */
+ private static final List<Class<? extends ClassLoader>>
notUserDefinedClassLoaderRegistry =
+ Collections.synchronizedList(new
ArrayList<>());
+
+ /** Register a ClassLoader class as being not a user-defined ClassLoader.
*/
+ public static void registerNotUserDefinedClassLoader(Class<? extends
ClassLoader> classLoaderClass) {
+ notUserDefinedClassLoaderRegistry.add(classLoaderClass);
+ }
+
+ /** A flag for whether a user-defined ClassLoaders has been constructed. */
+ private static volatile boolean constructedUserDefinedClassLoaderFlag =
false;
+
+ /**
+ * Note a ClassLoader construction, and record if a
+ * user-defined ClassLoader has been constructed.
+ */
+ public static void noteClassLoaderConstruction(Class<? extends
ClassLoader> classLoaderClass) {
+ /* Check if the ClassLoader class not a user-defined ClassLoader. */
+ if (notUserDefinedClassLoaderRegistry.contains(classLoaderClass)) {
+ return;
+ }
+ constructedUserDefinedClassLoaderFlag = true;
+ return;
+ }
+
/*
* Returns the first user-defined class loader up the execution stack,
* or the platform class loader if only code from the platform or
* bootstrap class loader is on the stack.
*/
public static ClassLoader latestUserDefinedLoader() {
+ if (!constructedUserDefinedClassLoaderFlag) {
+ /*
+ * If no user-defined ClassLoader has been constructed,
+ * then I will not find a user-defined ClassLoader in the stack.
+ */
+ return ClassLoader.getSystemClassLoader();
+ }
ClassLoader loader = latestUserDefinedLoader0();
return loader != null ? loader : ClassLoader.getPlatformClassLoader();
}
diff --git a/src/java.base/share/classes/jdk/internal/reflect/ClassDefiner.java
b/src/java.base/share/classes/jdk/internal/reflect/ClassDefiner.java
--- a/src/java.base/share/classes/jdk/internal/reflect/ClassDefiner.java
+++ b/src/java.base/share/classes/jdk/internal/reflect/ClassDefiner.java
@@ -30,6 +30,7 @@
import jdk.internal.access.JavaLangAccess;
import jdk.internal.access.SharedSecrets;
+import jdk.internal.misc.VM;
/** Utility class which assists in calling defineClass() by
creating a new class loader which delegates to the one needed in
@@ -73,4 +74,9 @@
DelegatingClassLoader(ClassLoader parent) {
super(parent);
}
+
+ static {
+ /* Register this ClassLoader as not a user-defined ClassLoader. */
+ VM.registerNotUserDefinedClassLoader(DelegatingClassLoader.class);
+ }
}
