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

[Naoto Sato]

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/

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, 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
>>>>
>>