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

[Claes Redestad]

On 2017-08-25 19:59, Xueming Shen wrote:
> On 08/24/2017 01:00 PM, Claes Redestad wrote:
>>
>>
>> On 2017-08-24 20:06, Claes Redestad wrote:
>>
>>
>> http://cr.openjdk.java.net/~redestad/8186517/jdk.03/
>>
>> Manually verified that generated charsets look correct and ran all 
>> charset tests without issue.
>>
>>>
>>> Should be a nice reduction in retained objects on minimal apps.
>>
>> 425 fewer objects allocated, -16.5Kb.
>>
>> /Claes
>
> Claes,
>
> It might be worth adding the "instance" field  for the utf-16/be/le as 
> well?  otherwise it
> appears a reference to any of the 
> java.nio.charset.StandardCharsets.UTF_8/ISO_8859_1/
> US_ASCII will still trigger the mapping loadup in 
> sun.nio.cs.StandardCharsets.

Right, this is something Peter prototyped a fix for elsewhere in this 
thread, but that I
think we can leave for a follow-up.  There's some complexity around 
when/how they
are added to the Cache, and we need to either add these as branches in 
the lookup
method or ensure that we can add them to the cache as instances are 
created in a
thread-safe way (CHM).

>
> java.nio.charset.Charset.java:642
> I doubt the "identity" still works here, does it? can CDS play the 
> magic here?

String literals are interned, so identity works here even without them 
being named
constants (and I can verify the startup optimization to not run through 
checkName is
still in effect)

>
> Take a step back, it might be reasonable to consider to insert the 
> "aliases_xyz"
> into individual class directly instead of keeping them in provider ... 
> not sure whether
> or not it is worth though, I think we can leave it for next visit :-)

We'd have to load fewer methods - which would be a gain at some level 
(hard to
measure) - but we'd have to generate all the classes, or hard-code the 
alias arrays
for the built-ins. There's also that icky SJIS / sun.nio.cs.map case.

Nothing insurmountable, but perhaps too little gain for the effort.

>
> btw, there are some tests at closed test repo for sun/nio/cs need to 
> be run as well. I should
> bring them into openjdk when have time.

All pass.

Thanks!

/Claes