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

[Peter Levart]

Hi,

On 08/24/2017 12:24 PM, Peter Levart wrote:
> Hi Claes,
>
> On 08/24/2017 12:13 AM, Claes Redestad wrote:
>>
>>>>
>>>> This allows us to lazily load the $Cache class too, further reducing
>>>> work done during System.initPhase1:
>>>>
>>>> http://cr.openjdk.java.net/~redestad/8186517/jdk.02/
>>>>
>>>
>> /Claes
>
> According to 2nd webrev, StandardCharsets.charsetForName("UTF-8") 
> returns a different instance than 
> StandardCharsets.charsetForName("utf-8"). Perhaps the argument of 
> lookup() should 1st be passed through 
> canonicalize(toLower(charsetName)) and then used in the 3 special-case 
> ifs..
>
> Regards, Peter
>

... but then canonicalization would pull-in the Aliases.

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.

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).

So, what do you think?

Regards, Peter