RFR 6642881: Improve performance of Class.getClassLoader() (original) (raw)

Frederic Parain frederic.parain at oracle.com
Tue Jun 24 15:14:58 UTC 2014

Looks good to me.

Thanks,

Fred

On 24/06/2014 17:11, Coleen Phillimore wrote: >> On 6/24/14, 4:41 AM, Frederic Parain wrote: >> Hi Coleen, >>>> It seems that there's still a reference to JVMGetClassLoader >> in file jdk/src/share/native/common/checkcode.c. The code looks >> like dead code, but it would be nice to clean it up. >> I removed this code. There are no other instances of the macro > BROKENJAVAC. I update the copyrights when I commit the changeset. >> http://cr.openjdk.java.net/~coleenp/6642881jdk5/ >> Thanks! > Coleen >>>> 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/6642881jdk4/ >>> 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 >>>>>>>>>>>

Frederic Parain - Oracle Grenoble Engineering Center - France Phone: +33 4 76 18 81 17 Email: Frederic.Parain at oracle.com