java.time.ZoneId.systemDefalut() overhead

[Peter Levart]

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