Hi,
I noticed internal JDK class java.util.zip.ZipUtils uses deprecated
java.util.Date API for implementing two methods for converting DOS to
Java time and back. I thought I'd try translating them to use new
java.time API. Translation was straightforward, but I noticed that new
implementations are not on par with speed to old java.util.Date
versions. Here's a JMH benchmark implementing those two conversion
methods with java.util.Date and java.time.ZonedDateTime APIs:
http://cr.openjdk.java.net/~plevart/jdk9-dev/ZoneId.systemDefault/ZipUtilsTest.java
The results show the following:
Benchmark Mode Samples Score Score
error Units
j.t.ZipUtilsTest.dosToJavaTime_Date avgt 5 101.890
17.570 ns/op
j.t.ZipUtilsTest.dosToJavaTime_ZDT avgt 5 137.587
13.533 ns/op
j.t.ZipUtilsTest.javaToDosTime_Date avgt 5 67.114
10.382 ns/op
j.t.ZipUtilsTest.javaToDosTime_ZDT avgt 5 143.739
15.243 ns/op
Quick sampling with jvisualvm shows that a substantial time is spent
repeatedly obtaining ZoneId.systemDefault() instance. I checked the
implementation and came up with the following optimization:
http://cr.openjdk.java.net/~plevart/jdk9-dev/ZoneId.systemDefault/webrev.01/
TimeZone is a mutable object and has to be defensively cloned when
TimeZone.getDefault() is invoked. So there's no point in caching a
ZoneId instance inside TimeZone instance if we cache it on a clone that
is thrown away each time ZoneId.systemDefault() is invoked. I use
SharedSecrets to access the uncloned TimeZone.defaultTimeZone instance
where caching of ZoneId pays of.
I think that it was never meant to change TimeZone's ID
(TimeZone.setID()) after such instance was put into operation (for
example installed as default time zone with TimeZone.setDefault()). Such
use is unintended and buggy. So I also changed the implementation of
TimeZone.setDefault() to save a defensive copy of TimeZone object as
default time zone instead of a reference to it.
With this patch, the DOS/Java time conversion benchmark shows an
improvement:
Benchmark Mode Samples Score Score
error Units
j.t.ZipUtilsTest.dosToJavaTime_Date avgt 5 97.986
18.379 ns/op
j.t.ZipUtilsTest.dosToJavaTime_ZDT avgt 5 85.010
10.863 ns/op
j.t.ZipUtilsTest.javaToDosTime_Date avgt 5 71.073
25.408 ns/op
j.t.ZipUtilsTest.javaToDosTime_ZDT avgt 5 95.298
17.620 ns/op
Is this patch correct or did I miss something from the internals of
java.time API that does not permit such caching?
Regards, Peter
