Merge "Remove unnecessary reflection lookup in Animators." into lmp-mr1-dev

core/java/android/animation/PropertyValuesHolder.java

@@ -105,10 +105,6 @@ public class PropertyValuesHolder implements Cloneable {
     private static final HashMap<Class, HashMap<String, Method>> sGetterPropertyMap =
             new HashMap<Class, HashMap<String, Method>>();
 
-    // This lock is used to ensure that only one thread is accessing the property maps
-    // at a time.
-    final ReentrantReadWriteLock mPropertyMapLock = new ReentrantReadWriteLock();
-
     // Used to pass single value to varargs parameter in setter invocation
     final Object[] mTmpValueArray = new Object[1];
 
@@ -737,16 +733,19 @@ public class PropertyValuesHolder implements Cloneable {
             HashMap<Class, HashMap<String, Method>> propertyMapMap,
             String prefix, Class valueType) {
         Method setterOrGetter = null;
-        try {
+        synchronized(propertyMapMap) {
             // Have to lock property map prior to reading it, to guard against
             // another thread putting something in there after we've checked it
             // but before we've added an entry to it
-            mPropertyMapLock.writeLock().lock();
             HashMap<String, Method> propertyMap = propertyMapMap.get(targetClass);
+            boolean wasInMap = false;
             if (propertyMap != null) {
+                wasInMap = propertyMap.containsKey(mPropertyName);
+                if (wasInMap) {
                     setterOrGetter = propertyMap.get(mPropertyName);
                 }
-            if (setterOrGetter == null) {
+            }
+            if (!wasInMap) {
                 setterOrGetter = getPropertyFunction(targetClass, prefix, valueType);
                 if (propertyMap == null) {
                     propertyMap = new HashMap<String, Method>();
@@ -754,8 +753,6 @@ public class PropertyValuesHolder implements Cloneable {
                 }
                 propertyMap.put(mPropertyName, setterOrGetter);
             }
-        } finally {
-            mPropertyMapLock.writeLock().unlock();
         }
         return setterOrGetter;
     }
@@ -811,6 +808,8 @@ public class PropertyValuesHolder implements Cloneable {
                 mProperty = null;
             }
         }
+        // We can't just say 'else' here because the catch statement sets mProperty to null.
+        if (mProperty == null) {
             Class targetClass = target.getClass();
             if (mSetter == null) {
                 setupSetter(targetClass);
@@ -839,6 +838,7 @@ public class PropertyValuesHolder implements Cloneable {
                 }
             }
         }
+    }
 
     private Object convertBack(Object value) {
         if (mConverter != null) {
@@ -1178,19 +1178,27 @@ public class PropertyValuesHolder implements Cloneable {
                 return;
             }
             // Check new static hashmap<propName, int> for setter method
-            try {
+            synchronized(sJNISetterPropertyMap) {
-                mPropertyMapLock.writeLock().lock();
                 HashMap<String, Long> propertyMap = sJNISetterPropertyMap.get(targetClass);
+                boolean wasInMap = false;
                 if (propertyMap != null) {
+                    wasInMap = propertyMap.containsKey(mPropertyName);
+                    if (wasInMap) {
                         Long jniSetter = propertyMap.get(mPropertyName);
                         if (jniSetter != null) {
                             mJniSetter = jniSetter;
                         }
                     }
-                if (mJniSetter == 0) {
+                }
+                if (!wasInMap) {
                     String methodName = getMethodName("set", mPropertyName);
+                    try {
                         mJniSetter = nGetIntMethod(targetClass, methodName);
-                    if (mJniSetter != 0) {
+                    } catch (NoSuchMethodError e) {
+                        // Couldn't find it via JNI - try reflection next. Probably means the method
+                        // doesn't exist, or the type is wrong. An error will be logged later if
+                        // reflection fails as well.
+                    }
                     if (propertyMap == null) {
                         propertyMap = new HashMap<String, Long>();
                         sJNISetterPropertyMap.put(targetClass, propertyMap);
@@ -1198,13 +1206,6 @@ public class PropertyValuesHolder implements Cloneable {
                     propertyMap.put(mPropertyName, mJniSetter);
                 }
             }
-            } catch (NoSuchMethodError e) {
-                // Couldn't find it via JNI - try reflection next. Probably means the method
-                // doesn't exist, or the type is wrong. An error will be logged later if
-                // reflection fails as well.
-            } finally {
-                mPropertyMapLock.writeLock().unlock();
-            }
             if (mJniSetter == 0) {
                 // Couldn't find method through fast JNI approach - just use reflection
                 super.setupSetter(targetClass);
@@ -1315,19 +1316,27 @@ public class PropertyValuesHolder implements Cloneable {
                 return;
             }
             // Check new static hashmap<propName, int> for setter method
-            try {
+            synchronized (sJNISetterPropertyMap) {
-                mPropertyMapLock.writeLock().lock();
                 HashMap<String, Long> propertyMap = sJNISetterPropertyMap.get(targetClass);
+                boolean wasInMap = false;
                 if (propertyMap != null) {
+                    wasInMap = propertyMap.containsKey(mPropertyName);
+                    if (wasInMap) {
                         Long jniSetter = propertyMap.get(mPropertyName);
                         if (jniSetter != null) {
                             mJniSetter = jniSetter;
                         }
                     }
-                if (mJniSetter == 0) {
+                }
+                if (!wasInMap) {
                     String methodName = getMethodName("set", mPropertyName);
+                    try {
                         mJniSetter = nGetFloatMethod(targetClass, methodName);
-                    if (mJniSetter != 0) {
+                    } catch (NoSuchMethodError e) {
+                        // Couldn't find it via JNI - try reflection next. Probably means the method
+                        // doesn't exist, or the type is wrong. An error will be logged later if
+                        // reflection fails as well.
+                    }
                     if (propertyMap == null) {
                         propertyMap = new HashMap<String, Long>();
                         sJNISetterPropertyMap.put(targetClass, propertyMap);
@@ -1335,13 +1344,6 @@ public class PropertyValuesHolder implements Cloneable {
                     propertyMap.put(mPropertyName, mJniSetter);
                 }
             }
-            } catch (NoSuchMethodError e) {
-                // Couldn't find it via JNI - try reflection next. Probably means the method
-                // doesn't exist, or the type is wrong. An error will be logged later if
-                // reflection fails as well.
-            } finally {
-                mPropertyMapLock.writeLock().unlock();
-            }
             if (mJniSetter == 0) {
                 // Couldn't find method through fast JNI approach - just use reflection
                 super.setupSetter(targetClass);
@@ -1419,16 +1421,19 @@ public class PropertyValuesHolder implements Cloneable {
             if (mJniSetter != 0) {
                 return;
             }
-            try {
+            synchronized(sJNISetterPropertyMap) {
-                mPropertyMapLock.writeLock().lock();
                 HashMap<String, Long> propertyMap = sJNISetterPropertyMap.get(targetClass);
+                boolean wasInMap = false;
                 if (propertyMap != null) {
-                    Long jniSetterLong = propertyMap.get(mPropertyName);
+                    wasInMap = propertyMap.containsKey(mPropertyName);
-                    if (jniSetterLong != null) {
+                    if (wasInMap) {
-                        mJniSetter = jniSetterLong;
+                        Long jniSetter = propertyMap.get(mPropertyName);
+                        if (jniSetter != null) {
+                            mJniSetter = jniSetter;
                         }
                     }
-                if (mJniSetter == 0) {
+                }
+                if (!wasInMap) {
                     String methodName = getMethodName("set", mPropertyName);
                     calculateValue(0f);
                     float[] values = (float[]) getAnimatedValue();
@@ -1437,9 +1442,13 @@ public class PropertyValuesHolder implements Cloneable {
                         mJniSetter = nGetMultipleFloatMethod(targetClass, methodName, numParams);
                     } catch (NoSuchMethodError e) {
                         // try without the 'set' prefix
-                        mJniSetter = nGetMultipleFloatMethod(targetClass, mPropertyName, numParams);
+                        try {
+                            mJniSetter = nGetMultipleFloatMethod(targetClass, mPropertyName,
+                                    numParams);
+                        } catch (NoSuchMethodError e2) {
+                            // just try reflection next
+                        }
                     }
-                    if (mJniSetter != 0) {
                     if (propertyMap == null) {
                         propertyMap = new HashMap<String, Long>();
                         sJNISetterPropertyMap.put(targetClass, propertyMap);
@@ -1447,9 +1456,6 @@ public class PropertyValuesHolder implements Cloneable {
                     propertyMap.put(mPropertyName, mJniSetter);
                 }
            }
-            } finally {
-                mPropertyMapLock.writeLock().unlock();
-            }
         }
     }
 
@@ -1522,16 +1528,19 @@ public class PropertyValuesHolder implements Cloneable {
             if (mJniSetter != 0) {
                 return;
             }
-            try {
+            synchronized(sJNISetterPropertyMap) {
-                mPropertyMapLock.writeLock().lock();
                 HashMap<String, Long> propertyMap = sJNISetterPropertyMap.get(targetClass);
+                boolean wasInMap = false;
                 if (propertyMap != null) {
-                    Long jniSetterLong = propertyMap.get(mPropertyName);
+                    wasInMap = propertyMap.containsKey(mPropertyName);
-                    if (jniSetterLong != null) {
+                    if (wasInMap) {
-                        mJniSetter = jniSetterLong;
+                        Long jniSetter = propertyMap.get(mPropertyName);
+                        if (jniSetter != null) {
+                            mJniSetter = jniSetter;
                         }
                     }
-                if (mJniSetter == 0) {
+                }
+                if (!wasInMap) {
                     String methodName = getMethodName("set", mPropertyName);
                     calculateValue(0f);
                     int[] values = (int[]) getAnimatedValue();
@@ -1540,9 +1549,13 @@ public class PropertyValuesHolder implements Cloneable {
                         mJniSetter = nGetMultipleIntMethod(targetClass, methodName, numParams);
                     } catch (NoSuchMethodError e) {
                         // try without the 'set' prefix
-                        mJniSetter = nGetMultipleIntMethod(targetClass, mPropertyName, numParams);
+                        try {
+                            mJniSetter = nGetMultipleIntMethod(targetClass, mPropertyName,
+                                    numParams);
+                        } catch (NoSuchMethodError e2) {
+                            // couldn't find it.
+                        }
                     }
-                    if (mJniSetter != 0) {
                     if (propertyMap == null) {
                         propertyMap = new HashMap<String, Long>();
                         sJNISetterPropertyMap.put(targetClass, propertyMap);
@@ -1550,9 +1563,6 @@ public class PropertyValuesHolder implements Cloneable {
                     propertyMap.put(mPropertyName, mJniSetter);
                 }
             }
-            } finally {
-                mPropertyMapLock.writeLock().unlock();
-            }
         }
     }