RFR 6642881: Improve performance of Class.getClassLoader()

[Coleen Phillimore]

Fred,

Thank you for finding this.  Yes, I meant to clean this up with the bug 
to remove JVM_GetClassLoader but I should remove this with this change 
instead, since the other change will be in hotspot only.
Yes, it's dead code.

Thanks!
Coleen

On 6/24/14, 4:41 AM, Frederic Parain wrote:
> Hi Coleen,
>
> It seems that there's still a reference to JVM_GetClassLoader
> in file jdk/src/share/native/common/check_code.c. The code looks
> like dead code, but it would be nice to clean it up.
>
> Thanks,
>
> Fred
>
> On 06/24/2014 01:45 AM, Coleen Phillimore wrote:
>>
>> Please review a change to the JDK code for adding classLoader field to
>> the instances of java/lang/Class.  This change restricts reflection from
>> changing access to the classLoader field.  In the spec,
>> AccessibleObject.setAccessible() may throw SecurityException if the
>> accessibility of an object may not be changed:
>>
>> http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean) 
>>
>>
>>
>> Only AccessibleObject.java has changes from the previous version of this
>> change.
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/
>> bug link https://bugs.openjdk.java.net/browse/JDK-6642881
>>
>> Thanks,
>> Coleen
>>
>> On 6/19/14, 11:01 PM, David Holmes wrote:
>>> On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote:
>>>> Hi Mandy,
>>>>
>>>> On 19 jun 2014, at 22:34, Mandy Chung <mandy.chung at oracle.com> wrote:
>>>>
>>>>> On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 19 jun 2014, at 20:46, Coleen Phillimore
>>>>>> <coleen.phillimore at oracle.com> wrote:
>>>>>>> On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote:
>>>>>>>> Have you considered hiding the Class.classLoader field from
>>>>>>>> reflection? I’m not sure it is necessary but I’m not to keen on
>>>>>>>> the idea of people poking at this field with Unsafe (which should
>>>>>>>> go away in 9 but …).
>>>>>>> I don't know how to hide the field from reflection. It's a
>>>>>>> private final field so you need to get priviledges to make it
>>>>>>> setAccessible.  If you mean injecting it on the JVM side, the
>>>>>>> reason for this change is so that the JIT compilers can inline
>>>>>>> this call and not have to call into the JVM to get the class 
>>>>>>> loader.
>>>>>>>
>>>>>> There is sun.reflect.Reflection.registerFieldsToFilter() that hides
>>>>>> a field from being found using reflection. It might very well be
>>>>>> the case that private and final is enough, I haven’t thought this
>>>>>> through 100%. On the other hand, is there a reason to give users
>>>>>> access through the field instead of having to use
>>>>>> Class.getClassLoader()?
>>>>>>
>>>>> There are many getter methods that returns a private final field.
>>>>> I'm not sure if hiding the field is necessary nor a good precedence
>>>>> to set. Accessing a private field requires "accessDeclaredMember"
>>>>> permission although it's a different security check (vs
>>>>> "getClassLoader"
>>>>> permission).  Arguably before this new classLoader field, one could
>>>>> call Class.getClassLoader0() via reflection to get a hold of class
>>>>> loader.
>>>>>
>>>>> Perhaps you are concerned that the "accessDeclaredMember" permission
>>>>> is too coarse-grained?  I think the security team is looking into
>>>>> the improvement in that regards.
>>>>
>>>> I think I’m a bit worried about two things, first as you wrote,
>>>> “accessDeclaredMember” isn’t the same as “getClassLoader”, but since
>>>> you could get the class loader through getClassLoader0() that
>>>> shouldn’t be a new issue.
>>>>
>>>> The second thing is that IIRC there are ways to set a final field
>>>> after initialization. I’m not sure we need to care about that either
>>>> if you need Unsafe to do it.
>>>
>>> Normal reflection can set a final field if you can successfully call
>>> setAccessible(true) on the Field object.
>>>
>>> David
>>> -----
>>>
>>>
>>>> cheers
>>>> /Joel
>>>>
>>
>