RRF: 8187123: (reflect) Class#getCanonicalName and Class#getSimpleName is a part of performance issue

[Claes Redestad]

On 2018-05-03 01:29, mandy chung wrote:
>
> On 4/30/18 5:49 AM, Claes Redestad wrote:
>> Hi,
>>
>> please review this patch to enable caching of getCanonicalName and 
>> getSimpleName, repeated calls of which has been reported to be a 
>> performance
>> bottleneck. The caching improves performance of these methods by up 
>> to 20x.
>>
>> Rather than adding new fields to Class itself, which would have 
>> footprint implications on classes, we can piggy-back on 
>> Class$ReflectionData object.
>>
>> Webrev: http://cr.openjdk.java.net/~redestad/8187123/open.02/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8187123
>>
>
> It's a fine idea to leverage ReflectionData to cache the names. Can 
> you update line 2916 for this change?  For example, reflection data 
> caches the names and members.  It will get invalidated when JVM TI 
> RedefineClasses is called.

Done.

>
> newReflectionData could be updated to copy the simpleName and 
> canonicalName from oldReflectionData.  The perf isn't a concern when 
> the class is being redefined.  As the class name is unchanged, it 
> might be good to copy the names from older reflection data.

I prefer not to, as I think we should optimize for the normal cases 
here, which means either a first time create or replacing a cleared 
reference. In neither of these cases there's any cached data to copy 
over, so we'd add code (including at least one branch) that'd never be 
beneficial for the normal case.  And as you say: perf probably isn't a 
primary concern when classes are being redefined, so why add an 
optimization that only applies to that case?

/Claes