RFR (S) 8057936: java.net.URLClassLoader.findClass uses exceptions in control flow

[Claes Redestad]

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
>>>
>>>
>>
>