RFR: 8243469: Lazily encode name in ZipFile.getEntryPos

[Volker Simonis]

On Thu, Apr 23, 2020 at 2:35 PM Claes Redestad
<claes.redestad at oracle.com> wrote:
>
> Hi,
>
> current implementation of ZipFile.getEntryPos takes the encoded byte[]
> of the String we're looking up. Which means when looking up entries
> across multiple jar files, we allocate the byte[] over and over for each
> jar file searched.
>
> If we instead refactor the internal hash table to use a normalized
> String-based hash value, we can almost always avoid both the repeated
> encoding and (most of) the hash calculation when the entry is not found
> in the jar/zip.
>
> This realizes a significant startup improvement on applications with
> several or many jar files. A rather typical case. For example I see a
> 25% speedup of ZipEnty.getEntry calls on a Spring PetClinic, which
> reduces total startup time by ~120ms, or ~2.5% of total, on my setup. At
> the same time remaining neutral on single-jar apps.
>
> Webrev: http://cr.openjdk.java.net/~redestad/8243469/open.00/
> Bug:    https://bugs.openjdk.java.net/browse/JDK-8243469
>
> Testing: tier1-2

Hi Claes,

that's a little puzzling change but also a nice enhancement and cleanup.

I think it looks good. I have only two minor comments:

There's no check for "end > 0" here:
  93         @Override
  94         boolean hasTrailingSlash(byte[] a, int end) {
  95             return a[end - 1] == '/';
  96         }
I think that's currently no real problem, but maybe put in a check for any case?

And while you're on it, I think the following comment should be updated from:

 641     /* Checks ensureOpen() before invoke this method */

to something like:

 641     /* Check ensureOpen() before invoking this method */


I've also had a quick look at the microbenchmark which you've
apparently only added today :)

It seems that 'getEntryHitUncached' is getting slightly slower with
your change while all the other variants get significantly faster. I
don't think that's a problem, but do you have an explanation why
that's the case?

Thanks for this nice improvement,
Volker

>
> Thanks!
>
> /Claes