RFR: 8035746: Add missing Klass::oop_is_instanceClassLoader() function

Stefan Karlsson stefan.karlsson at oracle.com
Wed Feb 26 12:32:07 PST 2014


On 2014-02-26 20:38, Coleen Phillimore wrote:
> On 2/25/2014 12:50 PM, Stefan Karlsson wrote:
>> On 2014-02-25 15:15, Mikael Gerdin wrote:
>>> Hi Stefan,
>>>
>>> On Tuesday 25 February 2014 14.43.49 Stefan Karlsson wrote:
>>>> Hi all,
>>>>
>>>> Please, review this small patch to add 
>>>> Klass::oop_is_instanceClassLoader().
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~stefank/8035746/webrev.00
>>> Code change looks good.
>>
>> Thanks.
>>
>>>
>>> There are a couple of places where we check whether or not a Klass is a
>>> ClassLoader, something like:
>>> if (klass->is_subclass_of(SystemDictionary::ClassLoader_klass()))
>>>
>>> These places could now be changed to
>>> klass->oop_is_ClassLoader(), saving a possibly expensive subtype check.
>>>
>>> Should we file an RFE to change these or do you want to change them 
>>> straight
>>> away?
>>
>> I'd prefer a separate RFE.
>
> This change looks good.

Thanks.

>   I assume this is actually for G1 class unloading.

Yes.

> Too bad though I wish you'd change it all in one pass.

I just wanted to bring over this small change from the G1 Class 
Unloading repository and not have to argue whether changing the usages 
is_subclass_of to oop_is_instanceClassLoader was worth it or not.

I did a quick search for this and found these three usages:

1)
src//share/vm/classfile/javaClasses.hpp-  static bool is_subclass(Klass* 
klass) {
src//share/vm/classfile/javaClasses.hpp:    return 
klass->is_subclass_of(SystemDictionary::ClassLoader_klass());
src//share/vm/classfile/javaClasses.hpp-  }

which is unused, AFAICT.

2)
src//share/vm/prims/jvm.cpp:      if 
(!vfst.method()->method_holder()->is_subclass_of(SystemDictionary::ClassLoader_klass())&&
src//share/vm/prims/jvm.cpp- 
!vfst.method()->method_holder()->is_subclass_of(access_controller_klass) &&
src//share/vm/prims/jvm.cpp- 
!vfst.method()->method_holder()->is_subclass_of(privileged_action_klass)) {

I don't think this helps the readability of the code if we change it. I 
don't know if this is performance critical.

thanks,
StefanK

> Thanks,
> Coleen
>
>>
>> thanks,
>> StefanK
>>
>>>
>>> /Mikael
>>>
>>>> RFE:
>>>> https://bugs.openjdk.java.net/browse/JDK-8035746
>>>>
>>>> thanks,
>>>> StefanK
>>
>



More information about the hotspot-dev mailing list