RFR: 8163798: Create a JarFile versionedStream method

Claes Redestad claes.redestad at oracle.com
Sun Sep 11 20:56:27 UTC 2016


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.drach at 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
>


More information about the core-libs-dev mailing list