Re: java.time.ZoneId.systemDefalut() overhead

2015-02-27 Thread Stephen Colebourne
On 27 February 2015 at 10:55, Daniel Fuchs daniel.fu...@oracle.com wrote: I don't think I saw an issue raised for caching the clock in ZoneId. That seems like a good plan to me (ZoneId instances are controlled singletons, so there is no big memory issue there) I'd been intending to log that.

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-27 Thread Stephen Colebourne
On 26 February 2015 at 23:29, Peter Levart peter.lev...@gmail.com wrote: Here's another variant that doesn't use a back link to base TimeZone: http://cr.openjdk.java.net/~plevart/jdk9-dev/ZoneId.systemDefault/webrev.03/ Yes, thats easier to follow. I think I'd be happy with that approach. I

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-27 Thread Daniel Fuchs
On 27/02/15 09:36, Stephen Colebourne wrote: On 26 February 2015 at 23:29, Peter Levart peter.lev...@gmail.com wrote: Here's another variant that doesn't use a back link to base TimeZone: http://cr.openjdk.java.net/~plevart/jdk9-dev/ZoneId.systemDefault/webrev.03/ Yes, thats easier to follow.

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-26 Thread Stephen Colebourne
Personally, I found the SharedSecrets version easier to follow. I also suspect that the toZoneId0() method would be invoked most of the time with this variant, resulting in no beneficial inlining effect. The ZoneOffsetTransition patch looks like a no-brainer to me. Stephen On 26 February

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-26 Thread Peter Levart
On 02/26/2015 11:47 PM, Stephen Colebourne wrote: Personally, I found the SharedSecrets version easier to follow. I also suspect that the toZoneId0() method would be invoked most of the time with this variant, resulting in no beneficial inlining effect. Not true. Once the toZoneId0() invokes

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-26 Thread Peter Levart
Hi, Using routines for converting DOS time to Java time and back (java.util.zip.ZipUtils.[dosToJavaTime,javaToDosTime]) as a base for comparing deprecated java.util.Date API with new java.time API, here's a JMH test with 3 distinct implementations of those routines using java.util.Date,

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-26 Thread Peter Levart
On 02/27/2015 12:10 AM, Peter Levart wrote: On 02/26/2015 11:47 PM, Stephen Colebourne wrote: Personally, I found the SharedSecrets version easier to follow. I also suspect that the toZoneId0() method would be invoked most of the time with this variant, resulting in no beneficial inlining

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread Daniel Fuchs
Hi Peter, On 23/02/15 11:29, Peter Levart wrote: Caching of ZoneId in the defensive clone of defualt TimeZone object would not have a desired effect as the clone is thrown away in each call to ZoneId.systemDefault(). We must get hold of the TimeZone instance that is cached. Another way to do

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread Peter Levart
On 02/23/2015 12:24 PM, Daniel Fuchs wrote: Hi Peter, On 23/02/15 11:29, Peter Levart wrote: Caching of ZoneId in the defensive clone of defualt TimeZone object would not have a desired effect as the clone is thrown away in each call to ZoneId.systemDefault(). We must get hold of the TimeZone

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread Stephen Colebourne
Having recently spent time on performance myself, I know that there are a number of things in the java.time package that need some work. Improving ZoneId.systemDefault() is definitely an improvement I welcome. The change is along the lines of that I would have made, involving a secret location to

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread Roger Riggs
Hi, Does it make sense that the shared secret mechanism may no longer be needed in JDK 9, at least within modules, since the module access controls should provide the desired scoping. Roger On 2/23/2015 6:24 AM, Daniel Fuchs wrote: Agreed. I'm just not a big fan of using SharedSecrets if

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread Roger Riggs
Hi, My understanding is that only whole classes can be explicitly exported from a module. If not exported, they are module private. Roger On 2/23/2015 10:54 AM, Peter Levart wrote: On 02/23/2015 03:32 PM, Roger Riggs wrote: Hi, Does it make sense that the shared secret mechanism may no

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread Peter Levart
On 02/23/2015 03:32 PM, Roger Riggs wrote: Hi, Does it make sense that the shared secret mechanism may no longer be needed in JDK 9, at least within modules, since the module access controls should provide the desired scoping. Roger Will there be a language-level module access modifier

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread Peter Levart
Hi Daniel, On 02/23/2015 10:50 AM, Daniel Fuchs wrote: Hi Peter, On 22/02/15 21:21, 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

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread Peter Levart
On 02/23/2015 10:55 PM, Xueming Shen wrote: 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... I was talking about to use

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread 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

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread Daniel Fuchs
Hi Peter, On 22/02/15 21:21, 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

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-23 Thread Claes Redestad
On 2015-02-23 22:58, Peter Levart wrote: On 02/23/2015 10:55 PM, Xueming Shen wrote: 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

java.time.ZoneId.systemDefalut() overhead

2015-02-22 Thread 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

Re: java.time.ZoneId.systemDefalut() overhead

2015-02-22 Thread Claes Redestad
Hi Peter, On 2015-02-22 21:21, 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