On 24/03/2012 2:47 AM, Naoto Sato wrote:
OK, I created a bug 7156459 for this and created a patch. Can you please
review this? It is exactly as you suggested.
http://cr.openjdk.java.net/~naoto/7156459/
Looks good to me. But probably better to start a new email thread.
Thanks,
David
Naoto
On 3/22/12 6:09 P, David Holmes wrote:
On 23/03/2012 1:33 AM, Naoto Sato wrote:
Hi David,
Sorry, the review was done in the i18n team and did not go to core libs
alias.
In your suggested fix, I think there is a very slight chance that two
threads could get different Currency instances if the first thread was
interrupted just after instantiating "currencyVal" instance, which has
not been the case in the prior implementation.
What do you mean by "interrupted" here? If two threads are racing to
install their own Currency instance one will win and putIfAbsent will
return null; and the other will lose and putIfAbsent returns the
instance installed by the winner. There is no difference between the old
code and new code in this regard.
Although returning the
singleton instance is not mandated by the spec, I though it is safer to
keep the same behavior and that get() cost is almost negligible.
Cost depends on how large and well-balanced the hashmap is.
Cheers,
David
Naoto
On 3/21/12 7:27 P, David Holmes wrote:
Hi,
I'm sorry I missed the review of this change. The following is somewhat
inefficient:
instance = instances.putIfAbsent(currencyCode,
new Currency(currencyCode, defaultFractionDigits, numericCode));
return (instance != null ? instance : instances.get(currencyCode));
If the putIfAbsent succeeds then the value to return is the newly
constructed Currency instance. So if we track that object then we don't
need to do the additional get():
Currency currencyVal =
new Currency(currencyCode, defaultFractionDigits, numericCode);
instance = instances.putIfAbsent(currencyCode, currencyVal);
return (instance != null ? instance : currencyVal);
Cheers,
David
On 22/03/2012 3:12 AM, [email protected] wrote:
Changeset: 4a5817f9e249
Author: naoto
Date: 2012-03-21 10:10 -0700
URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4a5817f9e249
7145454: JVM wide monitor lock in Currency.getInstance(String)
Reviewed-by: okutsu
! src/share/classes/java/util/Currency.java
