sdedic commented on code in PR #6309:
URL: https://github.com/apache/netbeans/pull/6309#discussion_r1295821065
##########
platform/o.n.bootstrap/src/org/netbeans/ProxyClassPackages.java:
##########
@@ -32,46 +31,43 @@
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);
+ static void removeCoveredPakcages(ProxyClassLoader loader) {
+ packageCoverage.values().removeIf( (Set<ProxyClassLoader> set) -> {
Review Comment:
I was not sure about this one, the add/remove concurrency would yield IMHO
the same results as the original code, but `find` is allowed to see an
intermediate results, with just some matching classloaders removed. This is
probably not a big deal however provided that data for a single package is
consistent; there's no query across different packages.
##########
platform/o.n.bootstrap/src/org/netbeans/ProxyClassPackages.java:
##########
@@ -32,46 +31,43 @@
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);
+ static void removeCoveredPakcages(ProxyClassLoader loader) {
+ packageCoverage.values().removeIf( (Set<ProxyClassLoader> set) -> {
if (set.contains(loader) && set.size() == 1) {
Review Comment:
This inner set is not synchronized - this should be synchronized with
possible `add` work on the collection value -- especially the decision based on
size().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
