Remove unnecessary reflection lookup in Animators.
Bug 17978210
When Properties are used with PropertyValuesHolders or
ObjectAnimators, the reflection lookup for getters and
setters is unnecessary.
Fixed problem in which static maps were being protected
by instance locks.
Fixed problem where we were repeatedly doing a
reflection lookup on methods that don't exist.
Change-Id: Ic0a1b62357f3aaaa4c900fef6087583ad0e964b6
diff --git a/core/java/android/animation/PropertyValuesHolder.java b/core/java/android/animation/PropertyValuesHolder.java
index 97426c3..bd7bca0 100644
--- a/core/java/android/animation/PropertyValuesHolder.java
+++ b/core/java/android/animation/PropertyValuesHolder.java
@@ -105,10 +105,6 @@
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 @@
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) {
- setterOrGetter = propertyMap.get(mPropertyName);
+ 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 @@
}
propertyMap.put(mPropertyName, setterOrGetter);
}
- } finally {
- mPropertyMapLock.writeLock().unlock();
}
return setterOrGetter;
}
@@ -811,30 +808,33 @@
mProperty = null;
}
}
- Class targetClass = target.getClass();
- if (mSetter == null) {
- setupSetter(targetClass);
- }
- List<Keyframe> keyframes = mKeyframes.getKeyframes();
- int keyframeCount = keyframes == null ? 0 : keyframes.size();
- for (int i = 0; i < keyframeCount; i++) {
- Keyframe kf = keyframes.get(i);
- if (!kf.hasValue() || kf.valueWasSetOnStart()) {
- if (mGetter == null) {
- setupGetter(targetClass);
+ // 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);
+ }
+ List<Keyframe> keyframes = mKeyframes.getKeyframes();
+ int keyframeCount = keyframes == null ? 0 : keyframes.size();
+ for (int i = 0; i < keyframeCount; i++) {
+ Keyframe kf = keyframes.get(i);
+ if (!kf.hasValue() || kf.valueWasSetOnStart()) {
if (mGetter == null) {
- // Already logged the error - just return to avoid NPE
- return;
+ setupGetter(targetClass);
+ if (mGetter == null) {
+ // Already logged the error - just return to avoid NPE
+ return;
+ }
}
- }
- try {
- Object value = convertBack(mGetter.invoke(target));
- kf.setValue(value);
- kf.setValueWasSetOnStart(true);
- } catch (InvocationTargetException e) {
- Log.e("PropertyValuesHolder", e.toString());
- } catch (IllegalAccessException e) {
- Log.e("PropertyValuesHolder", e.toString());
+ try {
+ Object value = convertBack(mGetter.invoke(target));
+ kf.setValue(value);
+ kf.setValueWasSetOnStart(true);
+ } catch (InvocationTargetException e) {
+ Log.e("PropertyValuesHolder", e.toString());
+ } catch (IllegalAccessException e) {
+ Log.e("PropertyValuesHolder", e.toString());
+ }
}
}
}
@@ -1178,32 +1178,33 @@
return;
}
// Check new static hashmap<propName, int> for setter method
- try {
- mPropertyMapLock.writeLock().lock();
+ synchronized(sJNISetterPropertyMap) {
HashMap<String, Long> propertyMap = sJNISetterPropertyMap.get(targetClass);
+ boolean wasInMap = false;
if (propertyMap != null) {
- Long jniSetter = propertyMap.get(mPropertyName);
- if (jniSetter != null) {
- mJniSetter = jniSetter;
- }
- }
- if (mJniSetter == 0) {
- String methodName = getMethodName("set", mPropertyName);
- mJniSetter = nGetIntMethod(targetClass, methodName);
- if (mJniSetter != 0) {
- if (propertyMap == null) {
- propertyMap = new HashMap<String, Long>();
- sJNISetterPropertyMap.put(targetClass, propertyMap);
+ wasInMap = propertyMap.containsKey(mPropertyName);
+ if (wasInMap) {
+ Long jniSetter = propertyMap.get(mPropertyName);
+ if (jniSetter != null) {
+ mJniSetter = jniSetter;
}
- 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 (!wasInMap) {
+ String methodName = getMethodName("set", mPropertyName);
+ try {
+ mJniSetter = nGetIntMethod(targetClass, methodName);
+ } 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);
+ }
+ propertyMap.put(mPropertyName, mJniSetter);
+ }
}
if (mJniSetter == 0) {
// Couldn't find method through fast JNI approach - just use reflection
@@ -1315,32 +1316,33 @@
return;
}
// Check new static hashmap<propName, int> for setter method
- try {
- mPropertyMapLock.writeLock().lock();
+ synchronized (sJNISetterPropertyMap) {
HashMap<String, Long> propertyMap = sJNISetterPropertyMap.get(targetClass);
+ boolean wasInMap = false;
if (propertyMap != null) {
- Long jniSetter = propertyMap.get(mPropertyName);
- if (jniSetter != null) {
- mJniSetter = jniSetter;
- }
- }
- if (mJniSetter == 0) {
- String methodName = getMethodName("set", mPropertyName);
- mJniSetter = nGetFloatMethod(targetClass, methodName);
- if (mJniSetter != 0) {
- if (propertyMap == null) {
- propertyMap = new HashMap<String, Long>();
- sJNISetterPropertyMap.put(targetClass, propertyMap);
+ wasInMap = propertyMap.containsKey(mPropertyName);
+ if (wasInMap) {
+ Long jniSetter = propertyMap.get(mPropertyName);
+ if (jniSetter != null) {
+ mJniSetter = jniSetter;
}
- 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 (!wasInMap) {
+ String methodName = getMethodName("set", mPropertyName);
+ try {
+ mJniSetter = nGetFloatMethod(targetClass, methodName);
+ } 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);
+ }
+ propertyMap.put(mPropertyName, mJniSetter);
+ }
}
if (mJniSetter == 0) {
// Couldn't find method through fast JNI approach - just use reflection
@@ -1419,16 +1421,19 @@
if (mJniSetter != 0) {
return;
}
- try {
- mPropertyMapLock.writeLock().lock();
+ synchronized(sJNISetterPropertyMap) {
HashMap<String, Long> propertyMap = sJNISetterPropertyMap.get(targetClass);
+ boolean wasInMap = false;
if (propertyMap != null) {
- Long jniSetterLong = propertyMap.get(mPropertyName);
- if (jniSetterLong != null) {
- mJniSetter = jniSetterLong;
+ 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);
calculateValue(0f);
float[] values = (float[]) getAnimatedValue();
@@ -1437,19 +1442,20 @@
mJniSetter = nGetMultipleFloatMethod(targetClass, methodName, numParams);
} catch (NoSuchMethodError e) {
// try without the 'set' prefix
- mJniSetter = nGetMultipleFloatMethod(targetClass, mPropertyName, numParams);
- }
- if (mJniSetter != 0) {
- if (propertyMap == null) {
- propertyMap = new HashMap<String, Long>();
- sJNISetterPropertyMap.put(targetClass, propertyMap);
+ try {
+ mJniSetter = nGetMultipleFloatMethod(targetClass, mPropertyName,
+ numParams);
+ } catch (NoSuchMethodError e2) {
+ // just try reflection next
}
- propertyMap.put(mPropertyName, mJniSetter);
}
+ if (propertyMap == null) {
+ propertyMap = new HashMap<String, Long>();
+ sJNISetterPropertyMap.put(targetClass, propertyMap);
+ }
+ propertyMap.put(mPropertyName, mJniSetter);
}
- } finally {
- mPropertyMapLock.writeLock().unlock();
- }
+ }
}
}
@@ -1522,16 +1528,19 @@
if (mJniSetter != 0) {
return;
}
- try {
- mPropertyMapLock.writeLock().lock();
+ synchronized(sJNISetterPropertyMap) {
HashMap<String, Long> propertyMap = sJNISetterPropertyMap.get(targetClass);
+ boolean wasInMap = false;
if (propertyMap != null) {
- Long jniSetterLong = propertyMap.get(mPropertyName);
- if (jniSetterLong != null) {
- mJniSetter = jniSetterLong;
+ 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);
calculateValue(0f);
int[] values = (int[]) getAnimatedValue();
@@ -1540,18 +1549,19 @@
mJniSetter = nGetMultipleIntMethod(targetClass, methodName, numParams);
} catch (NoSuchMethodError e) {
// try without the 'set' prefix
- mJniSetter = nGetMultipleIntMethod(targetClass, mPropertyName, numParams);
- }
- if (mJniSetter != 0) {
- if (propertyMap == null) {
- propertyMap = new HashMap<String, Long>();
- sJNISetterPropertyMap.put(targetClass, propertyMap);
+ try {
+ mJniSetter = nGetMultipleIntMethod(targetClass, mPropertyName,
+ numParams);
+ } catch (NoSuchMethodError e2) {
+ // couldn't find it.
}
- propertyMap.put(mPropertyName, mJniSetter);
}
+ if (propertyMap == null) {
+ propertyMap = new HashMap<String, Long>();
+ sJNISetterPropertyMap.put(targetClass, propertyMap);
+ }
+ propertyMap.put(mPropertyName, mJniSetter);
}
- } finally {
- mPropertyMapLock.writeLock().unlock();
}
}
}