Re: RFR 8066291: ZoneIdPrinterParser can be optimized

bourne [mailto:scolebou...@joda.org] Sent: Monday, May 09, 2016 4:43 PM To: core-libs-dev Subject: Re: RFR 8066291: ZoneIdPrinterParser can be optimized One point - the Javadoc for the method on ZoneRulesProvider @return a modifiable copy of the set of zone IDs, not null needs to change to @

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

(). Thanks, Bhanu -Original Message- From: Stephen Colebourne [mailto:scolebou...@joda.org] Sent: Friday, May 06, 2016 3:54 PM To: core-libs-dev Subject: Re: RFR 8066291: ZoneIdPrinterParser can be optimized The set of zones can only increase, it cannot decrease as there is no removal mechanism

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

. > > Thanks, > Bhanu > > -Original Message- > From: Stephen Colebourne [mailto:scolebou...@joda.org] > Sent: Monday, May 09, 2016 4:43 PM > To: core-libs-dev > Subject: Re: RFR 8066291: ZoneIdPrinterParser can be optimized > > One point - the Javadoc for th

RE: RFR 8066291: ZoneIdPrinterParser can be optimized

java/time/tck/java/time/TCKZoneId.java (). > > Thanks, > Bhanu > > -Original Message- > From: Stephen Colebourne [mailto:scolebou...@joda.org] > Sent: Friday, May 06, 2016 3:54 PM > To: core-libs-dev > Subject: Re: RFR 8066291: ZoneIdPrinterParser can be op

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

. > > Thanks, > Bhanu > > -Original Message- > From: Stephen Colebourne [mailto:scolebou...@joda.org] > Sent: Friday, May 06, 2016 3:54 PM > To: core-libs-dev > Subject: Re: RFR 8066291: ZoneIdPrinterParser can be optimized > > The set of zones can only increase, i

RE: RFR 8066291: ZoneIdPrinterParser can be optimized

: RFR 8066291: ZoneIdPrinterParser can be optimized The set of zones can only increase, it cannot decrease as there is no removal mechanism. As such, the size of the set is a proxy for the number you describe. One other point. The method that most users will call to get the set of ZoneIds

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

ConcurrentHashMap is (probably!) cheaper than it used to be. For read-mostly operations there won't be a lock. But yes, even better for read-mostly use is to keep an immutable map and CAS-loop whenever you need to update. The VarHandle work and the immutable collections work should make that

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

Hi Stephen, It still seems pretty elaborate and duplicates the set; but it would be a bigger change to restructure the internal ZONES map to have an immutable map in the implementation and synchronize its update when a new Provider is added (very infrequently/never). ConcurrentHashMap is a

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

The set of zones can only increase, it cannot decrease as there is no removal mechanism. As such, the size of the set is a proxy for the number you describe. One other point. The method that most users will call to get the set of ZoneIds is ZoneId.getAvailableZoneIds(). That method delegates to

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

Hi, Using the current number of ZoneIDs to avoid the recompilation of the cache is a bit weak anyway, though it seems unlikely that a ZoneID would be added and one deleted without being noticed. An alternative would be a API that returned a number that would change every time the set of

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

On reflection, both your and my solution have a race. the size method, is a clear check-then-act the read-only method uses Collections.unmodifiableSet() which only decorates the underlying set, thus is still check-thern-act (the current implementation does not have a race condition, as the data

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

Hi Stephen, The aspect of the current implementation that is problematic is the copying of the set, its not just single object creation but an entry for every ZoneID. Adding a size method exposing some internal state increases the possibility that when it is used independently it will be out

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

I fail to see why adding a new read-only method alongside the existing method adds any more value to the API than adding a new size method. At least with the size method the API is still sensible - a mutable and immutable method alongside each other shouts out that a mistake was made. A size

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

Hi Bhanu, Adding a trivial method to the public API used only for an optimization is not a good fix for this issue. A better fix was suggested to add a non-copying read-only version of ZoneRulesProvider.getAvailableZoneIds() Please revise the fix to instead implement and use: /** *

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

> From: Stephen Colebourne [mailto:scolebou...@joda.org] > Sent: Thursday, May 05, 2016 3:18 PM > To: core-libs-dev > Subject: Re: RFR 8066291: ZoneIdPrinterParser can be optimized > > In ZoneRulesProvider.getAvailableZoneIdsSize() there is no need for the > trailing paragr

RE: RFR 8066291: ZoneIdPrinterParser can be optimized

Please see the updated webrev: http://cr.openjdk.java.net/~bgopularam/bhanu/JDK-8066291/webrev.01/ Thanks, Bhanu -Original Message- From: Stephen Colebourne [mailto:scolebou...@joda.org] Sent: Thursday, May 05, 2016 3:18 PM To: core-libs-dev Subject: Re: RFR 8066291

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

In ZoneRulesProvider.getAvailableZoneIdsSize() there is no need for the trailing paragraph tag in the Javadoc. Otherwise, fine by me. thanks Stephen On 5 May 2016 at 10:10, Bhanu Gopularam wrote: > Hi all, > > > > Please review fix following issue > > > >