RFR: 8260617: Merge ZipFile encoding check with the initial hash calculation

[Lance Andersen]

On Fri, 29 Jan 2021 00:54:46 GMT, Claes Redestad <redestad at openjdk.org> 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