Initial webrev with changes for JDK 9

[Alan Bateman]

On 11/03/2016 16:50, Paul Sandoz wrote:
> :
> I browsed the code. Generally the feeling i get is it’s of good quality. Previously some of this area was quite legacy and it is refreshing to view more modern code.
Thanks for going through it. There is quite a mix here, lots of new code 
but lots of ancient code has to be changed along the way.

>
> Comments below, only the first is something i think we should really change, the others are up to you.
>
> Once Jigsaw goes in i wonder if we should opportunistically revisit the use of URL and Enumeration with class loaders?
Potentially, it's just hasn't been interesting so far because the legacy 
ClassLoader methods aren't specified to locate resources in modules.


> :
>
>
> The method getResourceAsStream has an optimisation for a BuiltinClassLoader:
>
> :
>
> Is that something you plan to include for getResource too at some point?
getResource returns a URL so the optimization to avoid the going through 
the URL protocol handler isn't applicable.


>
> If you can syncrhonize on this, just use double checked locking?
I'll look at it, part of the reason for the currently implementation is 
that we were setting several fields.


>
>
>
> j.l.i.BoundMethodHandle
> —
>
>   791                 // ## FIXME
>   792                 // assert F_SPECIES_DATA.getDeclaredAnnotation(Stable.class) != null;
>
> That’s odd. Why did that need commenting out?
getDeclarationAnnotation => annotation parsing and creating proxies and 
too early in the VM startup for that. It was commented out in one of the 
numerous sync ups I think.

I'll ask a comment to it.


>
>
> MethodHandles
> —
>
> That’s quite some trick to create PUBLIC_LOOKUP :-) spinning up a new class (“Unamed”) in a new class loader. If that mechanism is gonna stay and If there are costs associated with that, we could easily skip the ASM part and use hard coded bytes.
I think TBD, I think John would like PL to revert to using Object as the 
lookup class but requires extending the lookup modes. I have an issue in 
JIRA to track another phase of work here.


> :
>
>
>
>
>   198                 while (c.isArray()) {
>   199                     c = c.getComponentType();
>   200                 }
>
> This pattern is occurring a number of times.
>
> Memo: add a method on Arrays, or somewhere…,
I agree, we have several places that need the package name but they can 
be called with array objects.


>
>
> sun.net.www.protocol.jrt.JavaRuntimeURLConnection
> —
>
>    56     private static final ImageReader reader = ImageReaderFactory.getImageReader();
>
> reader -> READER
okay.

>
>
> java.lang.module.ModuleInfo
> —
>
> :
>
> Consider using Set.of
I agree, it pre-dates Set.of of course.


>
> j.l.ModuleReader
> —
>
> :
>
> Use InputStream.readAllBytes?
I meant to do that this, this code pre-dates readAllBytes.


>
>
> jdk.internal.module.Builder
> —
>
>    56     private static final Set<Requires.Modifier> MANDATED =
>    57         Collections.singleton(Requires.Modifier.MANDATED);
>    58     private static final Set<Requires.Modifier> PUBLIC =
>    59         Collections.singleton(Requires.Modifier.PUBLIC);
>
> Set.of ?
Okay.