Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 9 additions & 18 deletions core/src/main/java/hudson/PluginManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@
import hudson.security.ACLContext;
import hudson.security.Permission;
import hudson.security.PermissionScope;
import hudson.util.CachingClassLoader;
import hudson.util.CyclicGraphDetector;
import hudson.util.CyclicGraphDetector.CycleDetectedException;
import hudson.util.DelegatingClassLoader;
import hudson.util.ExistenceCheckingClassLoader;
import hudson.util.FormValidation;
import hudson.util.PersistedList;
Expand Down Expand Up @@ -108,13 +108,11 @@
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.ServiceLoader;
import java.util.Set;
import java.util.TreeMap;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.Future;
import java.util.function.Supplier;
Expand Down Expand Up @@ -2402,12 +2400,9 @@ public <T> T of(String key, Class<T> type, Supplier<T> func) {
/**
* {@link ClassLoader} that can see all plugins.
*/
public static final class UberClassLoader extends DelegatingClassLoader {
public static final class UberClassLoader extends CachingClassLoader {
private final List<PluginWrapper> activePlugins;

/** Cache of loaded, or known to be unloadable, classes. */
private final ConcurrentMap<String, Optional<Class<?>>> loaded = new ConcurrentHashMap<>();

/**
* The servlet container's {@link ClassLoader} (the parent of Jenkins core) is
* parallel-capable and maintains its own growing {@link Map} of {@link
Expand All @@ -2426,29 +2421,29 @@ public UberClassLoader(List<PluginWrapper> activePlugins) {
}

@Override
protected Class<?> findClass(String name) throws ClassNotFoundException {
protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
for (String namePrefixToSkip : CLASS_PREFIXES_TO_SKIP) {
if (name.startsWith(namePrefixToSkip)) {
throw new ClassNotFoundException("ignoring " + name);
}
}
return loaded.computeIfAbsent(name, this::computeValue).orElseThrow(() -> new ClassNotFoundException(name));
return super.loadClass(name, resolve);
}

private Optional<Class<?>> computeValue(String name) {
@Override
protected Class<?> findClass(String name) throws ClassNotFoundException {
for (PluginWrapper p : activePlugins) {
try {
if (FAST_LOOKUP) {
return Optional.of(ClassLoaderReflectionToolkit.loadClass(p.classLoader, name));
return ClassLoaderReflectionToolkit.loadClass(p.classLoader, name);
} else {
return Optional.of(p.classLoader.loadClass(name));
return p.classLoader.loadClass(name);
}
} catch (ClassNotFoundException e) {
// Not found. Try the next class loader.
}
}
// Not found in any of the class loaders. Delegate.
return Optional.empty();
throw new ClassNotFoundException(name);
}

@Override
Expand Down Expand Up @@ -2480,10 +2475,6 @@ protected Enumeration<URL> findResources(String name) throws IOException {
return Collections.enumeration(resources);
}

void clearCacheMisses() {
loaded.values().removeIf(Optional::isEmpty);
}

@Override
public String toString() {
// only for debugging purpose
Expand Down
56 changes: 56 additions & 0 deletions core/src/main/java/hudson/util/CachingClassLoader.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package hudson.util;

import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

/**
*
* ClassLoader with internal caching of class loading results.
*
* <p>
* Caches both successful and failed class lookups to avoid redundant delegation
* and repeated class resolution attempts. Designed for performance optimization
* in systems that repeatedly query class presence (e.g., plugin environments,
* reflective loading, optional dependencies).
* </p>
*
* Useful for classloaders that have heavy-weight loadClass() implementations
*
* @author Dmytro Ukhlov
*/
@Restricted(NoExternalUse.class)
public class CachingClassLoader extends DelegatingClassLoader {
private final ConcurrentHashMap<String, Object> cache = new ConcurrentHashMap<>();

public CachingClassLoader(String name, ClassLoader parent) {
super(name, parent);
}

@Override
protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
Object classOrEmpty = cache.computeIfAbsent(name, key -> {
try {
return super.loadClass(name, false);
} catch (ClassNotFoundException e) {
// Not found.
return Optional.empty();
}
});

if (classOrEmpty == Optional.empty()) {
throw new ClassNotFoundException(name);
}

Class<?> clazz = (Class<?>) classOrEmpty;
if (resolve) {
resolveClass(clazz);
}
return clazz;
}

public void clearCacheMisses() {
cache.values().removeIf(v -> v == Optional.empty());
}
Comment on lines +25 to +55
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Maybe the using Optional as map value type could make its usage more natural. WDYT?

Suggested change
private final ConcurrentHashMap<String, Object> cache = new ConcurrentHashMap<>();
public CachingClassLoader(String name, ClassLoader parent) {
super(name, parent);
}
@Override
protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
Object classOrEmpty = cache.computeIfAbsent(name, key -> {
try {
return super.loadClass(name, false);
} catch (ClassNotFoundException e) {
// Not found.
return Optional.empty();
}
});
if (classOrEmpty == Optional.empty()) {
throw new ClassNotFoundException(name);
}
Class<?> clazz = (Class<?>) classOrEmpty;
if (resolve) {
resolveClass(clazz);
}
return clazz;
}
public void clearCacheMisses() {
cache.values().removeIf(v -> v == Optional.empty());
}
private final ConcurrentHashMap<String, Optional<Class<?>>> cache = new ConcurrentHashMap<>();
public CachingClassLoader(String name, ClassLoader parent) {
super(name, parent);
}
@Override
protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
Optional<Class<?>> classOrEmpty = cache.computeIfAbsent(name, key -> {
try {
return Optional.of(super.loadClass(name, false));
} catch (ClassNotFoundException e) {
// Not found.
return Optional.empty();
}
});
if (classOrEmpty.isEmpty()) {
throw new ClassNotFoundException(name);
}
Class<?> clazz = classOrEmpty.get();
if (resolve) {
resolveClass(clazz);
}
return clazz;
}
public void clearCacheMisses() {
cache.values().removeIf(Optional::isEmpty);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wrapping with Optional consumes a bit more memory, so I decided not to use it. However, I agree it makes the code more natural. Let's wait for further reviews before making a final decision.

}
Loading