Hi Paul, On 19 Jun 2015, at 16:39, Paul Sandoz <paul.san...@oracle.com> wrote:
> > On Jun 18, 2015, at 2:08 AM, Jim Laskey (Oracle) <james.las...@oracle.com> > wrote: > >> https://bugs.openjdk.java.net/browse/JDK-8080511 >> >> This is an long overdue refresh of the jimage support in the JDK9-dev repo. >> This includes native support for reading jimage files, improved jrt-fs (java >> runtime file system) support for retrieving modules and packages from the >> runtime, and improved performance for langtools in the presence of jrt-fs. >> >> http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-top >> <http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-top> >> http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-jdk >> <http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-jdk> > > I eyeballed the JDK changes. Just two thing: > > make/src/classes/build/tools/module/ModuleArchive.java > > 90 @Override > 91 public Stream<Entry> entries() { > 92 List<Entry> entries = new ArrayList<>(); > 93 try { > 94 if (classes != null) > 95 Files.walk(classes) > 96 .sorted() > 97 .filter(p -> !Files.isDirectory(p) > 98 && > !classes.relativize(p).toString().startsWith("_the.") > 99 && > !classes.relativize(p).toString().equals("javac_state")) > 100 .map(p -> toEntry(p, classes, > EntryType.CLASS_OR_RESOURCE)) > 101 .forEach(entries::add); > 102 if (cmds != null) > 103 Files.walk(cmds) > 104 .filter(p -> !Files.isDirectory(p)) > 105 .map(p -> toEntry(p, cmds, EntryType.NATIVE_CMD)) > 106 .forEach(entries::add); > 107 if (libs != null) > 108 Files.walk(libs) > 109 .filter(p -> !Files.isDirectory(p)) > 110 .map(p -> toEntry(p, libs, EntryType.NATIVE_LIB)) > 111 .forEach(entries::add); > 112 if (configs != null) > 113 Files.walk(configs) > 114 .filter(p -> !Files.isDirectory(p)) > 115 .map(p -> toEntry(p, configs, EntryType.CONFIG)) > 116 .forEach(entries::add); > 117 } catch (IOException ioe) { > 118 throw new UncheckedIOException(ioe); > 119 } > 120 return entries.stream(); > 121 } > > You can use collect(toList()) ==> OK collect used. In addition, filter first then sort, tryWithResource for 4 Files stream. > > In general the contract for Archive.entries probably has to say the stream > needs to be closed after use since it might cover lazy I/O based resources, > so callers will need to use a try-with-resources block. ==> Added a note in javadoc, implemented explicit close for non lazy streams, added missing tryWithResource. Added a comment on what should be done in ModuleArchive to keep laziness. > > Paul. Thanks. > >> http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-hotspot >> <http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-hotspot> >> http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-langtools >> <http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-langtools> >> >> >> Details: >> >> - jrt-fs provides access, via the nio FileSystem API, to the classes in a >> .jimage file, organized by module or by package. >> - Shared code for jimage support converted to native. Currently residing in >> hotspot, but will migrate to it’s own jdk library >> https://bugs.openjdk.java.net/browse/JDK-8087181 >> <https://bugs.openjdk.java.net/browse/JDK-8087181> >> - A new archive abstraction for class/resource sources. >> - java based implementation layer for jimage reading to allow backport to >> JDK8 (jrt-fs.jar - IDE support.) >> - JNI support for jimage into hotspot. >> - White box tests written to exercise native jimage support. >> >
