On Fri, 29 Jan 2021 00:54:46 GMT, Claes Redestad <[email protected]> wrote:
> - Merge checkEncoding into the byte[]-based normalizedHash. The latter is
> only used from ZipFile.initCEN right after the checkEncoding today, so
> verifying this is equivalent is straightforward.
> - Factor out the logic to calculate hash, check encoding etc into the
> addEntry method to allow JITs to chew off larger chunks of the logic early on
>
> Roughly 1.2x speedup on the JarFileMeta microbenchmarks, which include the
> time required to open the jar file and thus exercising the optimized code.
>
> Testing: tier1-4
Hi Claes,
Overall, the changes seem good. A few comments below.
src/java.base/share/classes/java/util/zip/ZipCoder.java line 140:
> 138: // aborting the ASCII fast-path in the UTF8 implementation, so
> {@code h}
> 139: // might be a partially calculated hash code
> 140: int normalizedHashDecode(int h, byte[] a, int off, int end) {
Would it make sense to keep some of this comment for clarity?
src/java.base/share/classes/java/util/zip/ZipFile.java line 1531:
> 1529: entryPos = pos + CENHDR;
> 1530: }
> 1531: this.total = idx / 3;
It took me a moment to figure out why you made the change of total = idx/3 but
I understand now.
I guess my question is this part of your change more performant as I think it
was easier to read /understand when total was used?
If you believe this is the better way to go, I think it would be helpful to
add a comment as the change is not obvious on a first pass to the reader or at
least me ;-)
src/java.base/share/classes/java/util/zip/ZipFile.java line 1501:
> 1499: if (entryPos + nlen > limit)
> 1500: zerror("invalid CEN header (bad header size)");
> 1501: idx = addEntry(idx, table, nlen, pos, entryPos);
Perhaps consider adding a comment describing addEntry. Probably similar to
the line 1500 comment (or similar) would be beneficial
-------------
PR: https://git.openjdk.java.net/jdk/pull/2306
