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.
