RFR [10]: 8186517: sun.nio.cs.StandardCharsets$Aliases and ClassMap can be lazily loaded

[Claes Redestad]

Hi Peter,

this seems reasonable, since using the constants in 
java.nio.charset.StandardCharsets might be fairly
common practice and current impl will pull in the Aliases+Classes due to 
canonicalization. I can think
of a few ways to do this, and hopefully we don't have to add a Luxury 
holder class :-)

At this point I'm thinking this should be done as a follow-up though, 
since this RFE is motivated by
the need to move work out from happening during (or before) 
System.initPhase1.

/Claes

On 2017-08-24 19:11, Peter Levart wrote:
> Hi Sherman,
>
> On 08/24/2017 05:56 PM, Xueming Shen wrote:
>>> While looking at the code of StandardCharsets, I noticed a real 
>>> danger of using a thread-unsafe object while it is being modified. 
>>> The method charsets() returns an Iterator<Charset> which lazily 
>>> invokes an unsynchronized lookup() method for each charset as it 
>>> returns them. lookup() uses a Cache (a PreHashedMap) which is not a 
>>> thread-safe object. private lookup() is called from a synchronized 
>>> block of public charsetForName() method, but it is invoked 
>>> unsynchronized from iterator.
>>
>> There were tasty initialization/synchronization/deadlock issues here 
>> in the past (decade ago,
>> happened only on customers' site with specific use scenarios). Our 
>> system initialization
>> "sequence" has been changed and the way the standard charset provider 
>> is being loaded has
>> been changed since, it might be reasonable to reconsider how this 
>> part of code should work,
>> but I would still be very cautious here as the initial goal is for 
>> startup performance.
>
> It would be nice if startup performance improvement extended to 
> programs using just constants from java.nio.charset.StandardCharsets too.
>
> Here's a variant of that:
>
> http://cr.openjdk.java.net/~plevart/jdk10-dev/8186517_StandardCharsets_opt/webrev.02/
>
> Regards, Peter
>
>>
>> -sherman
>>
>>>
>>> One way to fix it would be to enclose the lookup() invocation in 
>>> Iterator.next with a synchronized block, but then I thought that 
>>> maybe a PreHashedMap is not the right tool for the job. What if the 
>>> cache was simply a ConcurrentHashMap which need not be pre-populated 
>>> with all possible keys (saving on footprint) and could therefore be 
>>> constructed eagerly. The whole lookup() could then be 
>>> unsynchronized. Here's what I came up with:
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk10-dev/8186517_StandardCharsets_opt/webrev.01/ 
>>>
>>>
>>> I also ensured that the same instance of Charset object gets 
>>> returned for all aliases of a charset (including standard 3). 
>