Request for review: Race conditions in java.nio.charset.Charset

[Ulf Zibis]

Am 07.10.2009 23:01, Xueming Shen schrieb:
> Ulf Zibis wrote:
>> Sherman, thanks for your review ...
>>
>> Am 07.10.2009 19:56, Xueming Shen schrieb:
>>> Though I did not write the cache code my reading suggests the 
>>> existing cache
>>> impl tries to avoid the relatively "expensive" synchronization for 
>>> most use scenarios,
>>> your approach however hits the synchronization for each/every lookup 
>>> invocation.
>>> So you should at least consider to put the sync block in a separate 
>>> lookup2.
>>
>> Agreed:
>> (But why separate lookup2? I suspect that Charset.forName() would 
>> occur in excessive loops, so hotspot potentially cold inline it.)
>
> Maybe I meant to say "to put the sync in a separate block"


I have updated the changeset accordingly:
https://bugs.openjdk.java.net/show_bug.cgi?id=100107


>
>>
>> Is synchronization more expensive, than instantiating new Object[] 
>> each time + 2 castings (to String + to Charset)?
>>
>
> The difference is fast cpu + a good gc probably can make 
> "instantiating new Object[] each time + 2 castings" trivial,
> but a synchronization only allows one thread run into the code block 
> and keeps anyone else outside waiting till the
> first done.
>

See my benchmark I posted yesterday.
The synchronized cache is ~10-20 % slower, if only 2 charsets are 
retrieved, but this should not count much, because the operations after 
(de/encoding some/many characters) should be much more expensive.
If 4 charsets are retrieved, the new synchronized cache is up to 800 % 
faster.

-Ulf