Re: PING! Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-04-15 Thread Paul Sandoz
Hi Brian, My inclination is if it ain't broke... and AFAICT nothing indicates toString it is particular broken [*], so perhaps just focus on the cleanup aspects and revisit the other when the JMM is updated (and maybe enhanced volatiles is ready)? Paul. [*] although i still question the use

Re: PING! Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-04-15 Thread Brian Burkhalter
Hi Paul, On Apr 15, 2014, at 12:23 AM, Paul Sandoz paul.san...@oracle.com wrote: Hi Brian, My inclination is if it ain't broke... and AFAICT nothing indicates toString it is particular broken [*], so perhaps just focus on the cleanup aspects and revisit the other when the JMM is updated

Re: PING! Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-04-15 Thread Brian Burkhalter
On Apr 15, 2014, at 9:24 AM, Brian Burkhalter brian.burkhal...@oracle.com wrote: On Apr 15, 2014, at 12:23 AM, Paul Sandoz paul.san...@oracle.com wrote: My inclination is if it ain't broke... and AFAICT nothing indicates toString it is particular broken [*], so perhaps just focus on the

PING! Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-04-10 Thread Brian Burkhalter
Second day back from vacation so I guess it’s time to beat this horse again … As there was no response to the message included below I am simply re-posting it. Thanks, Brian On Mar 25, 2014, at 7:19 AM, Brian Burkhalter brian.burkhal...@oracle.com wrote: On Mar 25, 2014, at 1:58 AM, Paul

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-25 Thread Peter Levart
On 03/24/2014 06:48 PM, Brian Burkhalter wrote: Hi Peter, On Mar 24, 2014, at 2:21 AM, Peter Levart peter.lev...@gmail.com mailto:peter.lev...@gmail.com wrote: Thanks to Aleksey for re-establishing the access, I bring you results of the microbenchmark from his quad-core Cortex-A9: Thanks

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-25 Thread Paul Sandoz
On Mar 24, 2014, at 8:30 PM, Mike Duigou mike.dui...@oracle.com wrote: The other issue here, and why this thread has gone on so long, is that we've been trying to establish what the best idiom is (for 2014-) for this lazy initialization situation so that we can reuse it with less thought

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-25 Thread Brian Burkhalter
On Mar 25, 2014, at 1:58 AM, Paul Sandoz paul.san...@oracle.com wrote: This is another example of a stable variable. I would like to re-iterate my scepticism that such changes are necessary in this case (i am not sure if it is possible to create a benchmark that could better exacerbate

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-24 Thread Peter Levart
On 03/20/2014 08:49 AM, Aleksey Shipilev wrote: On 03/20/2014 11:06 AM, Peter Levart wrote: I was thinking about last night, for question: Why is this double-checked non-volatile-then-volatile trick not any faster than pure volatile variant even on ARM platform where volatile read should have

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-24 Thread Brian Burkhalter
Hi Peter, On Mar 22, 2014, at 2:01 AM, Peter Levart peter.lev...@gmail.com wrote: Looks good. Just a nit. In the following method: 3726 private static void matchScale(BigDecimal[] val) { I concur. One of 3 ifs is superfluous. Either the 1st one: private static void

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-24 Thread Brian Burkhalter
Hi Peter, On Mar 24, 2014, at 2:21 AM, Peter Levart peter.lev...@gmail.com wrote: Thanks to Aleksey for re-establishing the access, I bring you results of the microbenchmark from his quad-core Cortex-A9: Thanks to you and Aleksey for taking the initiative to test on that platform. ...as

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-24 Thread Peter Levart
On 03/24/2014 06:52 PM, Martin Buchholz wrote: On Wed, Mar 12, 2014 at 1:45 AM, Peter Levart peter.lev...@gmail.com mailto:peter.lev...@gmail.com wrote: What would the following cost? private transient String stringCache; public String toString() {

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-24 Thread Mike Duigou
On Mar 24 2014, at 12:25 , Martin Buchholz marti...@google.com wrote: On Mon, Mar 24, 2014 at 11:25 AM, Peter Levart peter.lev...@gmail.com wrote: On 03/24/2014 06:52 PM, Martin Buchholz wrote: On Wed, Mar 12, 2014 at 1:45 AM, Peter Levart peter.lev...@gmail.com wrote: What

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-22 Thread Peter Levart
On 03/22/2014 02:01 AM, Brian Burkhalter wrote: On Mar 20, 2014, at 12:49 AM, Aleksey Shipilev aleksey.shipi...@oracle.com mailto:aleksey.shipi...@oracle.com wrote: On 03/20/2014 11:06 AM, Peter Levart wrote: I was thinking about last night, for question: Why is this double-checked

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-21 Thread Brian Burkhalter
On Mar 20, 2014, at 12:49 AM, Aleksey Shipilev aleksey.shipi...@oracle.com wrote: On 03/20/2014 11:06 AM, Peter Levart wrote: I was thinking about last night, for question: Why is this double-checked non-volatile-then-volatile trick not any faster than pure volatile variant even on ARM

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-20 Thread Aleksey Shipilev
On 03/20/2014 11:06 AM, Peter Levart wrote: I was thinking about last night, for question: Why is this double-checked non-volatile-then-volatile trick not any faster than pure volatile variant even on ARM platform where volatile read should have some penalty compared to normal read?, might be

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-19 Thread Brian Burkhalter
On Mar 14, 2014, at 7:17 AM, Brian Burkhalter brian.burkhal...@oracle.com wrote: On Mar 14, 2014, at 3:39 AM, Peter Levart wrote: But in general it would be better to just use ThreadLocalRandom.current() everywhere you use rnd variable. This is precisely it's purpose - a random number

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-19 Thread Peter Levart
On 03/19/2014 11:01 PM, Brian Burkhalter wrote: On Mar 14, 2014, at 7:17 AM, Brian Burkhalter brian.burkhal...@oracle.com mailto:brian.burkhal...@oracle.com wrote: On Mar 14, 2014, at 3:39 AM, Peter Levart wrote: But in general it would be better to just use ThreadLocalRandom.current()

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-19 Thread Mike Duigou
I On Mar 19 2014, at 15:01 , Brian Burkhalter brian.burkhal...@oracle.com wrote: On Mar 14, 2014, at 7:17 AM, Brian Burkhalter brian.burkhal...@oracle.com wrote: On Mar 14, 2014, at 3:39 AM, Peter Levart wrote: But in general it would be better to just use ThreadLocalRandom.current()

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-19 Thread Brian Burkhalter
On Mar 19, 2014, at 4:32 PM, Mike Duigou mike.dui...@oracle.com wrote: Since the Unsafe.getObjectVolatile() allows use of volatile semantics without having to declare the field volatile I think this is a better idiom than what I had previously suggested. Hopefully this is a pattern we can

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-19 Thread Brian Burkhalter
Hi Peter, On Mar 19, 2014, at 4:32 PM, Peter Levart peter.lev...@gmail.com wrote: So the solution is to reduce number of bytecodes in toString(). For example, the following: public String toString() { String sc = stringCache; if (sc == null) { sc =

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-19 Thread Brian Burkhalter
On Mar 19, 2014, at 4:41 PM, Brian Burkhalter brian.burkhal...@oracle.com wrote: Benchmark Mode Samples Mean Mean error Units o.s.Bench6375303.testToString avgt10 80.9250.313 ns/op That’s great! I can re-try that

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-14 Thread Peter Levart
On 03/14/2014 01:29 AM, Brian Burkhalter wrote: On Mar 12, 2014, at 2:08 AM, Peter Levart wrote: Huh! This is a ThreadLocalRandom anti-pattern. Thou should not use a ThreadLocalRandom instance in a thread that did not obtain it via a call to ThreadLocalRandom.current()… Good point. You

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-14 Thread Peter Levart
On 03/14/2014 01:32 AM, Brian Burkhalter wrote: On Mar 12, 2014, at 1:45 AM, Peter Levart wrote: What would the following cost? private transient String stringCache; public String toString() { […] This version did not show any significant difference in the benchmark results

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-14 Thread Brian Burkhalter
On Mar 14, 2014, at 3:39 AM, Peter Levart wrote: But in general it would be better to just use ThreadLocalRandom.current() everywhere you use rnd variable. This is precisely it's purpose - a random number generator that is never contended. The overhead of ThreadLocalRandom.current() call

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-14 Thread Brian Burkhalter
On Mar 14, 2014, at 3:42 AM, Peter Levart wrote: This approach does have the advantage or not using volatile. I've forgot to run this on my Raspberry Pi. Stay tuned... I used to have a BeagleBoard on a previous project but do not have any of these sorts of devices at my disposal at this

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-13 Thread Brian Burkhalter
On Mar 12, 2014, at 2:08 AM, Peter Levart wrote: Huh! This is a ThreadLocalRandom anti-pattern. Thou should not use a ThreadLocalRandom instance in a thread that did not obtain it via a call to ThreadLocalRandom.current()… Good point. You could create a new BigInteger(512, rnd) in the

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-13 Thread Brian Burkhalter
On Mar 12, 2014, at 1:45 AM, Peter Levart wrote: What would the following cost? private transient String stringCache; public String toString() { […] This version did not show any significant difference in the benchmark results (updated)

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-12 Thread Peter Levart
On 03/11/2014 06:07 PM, Mike Duigou wrote: You are making stringCache volatile - performance penalty on ARM PPC. It is understood that the volatile is not free. For this purpose it was believed that the performance cost was a fair trade off to avoid the heap pollution. Regardless of the

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-12 Thread Peter Levart
On 03/11/2014 06:10 PM, Brian Burkhalter wrote: Sergey, On Mar 11, 2014, at 1:18 AM, Sergey Kuksenko wrote: Could you share your benchmarks? Of course. Please see: benchmark source: http://cr.openjdk.java.net/~bpb/6375303/Bench6375303.java benchmark results:

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-12 Thread Peter Levart
On 03/11/2014 06:10 PM, Brian Burkhalter wrote: Sergey, On Mar 11, 2014, at 1:18 AM, Sergey Kuksenko wrote: Could you share your benchmarks? Of course. Please see: benchmark source: http://cr.openjdk.java.net/~bpb/6375303/Bench6375303.java public class Bench6375303 { * static

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-12 Thread Paul Sandoz
On Mar 11, 2014, at 9:05 AM, Sergey Kuksenko sergey.kukse...@oracle.com wrote: Brian, Mike. Could you explain what is the problem with the old caching? String is immutable, BigDecimal is immutable. So we have data race at that place, but it is safe data race. What is the problem if we

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-11 Thread Sergey Kuksenko
Brian, Mike. Could you explain what is the problem with the old caching? String is immutable, BigDecimal is immutable. So we have data race at that place, but it is safe data race. What is the problem if we create several identical strings? You are making stringCache volatile - performance

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-11 Thread Sergey Kuksenko
Brian, Could you share your benchmarks? On 03/04/2014 04:14 AM, Brian Burkhalter wrote: Hello all, This review request concerns this issue and proposed patch: issue https://bugs.openjdk.java.net/browse/JDK-6375303 patch http://cr.openjdk.java.net/~bpb/6375303/webrev.00/ A review of the

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-11 Thread Mike Duigou
On Mar 11 2014, at 01:05 , Sergey Kuksenko sergey.kukse...@oracle.com wrote: Brian, Mike. Could you explain what is the problem with the old caching? Concern over heap pollution with extra string copies was the motivation to ensure that only a single cached instance was ever returned.

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-11 Thread Brian Burkhalter
Sergey, On Mar 11, 2014, at 1:18 AM, Sergey Kuksenko wrote: Could you share your benchmarks? Of course. Please see: benchmark source: http://cr.openjdk.java.net/~bpb/6375303/Bench6375303.java benchmark results: http://cr.openjdk.java.net/~bpb/6375303/6375303-bench.html Let us

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-07 Thread Brian Burkhalter
On Mar 3, 2014, at 4:14 PM, Brian Burkhalter wrote: PING-INg-Ing-ing! This review request concerns this issue and proposed patch: issue https://bugs.openjdk.java.net/browse/JDK-6375303 patch http://cr.openjdk.java.net/~bpb/6375303/webrev.00/ On Mar 4, 2014, at 9:21 AM, Mike Duigou wrote:

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-04 Thread Peter Levart
On 03/04/2014 01:14 AM, Brian Burkhalter wrote: - add AtomicReferenceFieldUpdater-type constant for stringCache initialization Hi Brian, By using volatile read and CAS, there's still a chance that multiple concurrent threads will be invoking the layoutChars(true) concurrently, but you

Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal

2014-03-04 Thread Mike Duigou
On Mar 4 2014, at 07:13 , Peter Levart peter.lev...@gmail.com wrote: On 03/04/2014 01:14 AM, Brian Burkhalter wrote: - add AtomicReferenceFieldUpdater-type constant for stringCache initialization Hi Brian, By using volatile read and CAS, there's still a chance that multiple