On Sun, 6 Oct 2024 14:16:44 GMT, Claes Redestad <[email protected]> wrote:
> #14632 showed that coalescing loads in the `ZipUtils` utility methods could
> improve performance in zip-related microbenchmarks, but doing so would
> increase startup overheads. Progress was stalled as we backed out some
> related early use of `ByteArray(LittleEndian)` and started exploring merge
> store optimizations in C2.
>
> In this PR I instead suggest using `Unsafe` directly to coalesce `short`,
> `int`, and `long` reads from zip data. Even with explicit bounds checking to
> ensure these utilities are _always_ safe there are significant improvements
> both to lookup speed and speed of opening zip files (most if not all bounds
> checks are optimized away):
>
>
> make test TEST=micro:java.util.zip.ZipFile
>
> Name (size) Cnt Base Error Test
> Error Unit Change
> GetEntry.getEntryHit 512 15 37.999 ± 0.841 34.641 ±
> 0.389 ns/op 1.10x (p = 0.000*)
> GetEntry.getEntryHit 1024 15 39.557 ± 0.523 36.959 ±
> 1.488 ns/op 1.07x (p = 0.000*)
> GetEntry.getEntryHitUncached 512 15 69.250 ± 0.931 64.851 ±
> 0.987 ns/op 1.07x (p = 0.000*)
> GetEntry.getEntryHitUncached 1024 15 71.628 ± 0.307 67.927 ±
> 0.714 ns/op 1.05x (p = 0.000*)
> GetEntry.getEntryMiss 512 15 22.961 ± 0.336 22.825 ±
> 0.188 ns/op 1.01x (p = 0.158 )
> GetEntry.getEntryMiss 1024 15 22.940 ± 0.115 23.502 ±
> 0.273 ns/op 0.98x (p = 0.000*)
> GetEntry.getEntryMissUncached 512 15 35.886 ± 0.429 35.598 ±
> 1.296 ns/op 1.01x (p = 0.395 )
> GetEntry.getEntryMissUncached 1024 15 38.168 ± 0.911 36.141 ±
> 0.356 ns/op 1.06x (p = 0.000*)
> Open.openCloseZipFile 512 15 62425.563 ± 997.455 56263.401 ±
> 896.892 ns/op 1.11x (p = 0.000*)
> Open.openCloseZipFile 1024 15 117491.250 ± 962.928 108055.491 ±
> 1595.577 ns/op 1.09x (p = 0.000*)
> Open.openCloseZipFilex2 512 15 62974.575 ± 911.095 57996.388 ±
> 910.929 ns/op 1.09x (p = 0.000*)
> Open.openCloseZipFilex2 1024 15 119164.769 ± 1756.065 108803.468 ±
> 929.483 ns/op 1.10x (p = 0.000*)
> * = significant
>
>
> This PR also address some code duplication in `ZipUtils`.
>
> An appealing alternative would be to implement a merge load analogue to the
> merge store optimizations in C2. Such optimizations would be very welcome
> since it would improve similar code outside of java.base (jdk.zipfs has some
> duplicate code that is left untouched) ...
Changes requested by liach (Reviewer).
src/java.base/share/classes/java/util/zip/ZipUtils.java line 175:
> 173: public static final int get16(byte[] b, int off) {
> 174: Preconditions.checkIndex(off, b.length,
> Preconditions.AIOOBE_FORMATTER);
> 175: Preconditions.checkIndex(off + 1, b.length,
> Preconditions.AIOOBE_FORMATTER);
Please use `Preconditions.checkFromIndexSize`, which should be less overhead:
Suggestion:
Preconditions.checkFromIndexSize(off, 2, b.length,
Preconditions.AIOOBE_FORMATTER);
Similarly for other methods.
-------------
PR Review: https://git.openjdk.org/jdk/pull/21377#pullrequestreview-2350546885
PR Review Comment: https://git.openjdk.org/jdk/pull/21377#discussion_r1789116076
