Hi, Here’s a new webrev that addresses the issues Paul brought up. The versioned entry cache has been removed, the search space has been reduced, the documentation for setVersioned(int) and setRuntimeVersioned() has been updated to clarify when IllegalStateException is thrown, and the tests have been changed to accommodate a jar file with versions 9 and 10, rather than 8 and 9.
Issue: https://bugs.openjdk.java.net/browse/JDK-8132734 <https://bugs.openjdk.java.net/browse/JDK-8132734> JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305 <https://bugs.openjdk.java.net/browse/JDK-8047305> Webrev: http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/ <http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/> Steve > On Nov 4, 2015, at 1:01 AM, Paul Sandoz <paul.san...@oracle.com> wrote: > > Hi Steve, > > Hi Steve, > > I don’t think we need to cache versioned entries (as we discussed a while > back). For class loading it’s likely to increase memory costs without any > performance benefit (if anything a performance decrease). It’s easy to add > back later on if we have data that suggests otherwise. > > We can reduce the search space for searching for versioned entries by setting > the lower bound of the base version to one minus the first Java major release > that has runtime support for multi-release jar files i.e. 8 as it currently > stands. > > 312 * @throws IllegalStateException if called after an entry has been > read > > … after an entry has been obtained (see ….) > > You might need some further clarification in setVersioned/setRuntimeVersioned > itself describing the lifecycle e.g. > > this method may be called one or more times after construction and before > the first operation that obtains an entry, > after which the jar file configuration’s is considered immutable, and > subsequent calls to this method will result in an ISE. > > Paul. > >> On 3 Nov 2015, at 18:11, Steve Drach <steve.dr...@oracle.com> wrote: >> >> Please review the latest webrev that addresses the issue raised in Sherman’s >> comments below, with my comments interspersed in-line. The changes between >> this webrev and the last one are in the definition and use of the >> ismultiRelease() method and the introduction of a configuration lock, the >> boolean configured, that prevents setting the version after an entry has >> been read from the jar file. As a consequence of the configuration lock, I >> had to modify a couple tests and add a new one. >> >> Issue: https://bugs.openjdk.java.net/browse/JDK-8132734 >> JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305 >> Webrev: http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/ >> >
