java.time.ZoneId.systemDefalut() overhead

[Peter Levart]

On 02/23/2015 08:36 PM, Stephen Colebourne wrote:
> Looking at the patch, all that may be needed is to change the type of
> the instance variable used for the cache from ZoneId to Object, and
> cast where necessary. Adds the cost of the cast to the main flow, but
> that is pretty minimal.
>
> Stephen

Having an instance variable in a class does not trigger loading of the 
class for the type of instance variable just yet.  Try the following 
with -verbose:class and you'll see that only Test.A is loaded...

public class Test {

     static class A {
         B b;

         B toB() {
             if (b == null) {
                 b = new B();
             }
             return b;
         }
     }

     static class B {
     }

     public static void main(String[] args) throws Exception {
         A a = new A();
     }
}


If we're talking about using java.time from ZipEntry, then that's 
another story. I belive that VM startup does not need the conversion 
from DOS time to Java time in ZipEntry, but that should be checked to be 
sure...

Regards, Peter

>
>
> On 23 February 2015 at 19:24, Xueming Shen <xueming.shen at oracle.com> wrote:
>> I think I did have a j.time version somewhere when working on JSR310 and
>> measured
>> the difference of the performance . If my memory serves me correctly one of
>> the issues
>> back then is that it has an impact to the startup time, because using the
>> JSR310 LDT
>> and its timezone implementation means to load and initialize LOTS of the
>> java.time
>> classes and initialize the JSR310 tz data. We did lots of rounds of
>> implementation of
>> using the JSR310 tzdata for the java.util.TimeZone implementation in jdk8
>> (so the
>> JSR310 and j.u.TimeZone share the same tz data) and tried the best to use as
>> less
>> JSR310 classes as possible to limit the startup "regression".  It's true
>> that it might not
>> matter if the app is going to use the new JSR310 anyway, but for most of
>> existing
>> applications, you will see a slowdown of the startup without too much
>> benefit. That
>> was the consideration back to jdk8, in which the zip/jar was a must for
>> anything to
>> startup, if it's no longer true for jdk9 module system, then it might no
>> longer be an
>> concern.
>>
>> -Sherman
>>
>>
>> On 02/22/2015 12:21 PM, Peter Levart wrote:
>>> 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
>>>