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.s...@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
