Replace usage of CopyOnWriteArraySet.

Use a List<Class<?>> as the cache key instead, but
dedup and check for nulls in the list using a HashSet
instead. This will improve performance for large
lists of interfaces.

bug: 10316383

Change-Id: Id0c36a731e6e3cb39bf36c083180c0c4da5c2dd4
(cherry picked from commit 6074af533f57a93e1b8e1ab63dce6a264a4d5dae)

2 files changed, 40 insertions, 22 deletions

diff --git a/libart/src/main/java/java/lang/ClassLoader.java b/libart/src/main/java/java/lang/ClassLoader.java
index fb2eb8f..9079dc4 100644
--- a/libart/src/main/java/java/lang/ClassLoader.java
+++ b/libart/src/main/java/java/lang/ClassLoader.java
@@ -42,8 +42,8 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 /**
  * Loads classes and resources from a repository. One or more class loaders are
@@ -100,8 +100,8 @@ public abstract class ClassLoader {
      *
      * @hide
      */
-    public final Map<Set<Class<?>>, Class<?>> proxyCache
-            = Collections.synchronizedMap(new HashMap<Set<Class<?>>, Class<?>>());
+    public final Map<List<Class<?>>, Class<?>> proxyCache =
+            new HashMap<List<Class<?>>, Class<?>>();
 
     /**
      * Create the system class loader. Note this is NOT the bootstrap class
diff --git a/libart/src/main/java/java/lang/reflect/Proxy.java b/libart/src/main/java/java/lang/reflect/Proxy.java
index 51b3ad5..31f9cd9 100644
--- a/libart/src/main/java/java/lang/reflect/Proxy.java
+++ b/libart/src/main/java/java/lang/reflect/Proxy.java
@@ -25,7 +25,6 @@ import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
 import libcore.util.EmptyArray;
 
 /**
@@ -40,7 +39,7 @@ public class Proxy implements Serializable {
 
     private static final long serialVersionUID = -2222568056686623797L;
 
-    private static int NextClassNameIndex = 0;
+    private static int nextClassNameIndex = 0;
 
     /**
      * Orders methods by their name, parameters, return type and inheritance relationship.
@@ -117,22 +116,33 @@ public class Proxy implements Serializable {
             throw new NullPointerException("interfaces == null");
         }
 
-        Set<Class<?>> interfacesSet = new CopyOnWriteArraySet<Class<?>>(Arrays.asList(interfaces));
+        // Make a copy of the list early on because we're using the list as a
+        // cache key and we don't want it changing under us.
+        final List<Class<?>> interfaceList = new ArrayList<Class<?>>(interfaces.length);
+        Collections.addAll(interfaceList, interfaces);
+
+        // We use a HashSet *only* for detecting duplicates and null entries. We
+        // can't use it as our cache key because we need to preserve the order in
+        // which these interfaces were specified. (Different orders should define
+        // different proxies.)
+        final Set<Class<?>> interfaceSet = new HashSet<Class<?>>(interfaceList);
+        if (interfaceSet.contains(null)) {
+            throw new NullPointerException("interface list contains null: " + interfaceList);
+        }
 
-        Class<?> proxy = loader.proxyCache.get(interfacesSet);
-        if (proxy != null) {
-            return proxy;
+        if (interfaceSet.size() != interfaces.length) {
+            throw new IllegalArgumentException("duplicate interface in list: " + interfaceList);
         }
 
-        if (interfacesSet.size() != interfaces.length) {
-            throw new IllegalArgumentException(
-                    "interfaces has duplicates: " + Arrays.toString(interfaces));
+        synchronized (loader.proxyCache) {
+            Class<?> proxy = loader.proxyCache.get(interfaceList);
+            if (proxy != null) {
+                return proxy;
+            }
         }
+
         String commonPackageName = null;
         for (Class<?> c : interfaces) {
-            if (c == null) {
-                throw new NullPointerException("interfaces contained a null element");
-            }
             if (!c.isInterface()) {
                 throw new IllegalArgumentException(c + " is not an interface");
             }
@@ -152,11 +162,6 @@ public class Proxy implements Serializable {
             }
         }
 
-        String baseName = commonPackageName != null && !commonPackageName.isEmpty()
-                ? commonPackageName + ".$Proxy"
-                : "$Proxy";
-        String name = baseName + NextClassNameIndex++;
-
         List<Method> methods = getMethods(interfaces);
         Collections.sort(methods, ORDER_BY_SIGNATURE_AND_SUBTYPE);
         validateReturnTypes(methods);
@@ -167,8 +172,21 @@ public class Proxy implements Serializable {
             methodsArray[i] = methods.get(i).getArtMethod();
         }
         Class<?>[][] exceptionsArray = exceptions.toArray(new Class<?>[exceptions.size()][]);
-        Class<?> result = generateProxy(name, interfaces, loader, methodsArray, exceptionsArray);
-        loader.proxyCache.put(interfacesSet, result);
+
+        String baseName = commonPackageName != null && !commonPackageName.isEmpty()
+                ? commonPackageName + ".$Proxy"
+                : "$Proxy";
+
+        Class<?> result;
+        synchronized (loader.proxyCache) {
+            result = loader.proxyCache.get(interfaceSet);
+            if (result == null) {
+                String name = baseName + nextClassNameIndex++;
+                result = generateProxy(name, interfaces, loader, methodsArray, exceptionsArray);
+                loader.proxyCache.put(interfaceList, result);
+            }
+        }
+
         return result;
     }