8037325: Class.getConstructor() performance regression
authorredestad
Wed, 18 Jan 2017 10:24:47 +0100
changeset 43184 ac0aed3194d3
parent 43183 b50e0f90d284
child 43185 d75d9ff8d4e7
8037325: Class.getConstructor() performance regression Reviewed-by: mchung Contributed-by: claes.redestad@oracle.com, sean.mullan@oracle.com
jdk/src/java.base/share/classes/java/lang/Class.java
jdk/src/java.base/share/classes/sun/reflect/misc/ReflectUtil.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<T> 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<T> 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 @@
      *
      * <p> Default policy: allow all clients access with normal Java access
      * control.
+     *
+     * <p> 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() + ".<init>" + argumentTypesToString(parameterTypes));
+        throw new NoSuchMethodException(methodToString("<init>", parameterTypes));
     }
 
     //
@@ -3294,8 +3323,11 @@
     private native Constructor<T>[] 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];
--- 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");
         }