RFR: 8163798: Create a JarFile versionedStream method
steve.drach at oracle.com
Mon Sep 12 19:35:09 UTC 2016
> - 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
> - 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
> - nit: rename *ptr to *Index
> Test and JarFile changes seem fine to me.
> 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:
>>> 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>
More information about the core-libs-dev