Skip to content

8342283: CDS cannot handle a large number of classes #21797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
iklam wants to merge 3 commits into from
Closed

8342283: CDS cannot handle a large number of classes #21797

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0320ba9
8342283: CDS cannot handle a large number of classes
iklam Oct 31, 2024
6f76fd8
Merge branch 'master' into 8342283-cds-cannot-handle-lots-of-classes
iklam Nov 28, 2024
42296ac
Use ArrayList::trimToSize() instead of reallocation; there is no equi…
iklam Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions src/java.base/share/classes/java/lang/ClassLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import jdk.internal.reflect.CallerSensitiveAdapter;
import jdk.internal.reflect.Reflection;
import jdk.internal.util.StaticProperty;
import jdk.internal.vm.annotation.Stable;

/**
* A class loader is an object that is responsible for loading classes. The
Expand Down Expand Up @@ -290,10 +291,10 @@
// class loader is parallel capable.
// Note: VM also uses this field to decide if the current class loader
// is parallel capable and the appropriate lock object for class loading.
private final ConcurrentHashMap<String, Object> parallelLockMap;
private @Stable ConcurrentHashMap<String, Object> parallelLockMap;
Copy link
Member

@shipilev shipilev Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we reworked memory barriers for @Stable fields (JDK-8333791), we had a discussion on memory model implications of @Stable. There were competing proposals, but we have settled on "@Stable behave like final-s, in order to provide safety margin for uses where we accidentally rely on this property for some final-like fields". This change introduces such the "accident", AFAICS.


// Maps packages to certs
private final ConcurrentHashMap<String, Certificate[]> package2certs;
private @Stable ConcurrentHashMap<String, Certificate[]> package2certs;

// Shared among all packages with unsigned classes
private static final Certificate[] nocerts = new Certificate[0];
Expand Down Expand Up @@ -322,7 +323,7 @@
// Class::getPackage, ClassLoader::getDefinePackage(s) or
// Package::getPackage(s) method is called to define it.
// Otherwise, the value is a NamedPackage object.
private final ConcurrentHashMap<String, NamedPackage> packages
private @Stable ConcurrentHashMap<String, NamedPackage> packages
= new ConcurrentHashMap<>();

/*
Expand Down Expand Up @@ -2572,15 +2573,20 @@
}

/**
* Called by the VM, during -Xshare:dump
* Called only by the VM, during -Xshare:dump.
*
* @implNote This is done while the JVM is running in single-threaded mode,
* and at the very end of Java bytecode execution. We know that no more classes
* will be loaded and none of the fields modified by this method will be used again.
Comment on lines +2578 to +2580
Copy link
Member

@shipilev shipilev Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if moving all this to VM side, which is not bound to Java language rules, would be conceptually cleaner. It would be more work, but I think there any only three classes that implement resetArchivedStates, which limits the scope of the change. I am prototyping this locally...

Copy link
Member

@shipilev shipilev Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this: master...shipilev:jdk:JDK-8342283-cds-reset-archived-states. This passes runtime/cds at least.

*/
private void resetArchivedStates() {
if (parallelLockMap != null) {
parallelLockMap.clear();
parallelLockMap = new ConcurrentHashMap<>();
}
packages.clear();
package2certs.clear();
classes.clear();
packages = new ConcurrentHashMap<>();
package2certs = new ConcurrentHashMap<>();
classes.clear();

Check failure on line 2588 in src/java.base/share/classes/java/lang/ClassLoader.java

View check run for this annotation

openjdk / jcheck-openjdk/jdk-21797

Whitespace error 

Column 0: tab
classes.trimToSize();
classLoaderValueMap = null;
}
}
Expand Down