> On Apr 13, 2016, at 8:43 AM, Chris Hegarty <chris.hega...@oracle.com> wrote:
>
> This review is for the second significant part of the changes for JEP
> 260 [1]. When created, the jdk.unsupported module [2] initially
> contained the sun.misc package. This issue is will move all the
> non-Critical APIs out of sun.reflect, leaving only the critical ones, at
> which point sun.reflect can be moved to the jdk.unsupported module.
>
> http://cr.openjdk.java.net/~chegar/8137058/
> https://bugs.openjdk.java.net/browse/JDK-8137058
>
> Summary of the changes:
>
> - Move all existing sun.reflect types to jdk.internal.reflect, and
> fix up references in the libraries, and the VM, to use the new internal
> location.
I hope the getCallerClass(int depth) method is moved out of
jdk.internal.reflect.Reflection. JDK is not using it and it’s retained for
existing libraries to migrate to the new StackWalker API. Of course the cost
is to add a native library and the build infra work has made this
straight-forward.
I agree with Alan that the @deprecated javadoc of
sun.reflect.Reflection::getCallerClass should be updated to make it clear.
What about:
/**
* @deprecated This method is an internal API and will be removed in JDK 10.
* Use {@link StackWalker} to walk the stack and obtain the caller class
* with {@link StackWalker.StackFrame#getDeclaringClass} instead.
*/
This patch will likely impact existing libraries that filter out reflection
frames (IIRC Groovy and log4j may be examples) doing
Class::getName().startsWith(“sun.reflect”). It may worth call out this
incompatibility in JEP 260.
StackStreamFactory.java shows an example:
1085 if (c.getName().startsWith("sun.reflect") &&
This should be changed to “jdk.internal.reflect”.
test/java/lang/StackWalker/EmbeddedStackWalkTest.java and maybe a few other
stack walker tests. You may want to check any other of any similar pattern in
the JDK code/tests (I think I got them all)
The hotspot change drops the package name prefix in javaClasses.hpp which is
inconsistent with other classes. I found including jdk_internal_reflect_ in
the class name is useful. Coleen and others from the hotspot team may have
opinion on this.
FX will need to adjust for this patch (cc’ing Kevin to prepare for this)
Mandy
