Yes, this saves creating the type and wrapper object altogether, while
testing suggest it's only the
getResource() == null cases wecommonly seehurting startup in our tests.
I'd say let's go with your patch.
New webrev: http://cr.openjdk.java.net/~redestad/8057936/webrev.4/
Benchmark referred to in bug (java -jar benchmarks.jar -f 4 -wi 5 -i 10
-p file=classes.jar)show
similar benefits (somewhere in the 4-12% range, statistically):
Benchmark (file) Mode Samples Score
Score error Units
baseline:
o.o.ClassLoaderBenchmark.loadAll classes.jar avgt 40
0.679 0.037 s/op
patched:
o.o.ClassLoaderBenchmark.loadAll classes.jar avgt 40
0.598 0.007 s/op
/Claes
ps. I think lambdas give little or no benefit here, and as Alan points
out it could lead to tricky
classloading bugs.
On 09/10/2014 01:21 PM, Michael McMahon wrote:
or how about just returning a null Class<?> from the privileged block
instead of the new result type only in the case where
URLClassPath.getResource() returns null?
That's the main "normal" case where the resource doesn't exist, I think.
If defineClass() throws an IOException, then that is more likely to be
a "genuine" exception
- Michael
diff --git a/src/java.base/share/classes/java/net/URLClassLoader.java
b/src/java.base/share/classes/java/net/URLClassLoader.java
--- a/src/java.base/share/classes/java/net/URLClassLoader.java
+++ b/src/java.base/share/classes/java/net/URLClassLoader.java
@@ -356,8 +356,9 @@
protected Class<?> findClass(final String name)
throws ClassNotFoundException
{
+ final Class<?> result;
try {
- return AccessController.doPrivileged(
+ result = AccessController.doPrivileged(
new PrivilegedExceptionAction<Class<?>>() {
public Class<?> run() throws
ClassNotFoundException {
String path = name.replace('.',
'/').concat(".class");
@@ -369,13 +370,17 @@
throw new
ClassNotFoundException(name, e);
}
} else {
- throw new ClassNotFoundException(name);
+ return null;
}
}
}, acc);
} catch (java.security.PrivilegedActionException pae) {
throw (ClassNotFoundException) pae.getException();
}
+ if (result == null) {
+ throw new ClassNotFoundException(name);
+ }
+ return result;
}
/*
On 10/09/14 11:55, Ivan Gerasimov wrote:
If a lambda were used instead of an anonymous class, it would save us
one line of code :-)
Sincerely yours,
Ivan
On 10.09.2014 14:11, Claes Redestad wrote:
Hi,
please review this simple patch to avoid raising
PrivilegedActionExceptions
when failing to find a class in URLClassLoader.
bug: https://bugs.openjdk.java.net/browse/JDK-8057936
webrev: http://cr.openjdk.java.net/~redestad/8057936/webrev.2/
/Claes
