Re: jdk9/10 reject zip/jar files where seconds value of timestamp is out of supported range 0 - 59
Hi Matthias,
I have closed your bug JDK-8188901 in favor of the bug that Sherman had opened before: https://bugs.openjdk.java.net/browse/JDK-8188869.
Please post a webrev for the change as suggested by Claes.
Sounds good to me.
In addition to being a cleanup, the move to use java.time did provide a
speedup, however, which
might become significant when loading lots of jar files.
I've not found my notes on how big this speed-up was (I recall ~3x in
micros), but if anyone has time
to fix (and test) the overflows without reverting to java.util.Date then
we might be able to keep most
of that gain intact.? This can of course be considered a follow-up, or
ignored.
Hi Claes, to me it looks like the old jdk8 coding is not slower than the jdk9/10 coding . Best regards, Matthias
Hi Matthias, On 2017-10-10 18:23, Baesken, Matthias wrote:
Hi Claes, to me it looks like the old jdk8 coding is not slower than the jdk9/10 coding .
a simple and somewhat naïve JMH benchmark to assess performance: http://cr.openjdk.java.net/~redestad/scratch/ZipUtils.java I've benchmarked the code I proposed to fall back to, adding a variant to do some quick sanity checking to avoid exceptional control flow on common mal-formed cases (dosToJavaTime9Bench2), and can show it doesn't hurt performance too much: Benchmark (dosTime) Mode Cnt Score Error Units ZipUtils.dosToJavaTime8Bench 677456332112 thrpt 10 3.723 ± 0.103 ops/us ZipUtils.dosToJavaTime9Bench 677456332112 thrpt 10 15.207 ± 0.540 ops/us ZipUtils.dosToJavaTime9Bench2 677456332112 thrpt 10 14.241 ± 1.193 ops/us As I recalled it's about a 4x difference. Running this benchmark with more threads (4) shows that the JDK 8 impl hides a scalability issue (Date() uses a shared calendar object, with synchronization to keep it thread safe): Benchmark (dosTime) Mode Cnt Score Error Units ZipUtils.dosToJavaTime8Bench 677456332112 thrpt 10 1.441 ± 0.112 ops/us ZipUtils.dosToJavaTime9Bench 677456332112 thrpt 10 53.166 ± 0.886 ops/us ZipUtils.dosToJavaTime9Bench2 677456332112 thrpt 10 52.238 ± 0.865 ops/us We can see that we're slightly worse on the odd corner case of dosTime=0: Benchmark (dosTime) Mode Cnt Score Error Units ZipUtils.dosToJavaTime8Bench 0 thrpt 10 40.069 ± 0.719 ops/us ZipUtils.dosToJavaTime9Bench 0 thrpt 10 2.177 ± 0.032 ops/us ZipUtils.dosToJavaTime9Bench2 0 thrpt 10 33.439 ± 0.842 ops/us ... but a naïve variant testing mixed valid/invalid inputs shows we're still much better off with dosToJavaTime9Bench2: Benchmark (dosTime) (dosTime2) Mode Cnt Score Error Units ZipUtilsMixed.dosToJavaTime8Bench 677456332112 0 thrpt 5 1467.281 ± 328.030 ops/ms ZipUtilsMixed.dosToJavaTime9Bench 677456332112 0 thrpt 5 24412.943 ± 1151.896 ops/ms ZipUtilsMixed.dosToJavaTime9Bench2 677456332112 0 thrpt 5 20687.981 ± 1535.571 ops/ms http://cr.openjdk.java.net/~redestad/scratch/ZipUtilsMixed.java Finally we can see that we're generally faster in interpreted mode (this code is of general interest to startup on many internal startup benchmarks): Benchmark (dosTime) Mode Cnt Score Error Units ZipUtils.dosToJavaTime8Bench 677456332112 thrpt 5 152.447 ± 5.899 ops/ms ZipUtils.dosToJavaTime9Bench 677456332112 thrpt 5 263.718 ± 19.450 ops/ms ZipUtils.dosToJavaTime9Bench2 677456332112 thrpt 5 266.059 ± 5.003 ops/ms I'll send out a RFR for an implementation similar to the dosToJavaTime9Bench2 experiment soon, so that we can be both backwards compatible and avoid scalability bottlenecks. Thanks! /Claes
participants (2)
-
Baesken, Matthias
-
Claes Redestad