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

[Claes Redestad]

On Wed, 11 Sep 2024 08:34:09 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> 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 investigation, 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

Just a guess but perhaps this is down to a cache effect where the `ZipEntry` allocation has a chance to evict the cen data array from some cache. Batching all the reads from CEN together could counter-act some such effects and better streamline memory accesses.

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

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