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

[Coleen Phillimore]

Okay with below.  Thanks for pointing out the code that could be changed 
- I agree, they are not really worth doing.
Coleen


On 2/26/2014 3:32 PM, Stefan Karlsson wrote:
> 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
>>>
>>
>