> jdk/internal/util/jar/VersionedStream.java: > > - use Integer.parseInt(name, vptr, ptr, 10) to avoid creating vstr on > every entry
done > > - folding allowedVersions into getBaseSuffix seems easier than to keep > global state (sptr) and splitting the logic over a filter and a map > operation done > > - distinct() could be used instead of explicitly collecting to a I did use distinct in webrev.02. I left it in webrev.03 (coming soon). > LinkedHashSet, but this does make me wonder if we could do even > better by creating a map from base name to the appropriate versioned > JarEntry and return a stream over that map to avoid looking things up > via jf::getJarEntry. I look things up with getJarEntry to get the aliased entry. Otherwise we get the real name and we want to refer to entries by the base name. > Could be quite a bit more efficient, and as we > have to create a set or do distinct the overheads should be roughly > the same either way > > - declaring META_INF_VERSIONS_LEN seems like a premature optimization Left it in, it’s harmless and doesn’t need to be computed every time > > - nit: static final is preferred over final static done > > - nit: rename *ptr to *Index done > > Test and JarFile changes seem fine to me. > > Thanks! > > /Claes > > On 2016-09-11 22:12, Steve Drach wrote: >> I made a simple change, the new webrev is >> http://cr.openjdk.java.net/~sdrach/8163798/webrev.02/ >> <http://cr.openjdk.java.net/~sdrach/8163798/webrev.02/> >> >>> On Sep 9, 2016, at 4:02 PM, Steve Drach <steve.dr...@oracle.com> wrote: >>> >>> Hi, >>> >>> Please review this changeset that adds a VersionedStream class to the >>> jdk.internal.util.jar package. Some may recall that I submitted a similar >>> RFR a few weeks ago; this is a redesign from that one. We decided not to >>> make a public JarFile::versionedStream method at this time. Once we get >>> sufficient experience with this and find a few more use cases, we will >>> revisit the idea of making this a public method in JarFile. >>> >>> issue: https://bugs.openjdk.java.net/browse/JDK-8163798 >>> <https://bugs.openjdk.java.net/browse/JDK-8163798> >>> webrev: http://cr.openjdk.java.net/~sdrach/8163798/webrev.01/index.html >>> <http://cr.openjdk.java.net/~sdrach/8163798/webrev.01/index.html> >>> >>> Thanks, >>> Steve >>
