Skip to content

Commit

Permalink
For ProxyClassLoader:
Browse files Browse the repository at this point in the history 
- Use ConcurrentMap for 'packages' map and remove some synchronized blocks
- Use ConcurrentHashMap.newKeySet() instead of
  Collections.synchronizedSet for 'ARBITRARY_LOAD_WARNINGS' Set
- Move some logic outside a sync block in 'getPackageFast' method
- Remove unused variables
For ProxyClassPackages:
- Use ConcurrentMap for 'PACKAGE_COVERAGE' map and remove the
  synchronized methods for the get operation
- Use synchronized on the 'PACKAGE_COVERAGE' map for the add and remove
  operations, they need to be linearizable.
- The 'findCoveredPkg' method should return a consistent value. The
  'computeIfPresent' implementation from ConcurrentHashMap
  synchronize the field that is reading to prevent consistency issues
  with the other methods (add or remove) modifying the Map.

Thanks to mbien, neilcsmith, and sdedic.
  • Loading branch information
@pepness
pepness committed Sep 29, 2023
1 parent 2196e46 commit 0f08732
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 54 deletions.
46 changes: 24 additions & 22 deletions platform/o.n.bootstrap/src/org/netbeans/ProxyClassLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.function.BiFunction;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -62,7 +64,7 @@ public class ProxyClassLoader extends ClassLoader {
/** All known packages
* @GuardedBy("packages")
*/
private final Map<String, Package> packages = new HashMap<String, Package>();
private final ConcurrentMap<String, Package> packages = new ConcurrentHashMap<>();

/** keeps information about parent classloaders, system classloader, etc.*/
volatile ProxyClassParents parents;
Expand Down Expand Up @@ -107,7 +109,6 @@ public void append(ClassLoader[] nueparents) throws IllegalArgumentException {
if (cl == null) throw new IllegalArgumentException("null parent: " + Arrays.asList(nueparents)); // NOI18N
}

ProxyClassLoader[] resParents = null;
ModuleFactory moduleFactory = Lookup.getDefault().lookup(ModuleFactory.class);
if (moduleFactory != null && moduleFactory.removeBaseClassLoader()) {
// this hack is here to prevent having the application classloader
Expand Down Expand Up @@ -232,7 +233,6 @@ private Class<?> doFindClass(String name) throws ClassNotFoundException {
}

private String diagnosticCNFEMessage(String base, Set<ProxyClassLoader> del) {
String parentSetS;
int size = parents.size();
// Too big to show in its entirety - overwhelms the log file.
StringBuilder b = new StringBuilder();
Expand All @@ -250,7 +250,7 @@ private String diagnosticCNFEMessage(String base, Set<ProxyClassLoader> del) {
b.append(']');
return b.toString();
}
private static final Set<String> arbitraryLoadWarnings = Collections.synchronizedSet(new HashSet<String>());
private static final Set<String> arbitraryLoadWarnings = ConcurrentHashMap.newKeySet();

/** May return null */
private synchronized Class<?> selfLoadClass(String pkg, String name) {
Expand All @@ -266,7 +266,7 @@ private synchronized Class<?> selfLoadClass(String pkg, String name) {
}
if (LOG_LOADING && !name.startsWith("java.")) LOGGER.log(Level.FINEST, "{0} loaded {1}",
new Object[] {this, name});
}
}
return cls;
}

Expand Down Expand Up @@ -468,28 +468,32 @@ protected Package getPackage(String name) {
* @return located package, or null
*/
protected Package getPackageFast(String name, boolean recurse) {
synchronized (packages) {
Package pkg = packages.get(name);
if (pkg != null) {
return pkg;
}
if (!recurse) {
return null;
}
Package pkg = packages.get(name);
if (pkg != null) {
return pkg;
}
if (!recurse) {
return null;
}
synchronized(packages) {
pkg = packages.get(name);
String path = name.replace('.', '/');
for (ProxyClassLoader par : this.parents.loaders()) {
if (!shouldDelegateResource(path, par))
if (!shouldDelegateResource(path, par)) {
continue;
}
pkg = par.getPackageFast(name, false);
if (pkg != null) break;
if (pkg != null) {
break;
}
}
// pretend the resource ends with "/". This works better with hidden package and
// prefix-based checks.
if (pkg == null && shouldDelegateResource(path + "/", null)) {
// Cannot access either Package.getSystemPackages nor ClassLoader.getPackage
// from here, so do the best we can though it will cause unnecessary
// duplication of the package cache (PCL.packages vs. CL.packages):
pkg = super.getPackage(name);
pkg = super.getPackage(name);
}
if (pkg != null) {
packages.put(name, pkg);
Expand All @@ -508,12 +512,10 @@ protected Package definePackage(String name, String specTitle,
String specVersion, String specVendor, String implTitle,
String implVersion, String implVendor, URL sealBase )
throws IllegalArgumentException {
synchronized (packages) {
Package pkg = super.definePackage(name, specTitle, specVersion, specVendor, implTitle,
Package pkg = super.definePackage(name, specTitle, specVersion, specVendor, implTitle,
implVersion, implVendor, sealBase);
packages.put(name, pkg);
return pkg;
}
packages.put(name, pkg);
return pkg;
}

/**
Expand Down Expand Up @@ -585,7 +587,7 @@ final ClassLoader firstParent() {
// System Class Loader Packages Support
//

private static Map<String,Boolean> sclPackages = Collections.synchronizedMap(new HashMap<String,Boolean>());
private static ConcurrentMap<String,Boolean> sclPackages = new ConcurrentHashMap<>();
private static Boolean isSystemPackage(String pkg) {
return sclPackages.get(pkg);
}
Expand Down
64 changes: 32 additions & 32 deletions platform/o.n.bootstrap/src/org/netbeans/ProxyClassPackages.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@
package org.netbeans;

import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

/** Keeps the coverage of various packages by existing ProxyClassLoaders.
*
Expand All @@ -32,47 +31,48 @@
final class ProxyClassPackages {
private ProxyClassPackages() {
}

/** A shared map of all packages known by all classloaders. Also covers META-INF based resources.
* It contains two kinds of keys: dot-separated package names and slash-separated
* META-INF resource names, e.g. {"org.foobar", "/services/org.foobar.Foo"}
*/
private static final Map<String, Set<ProxyClassLoader>> packageCoverage = new HashMap<String, Set<ProxyClassLoader>>();
private static final ConcurrentMap<String, Set<ProxyClassLoader>> packageCoverage = new ConcurrentHashMap<>();

static synchronized void addCoveredPackages(
ProxyClassLoader loader, Iterable<String> coveredPackages
) {
for (String pkg : coveredPackages) {
Set<ProxyClassLoader> delegates = ProxyClassPackages.packageCoverage.get(pkg);
if (delegates == null) {
delegates = Collections.<ProxyClassLoader>singleton(loader);
ProxyClassPackages.packageCoverage.put(pkg, delegates);
} else if (delegates.size() == 1) {
delegates = new HashSet<ProxyClassLoader>(delegates);
ProxyClassPackages.packageCoverage.put(pkg, delegates);
delegates.add(loader);
} else {
delegates.add(loader);
static void addCoveredPackages( ProxyClassLoader loader, Iterable<String> coveredPackages) {
synchronized(packageCoverage) {
for (String pkg : coveredPackages) {
Set<ProxyClassLoader> delegates = ProxyClassPackages.packageCoverage.get(pkg);
if (delegates == null) {
delegates = Collections.<ProxyClassLoader>singleton(loader);
ProxyClassPackages.packageCoverage.put(pkg, delegates);
} else if (delegates.size() == 1) {
delegates = new HashSet<ProxyClassLoader>(delegates);
ProxyClassPackages.packageCoverage.put(pkg, delegates);
delegates.add(loader);
} else {
delegates.add(loader);
}
}
}
}

static synchronized void removeCoveredPakcages(
ProxyClassLoader loader
) {
for (Iterator<String> it = ProxyClassPackages.packageCoverage.keySet().iterator(); it.hasNext();) {
String pkg = it.next();
Set<ProxyClassLoader> set = ProxyClassPackages.packageCoverage.get(pkg);
if (set.contains(loader) && set.size() == 1) {
it.remove();
} else {
set.remove(loader);
}
static void removeCoveredPakcages(ProxyClassLoader loader) {
synchronized (packageCoverage) {
packageCoverage.values().removeIf( (Set<ProxyClassLoader> set) -> {
if (set.contains(loader) && set.size() == 1) {
return true;
} else {
set.remove(loader);
return false;
}
} );
}
}

static synchronized Set<ProxyClassLoader> findCoveredPkg(String pkg) {
return packageCoverage.get(pkg);
static Set<ProxyClassLoader> findCoveredPkg(String pkg) {
return packageCoverage.computeIfPresent(pkg, (k, v) -> {
return v;
});
}

}

0 comments on commit 0f08732

Please sign in to comment.