RFR (S) 8247296: Optimize JVM_GetDeclaringClass

[David Holmes]

Thanks for the second look Yumin!

David

On 25/07/2020 1:37 am, Yumin Qi wrote:
> Hi, David
> 
> On 7/23/20 11:44 PM, David Holmes wrote:
>>
>> I must stress I am sponsoring Christoph's change and only extended it 
>> within the current file. :) I'm sure there are many, many more 
>> opportunities for similar optimisations. Though you have to be careful 
>> to ensure you don't expose an unhandled oop.
>>
>>> 1) 
>>> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/compileBroker.cpp#L834 
>>>
>>>     Where thread_handle resolved multiple times.
>>
>> True that is unnecessary, but this is a compiler code and I'd need to 
>> extend the review so ... I'll pass on this one.
>>
> That is OK.
>>> 2) 
>>> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/jni.cpp#L1174 
>>>
>>>      Where clazz resolved twice.
>>
>> Fixed this as it is the same pattern in a core runtime file. Webrev 
>> updated in place.
>>
> Looks good!
> 
> 
> Thanks
> 
> Yumin
> 
>> Thanks,
>> David
>>
>>
>>
>>>      Do you want to include those two files in your list?
>>>
>>> Thanks
>>>
>>> Yumin
>>>
>>> On 7/23/20 8:49 PM, David Holmes wrote:
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8247296
>>>> webrev: http://cr.openjdk.java.net/~dholmes/8247296/webrev/
>>>>
>>>> Please review this simple optimization contributed by Christoph 
>>>> Dreis in its initial form and then expanded by me to cover other 
>>>> cases in jvm.cpp.
>>>>
>>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-June/040025.html 
>>>>
>>>>
>>>> There is a common pattern of code of the form:
>>>>
>>>> if 
>>>> (java_lang_Class::is_primitive(JNIHandles::resolve_non_null(ofClass)) || 
>>>>
>>>>     ! 
>>>> java_lang_Class::as_Klass(JNIHandles::resolve_non_null(ofClass))->is_instance_klass()) 
>>>> {
>>>>
>>>> which resolves cls twice. There are also duplicate calls to as_Klass 
>>>> that can be removed in a couple of cases.
>>>>
>>>> Testing: tiers 1 - 3
>>>>
>>>> Thanks,
>>>> David