hg: jdk8/tl/jdk: 7145454: JVM wide monitor lock in Currency.getInstance(String)

[David Holmes]

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, naoto.sato at oracle.com 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
>>>
>