Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 691487d2 authored by George Mount's avatar George Mount
Browse files

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
parent 7d3d5feb

core/java/android/animation/PropertyValuesHolder.java

+111 −101
Original line number Diff line number Diff line
@@ -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 {

                mPropertyMapLock.writeLock().lock();

            synchronized(sJNISetterPropertyMap) {

                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 {

                mPropertyMapLock.writeLock().lock();

            synchronized (sJNISetterPropertyMap) {

                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 {

                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,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 {

                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,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();

            }

        }

    }