# HG changeset patch # User redestad # Date 1484731487 -3600 # Node ID ac0aed3194d377ebe003493aad1ea92c0e82b174 # Parent b50e0f90d2846338b42541746ee43c85560de4f7 8037325: Class.getConstructor() performance regression Reviewed-by: mchung Contributed-by: claes.redestad@oracle.com, sean.mullan@oracle.com diff -r b50e0f90d284 -r ac0aed3194d3 jdk/src/java.base/share/classes/java/lang/Class.java --- a/jdk/src/java.base/share/classes/java/lang/Class.java Wed Jan 18 08:02:53 2017 +0800 +++ b/jdk/src/java.base/share/classes/java/lang/Class.java Wed Jan 18 10:24:47 2017 +0100 @@ -508,8 +508,9 @@ public T newInstance() throws InstantiationException, IllegalAccessException { - if (System.getSecurityManager() != null) { - checkMemberAccess(Member.PUBLIC, Reflection.getCallerClass(), false); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), false); } // NOTE: the following code may not be strictly correct under @@ -1223,38 +1224,27 @@ // Perform access check final Class enclosingCandidate = enclosingInfo.getEnclosingClass(); - enclosingCandidate.checkMemberAccess(Member.DECLARED, - Reflection.getCallerClass(), true); - // Client is ok to access declared methods but j.l.Class might not be. - Method[] candidates = AccessController.doPrivileged( - new PrivilegedAction<>() { - @Override - public Method[] run() { - return enclosingCandidate.getDeclaredMethods(); - } - }); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + enclosingCandidate.checkMemberAccess(sm, Member.DECLARED, + Reflection.getCallerClass(), true); + } + Method[] candidates = enclosingCandidate.privateGetDeclaredMethods(false); + /* * Loop over all declared methods; match method name, * number of and type of parameters, *and* return * type. Matching return type is also necessary * because of covariant returns, etc. */ - for(Method m: candidates) { - if (m.getName().equals(enclosingInfo.getName()) ) { - Class[] candidateParamClasses = m.getParameterTypes(); - if (candidateParamClasses.length == parameterClasses.length) { - boolean matches = true; - for(int i = 0; i < candidateParamClasses.length; i++) { - if (!candidateParamClasses[i].equals(parameterClasses[i])) { - matches = false; - break; - } - } - - if (matches) { // finally, check return type - if (m.getReturnType().equals(returnType) ) - return m; - } + ReflectionFactory fact = getReflectionFactory(); + for (Method m : candidates) { + if (m.getName().equals(enclosingInfo.getName()) && + arrayContentsEq(parameterClasses, + fact.getExecutableSharedParameterTypes(m))) { + // finally, check return type + if (m.getReturnType().equals(returnType)) { + return fact.copyMethod(m); } } } @@ -1390,33 +1380,23 @@ // Perform access check final Class enclosingCandidate = enclosingInfo.getEnclosingClass(); - enclosingCandidate.checkMemberAccess(Member.DECLARED, - Reflection.getCallerClass(), true); - // Client is ok to access declared methods but j.l.Class might not be. - Constructor[] candidates = AccessController.doPrivileged( - new PrivilegedAction<>() { - @Override - public Constructor[] run() { - return enclosingCandidate.getDeclaredConstructors(); - } - }); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + enclosingCandidate.checkMemberAccess(sm, Member.DECLARED, + Reflection.getCallerClass(), true); + } + + Constructor[] candidates = enclosingCandidate + .privateGetDeclaredConstructors(false); /* * Loop over all declared constructors; match number * of and type of parameters. */ - for(Constructor c: candidates) { - Class[] candidateParamClasses = c.getParameterTypes(); - if (candidateParamClasses.length == parameterClasses.length) { - boolean matches = true; - for(int i = 0; i < candidateParamClasses.length; i++) { - if (!candidateParamClasses[i].equals(parameterClasses[i])) { - matches = false; - break; - } - } - - if (matches) - return c; + ReflectionFactory fact = getReflectionFactory(); + for (Constructor c : candidates) { + if (arrayContentsEq(parameterClasses, + fact.getExecutableSharedParameterTypes(c))) { + return fact.copyConstructor(c); } } @@ -1446,9 +1426,13 @@ public Class getDeclaringClass() throws SecurityException { final Class candidate = getDeclaringClass0(); - if (candidate != null) - candidate.checkPackageAccess( + if (candidate != null) { + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + candidate.checkPackageAccess(sm, ClassLoader.getClassLoader(Reflection.getCallerClass()), true); + } + } return candidate; } @@ -1496,9 +1480,13 @@ enclosingCandidate = enclosingClass; } - if (enclosingCandidate != null) - enclosingCandidate.checkPackageAccess( + if (enclosingCandidate != null) { + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + enclosingCandidate.checkPackageAccess(sm, ClassLoader.getClassLoader(Reflection.getCallerClass()), true); + } + } return enclosingCandidate; } @@ -1688,7 +1676,10 @@ */ @CallerSensitive public Class[] getClasses() { - checkMemberAccess(Member.PUBLIC, Reflection.getCallerClass(), false); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), false); + } // Privileged so this implementation can look at DECLARED classes, // something the caller might not have privilege to do. The code here @@ -1754,7 +1745,10 @@ */ @CallerSensitive public Field[] getFields() throws SecurityException { - checkMemberAccess(Member.PUBLIC, Reflection.getCallerClass(), true); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), true); + } return copyFields(privateGetPublicFields(null)); } @@ -1841,7 +1835,10 @@ */ @CallerSensitive public Method[] getMethods() throws SecurityException { - checkMemberAccess(Member.PUBLIC, Reflection.getCallerClass(), true); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), true); + } return copyMethods(privateGetPublicMethods()); } @@ -1877,7 +1874,10 @@ */ @CallerSensitive public Constructor[] getConstructors() throws SecurityException { - checkMemberAccess(Member.PUBLIC, Reflection.getCallerClass(), true); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), true); + } return copyConstructors(privateGetDeclaredConstructors(true)); } @@ -1928,7 +1928,10 @@ public Field getField(String name) throws NoSuchFieldException, SecurityException { Objects.requireNonNull(name); - checkMemberAccess(Member.PUBLIC, Reflection.getCallerClass(), true); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), true); + } Field field = getField0(name); if (field == null) { throw new NoSuchFieldException(name); @@ -2034,10 +2037,13 @@ public Method getMethod(String name, Class... parameterTypes) throws NoSuchMethodException, SecurityException { Objects.requireNonNull(name); - checkMemberAccess(Member.PUBLIC, Reflection.getCallerClass(), true); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), true); + } Method method = getMethod0(name, parameterTypes); if (method == null) { - throw new NoSuchMethodException(getName() + "." + name + argumentTypesToString(parameterTypes)); + throw new NoSuchMethodException(methodToString(name, parameterTypes)); } return getReflectionFactory().copyMethod(method); } @@ -2092,8 +2098,12 @@ */ @CallerSensitive public Constructor getConstructor(Class... parameterTypes) - throws NoSuchMethodException, SecurityException { - checkMemberAccess(Member.PUBLIC, Reflection.getCallerClass(), true); + throws NoSuchMethodException, SecurityException + { + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.PUBLIC, Reflection.getCallerClass(), true); + } return getReflectionFactory().copyConstructor( getConstructor0(parameterTypes, Member.PUBLIC)); } @@ -2136,7 +2146,10 @@ */ @CallerSensitive public Class[] getDeclaredClasses() throws SecurityException { - checkMemberAccess(Member.DECLARED, Reflection.getCallerClass(), false); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.DECLARED, Reflection.getCallerClass(), false); + } return getDeclaredClasses0(); } @@ -2185,7 +2198,10 @@ */ @CallerSensitive public Field[] getDeclaredFields() throws SecurityException { - checkMemberAccess(Member.DECLARED, Reflection.getCallerClass(), true); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.DECLARED, Reflection.getCallerClass(), true); + } return copyFields(privateGetDeclaredFields(false)); } @@ -2244,7 +2260,10 @@ */ @CallerSensitive public Method[] getDeclaredMethods() throws SecurityException { - checkMemberAccess(Member.DECLARED, Reflection.getCallerClass(), true); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.DECLARED, Reflection.getCallerClass(), true); + } return copyMethods(privateGetDeclaredMethods(false)); } @@ -2289,7 +2308,10 @@ */ @CallerSensitive public Constructor[] getDeclaredConstructors() throws SecurityException { - checkMemberAccess(Member.DECLARED, Reflection.getCallerClass(), true); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.DECLARED, Reflection.getCallerClass(), true); + } return copyConstructors(privateGetDeclaredConstructors(false)); } @@ -2338,7 +2360,10 @@ public Field getDeclaredField(String name) throws NoSuchFieldException, SecurityException { Objects.requireNonNull(name); - checkMemberAccess(Member.DECLARED, Reflection.getCallerClass(), true); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.DECLARED, Reflection.getCallerClass(), true); + } Field field = searchFields(privateGetDeclaredFields(false), name); if (field == null) { throw new NoSuchFieldException(name); @@ -2399,10 +2424,13 @@ public Method getDeclaredMethod(String name, Class... parameterTypes) throws NoSuchMethodException, SecurityException { Objects.requireNonNull(name); - checkMemberAccess(Member.DECLARED, Reflection.getCallerClass(), true); + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.DECLARED, Reflection.getCallerClass(), true); + } Method method = searchMethods(privateGetDeclaredMethods(false), name, parameterTypes); if (method == null) { - throw new NoSuchMethodException(getName() + "." + name + argumentTypesToString(parameterTypes)); + throw new NoSuchMethodException(methodToString(name, parameterTypes)); } return getReflectionFactory().copyMethod(method); } @@ -2448,8 +2476,13 @@ */ @CallerSensitive public Constructor getDeclaredConstructor(Class... parameterTypes) - throws NoSuchMethodException, SecurityException { - checkMemberAccess(Member.DECLARED, Reflection.getCallerClass(), true); + throws NoSuchMethodException, SecurityException + { + SecurityManager sm = System.getSecurityManager(); + if (sm != null) { + checkMemberAccess(sm, Member.DECLARED, Reflection.getCallerClass(), true); + } + return getReflectionFactory().copyConstructor( getConstructor0(parameterTypes, Member.DECLARED)); } @@ -2697,51 +2730,49 @@ * *

Default policy: allow all clients access with normal Java access * control. + * + *

NOTE: should only be called if a SecurityManager is installed */ - private void checkMemberAccess(int which, Class caller, boolean checkProxyInterfaces) { - final SecurityManager s = System.getSecurityManager(); - if (s != null) { - /* Default policy allows access to all {@link Member#PUBLIC} members, - * as well as access to classes that have the same class loader as the caller. - * In all other cases, it requires RuntimePermission("accessDeclaredMembers") - * permission. - */ - final ClassLoader ccl = ClassLoader.getClassLoader(caller); + private void checkMemberAccess(SecurityManager sm, int which, + Class caller, boolean checkProxyInterfaces) { + /* Default policy allows access to all {@link Member#PUBLIC} members, + * as well as access to classes that have the same class loader as the caller. + * In all other cases, it requires RuntimePermission("accessDeclaredMembers") + * permission. + */ + final ClassLoader ccl = caller.getClassLoader0(); + if (which != Member.PUBLIC) { final ClassLoader cl = getClassLoader0(); - if (which != Member.PUBLIC) { - if (ccl != cl) { - s.checkPermission(SecurityConstants.CHECK_MEMBER_ACCESS_PERMISSION); - } + if (ccl != cl) { + sm.checkPermission(SecurityConstants.CHECK_MEMBER_ACCESS_PERMISSION); } - this.checkPackageAccess(ccl, checkProxyInterfaces); } + this.checkPackageAccess(sm, ccl, checkProxyInterfaces); } /* * Checks if a client loaded in ClassLoader ccl is allowed to access this * class under the current package access policy. If access is denied, * throw a SecurityException. + * + * NOTE: this method should only be called if a SecurityManager is active */ - private void checkPackageAccess(final ClassLoader ccl, boolean checkProxyInterfaces) { - final SecurityManager s = System.getSecurityManager(); - if (s != null) { - final ClassLoader cl = getClassLoader0(); - - if (ReflectUtil.needsPackageAccessCheck(ccl, cl)) { - String name = this.getName(); - int i = name.lastIndexOf('.'); - if (i != -1) { - // skip the package access check on a proxy class in default proxy package - String pkg = name.substring(0, i); - if (!Proxy.isProxyClass(this) || ReflectUtil.isNonPublicProxyClass(this)) { - s.checkPackageAccess(pkg); - } + private void checkPackageAccess(SecurityManager sm, final ClassLoader ccl, + boolean checkProxyInterfaces) { + final ClassLoader cl = getClassLoader0(); + + if (ReflectUtil.needsPackageAccessCheck(ccl, cl)) { + String pkg = this.getPackageName(); + if (pkg != null && !pkg.isEmpty()) { + // skip the package access check on a proxy class in default proxy package + if (!Proxy.isProxyClass(this) || ReflectUtil.isNonPublicProxyClass(this)) { + sm.checkPackageAccess(pkg); } } - // check package access on the proxy interfaces - if (checkProxyInterfaces && Proxy.isProxyClass(this)) { - ReflectUtil.checkProxyPackageAccess(ccl, this.getInterfaces()); - } + } + // check package access on the proxy interfaces + if (checkProxyInterfaces && Proxy.isProxyClass(this)) { + ReflectUtil.checkProxyPackageAccess(ccl, this.getInterfaces()); } } @@ -2755,11 +2786,9 @@ while (c.isArray()) { c = c.getComponentType(); } - String baseName = c.getName(); - int index = baseName.lastIndexOf('.'); - if (index != -1) { - name = baseName.substring(0, index).replace('.', '/') - +"/"+name; + String baseName = c.getPackageName(); + if (baseName != null && !baseName.isEmpty()) { + name = baseName.replace('.', '/') + "/" + name; } } else { name = name.substring(1); @@ -3233,7 +3262,7 @@ return constructor; } } - throw new NoSuchMethodException(getName() + "." + argumentTypesToString(parameterTypes)); + throw new NoSuchMethodException(methodToString("", parameterTypes)); } // @@ -3294,8 +3323,11 @@ private native Constructor[] getDeclaredConstructors0(boolean publicOnly); private native Class[] getDeclaredClasses0(); - private static String argumentTypesToString(Class[] argTypes) { - StringJoiner sj = new StringJoiner(", ", "(", ")"); + /** + * Helper method to get the method name from arguments. + */ + private String methodToString(String name, Class[] argTypes) { + StringJoiner sj = new StringJoiner(", ", getName() + "." + name + "(", ")"); if (argTypes != null) { for (int i = 0; i < argTypes.length; i++) { Class c = argTypes[i]; diff -r b50e0f90d284 -r ac0aed3194d3 jdk/src/java.base/share/classes/sun/reflect/misc/ReflectUtil.java --- a/jdk/src/java.base/share/classes/sun/reflect/misc/ReflectUtil.java Wed Jan 18 08:02:53 2017 +0800 +++ b/jdk/src/java.base/share/classes/sun/reflect/misc/ReflectUtil.java Wed Jan 18 10:24:47 2017 +0100 @@ -96,7 +96,7 @@ final Class declaringClass = m.getDeclaringClass(); - checkPackageAccess(declaringClass); + privateCheckPackageAccess(sm, declaringClass); if (Modifier.isPublic(m.getModifiers()) && Modifier.isPublic(declaringClass.getModifiers())) @@ -114,9 +114,27 @@ * also check the package access on the proxy interfaces. */ public static void checkPackageAccess(Class clazz) { - checkPackageAccess(clazz.getName()); + SecurityManager s = System.getSecurityManager(); + if (s != null) { + privateCheckPackageAccess(s, clazz); + } + } + + /** + * NOTE: should only be called if a SecurityManager is installed + */ + private static void privateCheckPackageAccess(SecurityManager s, Class clazz) { + while (clazz.isArray()) { + clazz = clazz.getComponentType(); + } + + String pkg = clazz.getPackageName(); + if (pkg != null && !pkg.isEmpty()) { + s.checkPackageAccess(pkg); + } + if (isNonPublicProxyClass(clazz)) { - checkProxyPackageAccess(clazz); + privateCheckProxyPackageAccess(s, clazz); } } @@ -195,15 +213,21 @@ public static void checkProxyPackageAccess(Class clazz) { SecurityManager s = System.getSecurityManager(); if (s != null) { - // check proxy interfaces if the given class is a proxy class - if (Proxy.isProxyClass(clazz)) { - for (Class intf : clazz.getInterfaces()) { - checkPackageAccess(intf); - } + privateCheckProxyPackageAccess(s, clazz); + } + } + + /** + * NOTE: should only be called if a SecurityManager is installed + */ + private static void privateCheckProxyPackageAccess(SecurityManager s, Class clazz) { + // check proxy interfaces if the given class is a proxy class + if (Proxy.isProxyClass(clazz)) { + for (Class intf : clazz.getInterfaces()) { + privateCheckPackageAccess(s, intf); } } } - /** * Access check on the interfaces that a proxy class implements and throw * {@code SecurityException} if it accesses a restricted package from @@ -220,7 +244,7 @@ for (Class intf : interfaces) { ClassLoader cl = intf.getClassLoader(); if (needsPackageAccessCheck(ccl, cl)) { - checkPackageAccess(intf); + privateCheckPackageAccess(sm, intf); } } } @@ -236,10 +260,11 @@ * package that bypasses checkPackageAccess. */ public static boolean isNonPublicProxyClass(Class cls) { - String name = cls.getName(); - int i = name.lastIndexOf('.'); - String pkg = (i != -1) ? name.substring(0, i) : ""; - return Proxy.isProxyClass(cls) && !pkg.startsWith(PROXY_PACKAGE); + if (!Proxy.isProxyClass(cls)) { + return false; + } + String pkg = cls.getPackageName(); + return pkg == null || !pkg.startsWith(PROXY_PACKAGE); } /** @@ -255,7 +280,7 @@ // check if it is a valid proxy instance if (proxy == null || !Proxy.isProxyClass(proxy.getClass())) { throw new IllegalArgumentException("Not a Proxy instance"); -} + } if (Modifier.isStatic(method.getModifiers())) { throw new IllegalArgumentException("Can't handle static method"); }