I noticed that in JarFile.getEntry(name) , the first call to
getEntry0(name) call is thrown away for cases where getVersionedEntry
actually finds a versioned entry.
public ZipEntry getEntry(String name) {
JarFileEntry je = getEntry0(name);
if (isMultiRelease()) {
return getVersionedEntry(name, je);
}
return je;
}
We could avoid this wasted lookup by letting getVersionedEntry perform the
default lookup instead. There are a couple of other callers
of getVersionedEntry with different fallbacks though, so not quite sure how
to deal with that. A Supplier<JarEntry> might work?
Eirik.
On Sun, Apr 12, 2020 at 9:06 PM Eirik Bjørsnøs <eir...@gmail.com> wrote:
>
>
> On Sun, Apr 12, 2020 at 4:18 PM Claes Redestad <claes.redes...@oracle.com>
> wrote:
>
>> I think this is shaping up nicely.
>>
>> If we ensure the array returned from getMetaInfVersions is already
>> sorted (by e.g. using a TreeMap during parse), we could remove the
>> stream expression. And if we check bounds in getVersionedEntry we don't
>> need the field in JarFile at all.
>>
>
> Yeah, that extra field is kind of redundant. I guess my thought was that
> ordering semantics of MRJ belongs in JarFile, but it's also great to reduce
> duplicated state. Perhaps we could update the ZipFile.metadataVersions
> field comment to say "ordered list"?
>
>
>> In initCEN I think you need to subtract 9 from nlen:
>>
>> getMetaVersion(cen, pos + CENHDR + 9, nlen - 9)
>>
>
> Nice catch. The constant 9 feels a bit magic here, so maybe we could add a
> comment saying // 9 is "META-INF/".length. (And / or extract a local
> variable)
>
>
>> And in getMetaVersion I think we can avoid creating Strings and parse
>> the version inline. By using 0 as the "not a version" value we can
>> simplify it further:
>>
>> int version = 0;
>> for (int i = vstart; i < nend; i++) {
>> final byte c = name[i];
>> if (c == '/') {
>> return version;
>> }
>> if (c < '0' || c > '9') {
>> return 0;
>> }
>> version = version * 10 + c - '0';
>> // Check for overflow
>> if (version < 0) {
>> return 0;
>> }
>> }
>>
>
> This is brilliantly clever. I stopped trying to be clever at some point in
> order to maintain readability, but I think this code is both readable _and_
> clever.
>
> The javadocs for getMetaVersion should be updated to specify "Otherwise,
> return 0"
>
> Eirik.
>
