RFR: 8339874: Avoid duplicate checking of trailing slash in ZipFile.getZipEntry [v4]

[Eirik Bjørsnøs]

On Tue, 10 Sep 2024 19:39:34 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Eirik Bjørsnøs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add whitespace per review feedback
>
> src/java.base/share/classes/java/util/zip/ZipFile.java line 681:
> 
>> 679: 
>> 680:         e.flag = CENFLG(cen, pos);
>> 681:         e.method = CENHOW(cen, pos);
> 
> Not reading `nlen` when it's not needed is a good change, and moving `clen` and `elen` down to be grouped with the others is fine, but some of the other shuffling around here doesn't seem motivated?

The reordering of field reads was motivated by some previous experiences where ordering the reads sequentially with their appearance in the CEN a had small, but positive performance gain. 

After closer invetigation, it turns out that the internal ordering of field reads only has small benefits, maybe in the noise. However, reading the length fields _before the allocation of the `ZipEntry`_ has a significant negative impact. In fact, it seems to explain most of the performance gains in this PR.

It seems that having `ZipEntry` allocation interspersed within the CEN field reads incurs a significant cost. I can't explain why, but perhaps @cl4es can?  (To reproduce, simply move the length reads to the beginning of the method)

I have kept the reordering of `nlen`, `elen`, `clen` reads, but reverted some other reorderings to make the PR cleaner. 

This PR:


Benchmark                    (size)  Mode  Cnt   Score   Error  Units
ZipFileGetEntry.getEntryHit     512  avgt   15  52.057 ? 2.021  ns/op
ZipFileGetEntry.getEntryHit    1024  avgt   15  54.753 ? 1.093  ns/op


Reads length fields before `ZipEntry` allocation:


Benchmark                    (size)  Mode  Cnt   Score   Error  Units
ZipFileGetEntry.getEntryHit     512  avgt   15  65.199 ? 0.823  ns/op
ZipFileGetEntry.getEntryHit    1024  avgt   15  69.407 ? 0.807  ns/op

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20939#discussion_r1753604647