Hi,
jdk/internal/util/jar/VersionedStream.java:
- use Integer.parseInt(name, vptr, ptr, 10) to avoid creating vstr on
every entry
- folding allowedVersions into getBaseSuffix seems easier than to keep
global state (sptr) and splitting the logic over a filter and a map
operation
- distinct() could be used instead of explicitly collecting to a
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. 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
- nit: static final is preferred over final static
- nit: rename *ptr to *Index
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
