[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-dev
Subject: Re: RFR 6642881: Improve performance of Class.getClassLoader()
From: Frederic Parain <frederic.parain () oracle ! com>
Date: 2014-06-24 15:14:58
Message-ID: 53A995F2.9090502 () oracle ! com
[Download RAW message or body]
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 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.
>
> I removed this code. There are no other instances of the macro
> BROKEN_JAVAC. I update the copyrights when I commit the changeset.
>
> http://cr.openjdk.java.net/~coleenp/6642881_jdk_5/
>
> 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/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@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@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@oracle.com
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic