8154956: Module system implementation refresh (4/2016)

[Andrej Golovnin]

Hi Peter,

src/java.base/share/classes/java/lang/module/ModuleReader.java

 146         if (oin.isPresent()) try (InputStream in = oin.get()) {
 147             return Optional.of(ByteBuffer.wrap(in.readAllBytes()));
 148         } else {
 149             return Optional.empty();
 150         }

I highly doubt that the line 146 is easy to read. Can you change it into:

 146         if (oin.isPresent()) {
 147             try (InputStream in = oin.get()) {
 148                 return Optional.of(ByteBuffer.wrap(in.readAllBytes()));
 149             }
 150         }
 151         return Optional.empty();

And maybe you can add following changes to your patch too.

http://cr.openjdk.java.net/~plevart/jdk9-jake/ModuleSystem.referesh.201604/webrev.02/src/java.base/share/classes/java/lang/module/ModulePath.java.html

The following lines use the String-based version of the #lastIndexOf-method.

341         int index = cf.lastIndexOf("/") + 1;
391         int i = fn.lastIndexOf(File.separator);
568         int index = cn.lastIndexOf("/");

They should be changed to use the char-based version.

I don't know how often the method #deriveModuleDescriptor(JarFile)
is executed. But compiling regular expressions over and over doesn't really
helps from the performance standpoint of view:

 400         Matcher matcher = Pattern.compile("-(\\d+(\\.|$))").matcher(mn);

 415         mn =  mn.replaceAll("[^A-Za-z0-9]", ".")  // replace
non-alphanumeric
 416                 .replaceAll("(\\.)(\\1)+", ".")   // collapse
repeating dots
 417                 .replaceAll("^\\.", "")           // drop leading dots
 418                 .replaceAll("\\.$", "");          // drop trailing dots

Maybe the regular expressions in the above lines should be precompiled
in static final fields (Pattern objects are immutable and thread safe)
to improve
the performance of the #deriveModuleDescriptor(JarFile)-method.

Best regards,
Andrej Golovnin