Adding JNI methods as a faster reflection mechanism

This approach is only for the common cases of void-return,
single-argument float/int methods.

Change-Id: Ifb31535a6f717b85417eced93c579be6e461e039

6 files changed, 227 insertions, 32 deletions

diff --git a/core/java/android/animation/ObjectAnimator.java b/core/java/android/animation/ObjectAnimator.java
index c898ae3..7c2e70d 100644
--- a/core/java/android/animation/ObjectAnimator.java
+++ b/core/java/android/animation/ObjectAnimator.java
@@ -26,6 +26,9 @@ import java.lang.reflect.Method;
  * as well as the name of the property that will be animated. Appropriate set/get functions
  * are then determined internally and the animation will call these functions as necessary to
  * animate the property.
+ *
+ * @see #setPropertyName(String)
+ *
  */
 public final class ObjectAnimator extends ValueAnimator {
 
@@ -42,6 +45,13 @@ public final class ObjectAnimator extends ValueAnimator {
      * <code>valueFrom</code> or <code>valueTo</code> is null, then a getter function will
      * also be derived and called.
      *
+     * <p>For best performance of the mechanism that calls the setter function determined by the
+     * name of the property being animated, use <code>float</code> or <code>int</code> typed values,
+     * and make the setter function for those properties have a <code>void</code> return value. This
+     * will cause the code to take an optimized path for these constrained circumstances. Other
+     * property types and return types will work, but will have more overhead in processing
+     * the requests due to normal reflection mechanisms.</p>
+     *
      * <p>Note that the setter function derived from this property name
      * must take the same parameter type as the
      * <code>valueFrom</code> and <code>valueTo</code> properties, otherwise the call to
diff --git a/core/java/android/animation/PropertyValuesHolder.java b/core/java/android/animation/PropertyValuesHolder.java
index 1945b05..97aa5a1 100644
--- a/core/java/android/animation/PropertyValuesHolder.java
+++ b/core/java/android/animation/PropertyValuesHolder.java
@@ -17,12 +17,9 @@
 package android.animation;
 
 import android.util.Log;
-import android.animation.Keyframe.IntKeyframe;
-import android.animation.Keyframe.FloatKeyframe;
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
@@ -39,7 +36,7 @@ public class PropertyValuesHolder implements Cloneable {
      * unless this object is being used with ObjectAnimator. But this is the name by which
      * aniamted values are looked up with getAnimatedValue(String) in ValueAnimator.
      */
-    private String mPropertyName;
+    String mPropertyName;
 
     /**
      * The setter function, if needed. ObjectAnimator hands off this functionality to
@@ -99,10 +96,10 @@ public class PropertyValuesHolder implements Cloneable {
 
     // This lock is used to ensure that only one thread is accessing the property maps
     // at a time.
-    private ReentrantReadWriteLock propertyMapLock = new ReentrantReadWriteLock();
+    final ReentrantReadWriteLock mPropertyMapLock = new ReentrantReadWriteLock();
 
     // Used to pass single value to varargs parameter in setter invocation
-    Object[] mTmpValueArray = new Object[1];
+    final Object[] mTmpValueArray = new Object[1];
 
     /**
      * The type evaluator used to calculate the animated values. This evaluator is determined
@@ -296,10 +293,7 @@ public class PropertyValuesHolder implements Cloneable {
     private Method getPropertyFunction(Class targetClass, String prefix, Class valueType) {
         // TODO: faster implementation...
         Method returnVal = null;
-        String firstLetter = mPropertyName.substring(0, 1);
-        String theRest = mPropertyName.substring(1);
-        firstLetter = firstLetter.toUpperCase();
-        String methodName = prefix + firstLetter + theRest;
+        String methodName = getMethodName(prefix, mPropertyName);
         Class args[] = null;
         if (valueType == null) {
             try {
@@ -361,7 +355,7 @@ public class PropertyValuesHolder implements Cloneable {
             // another thread putting something in there after we've checked it
             // but before we've added an entry to it
             // TODO: can we store the setter/getter per Class instead of per Object?
-            propertyMapLock.writeLock().lock();
+            mPropertyMapLock.writeLock().lock();
             HashMap<String, Method> propertyMap = propertyMapMap.get(targetClass);
             if (propertyMap != null) {
                 setterOrGetter = propertyMap.get(mPropertyName);
@@ -375,7 +369,7 @@ public class PropertyValuesHolder implements Cloneable {
                 propertyMap.put(mPropertyName, setterOrGetter);
             }
         } finally {
-            propertyMapLock.writeLock().unlock();
+            mPropertyMapLock.writeLock().unlock();
         }
         return setterOrGetter;
     }
@@ -384,7 +378,7 @@ public class PropertyValuesHolder implements Cloneable {
      * Utility function to get the setter from targetClass
      * @param targetClass The Class on which the requested method should exist.
      */
-    private void setupSetter(Class targetClass) {
+    void setupSetter(Class targetClass) {
         mSetter = setupSetterOrGetter(targetClass, sSetterPropertyMap, "set", mValueType);
     }
 
@@ -650,8 +644,32 @@ public class PropertyValuesHolder implements Cloneable {
         return mAnimatedValue;
     }
 
+    /**
+     * Utility method to derive a setter/getter method name from a property name, where the
+     * prefix is typically "set" or "get" and the first letter of the property name is
+     * capitalized.
+     *
+     * @param prefix The precursor to the method name, before the property name begins, typically
+     * "set" or "get".
+     * @param propertyName The name of the property that represents the bulk of the method name
+     * after the prefix. The first letter of this word will be capitalized in the resulting
+     * method name.
+     * @return String the property name converted to a method name according to the conventions
+     * specified above.
+     */
+    static String getMethodName(String prefix, String propertyName) {
+        char firstLetter = propertyName.charAt(0);
+        String theRest = propertyName.substring(1);
+        firstLetter = Character.toUpperCase(firstLetter);
+        return prefix + firstLetter + theRest;
+    }
+
     static class IntPropertyValuesHolder extends PropertyValuesHolder {
 
+        private static final HashMap<Class, HashMap<String, Integer>> sJNISetterPropertyMap =
+                new HashMap<Class, HashMap<String, Integer>>();
+        int mJniSetter;
+
         IntKeyframeSet mIntKeyframeSet;
         int mIntAnimatedValue;
 
@@ -699,6 +717,10 @@ public class PropertyValuesHolder implements Cloneable {
          */
         @Override
         void setAnimatedValue(Object target) {
+            if (mJniSetter != 0) {
+                nCallIntMethod(target, mJniSetter, mIntAnimatedValue);
+                return;
+            }
             if (mSetter != null) {
                 try {
                     mTmpValueArray[0] = mIntAnimatedValue;
@@ -710,10 +732,48 @@ public class PropertyValuesHolder implements Cloneable {
                 }
             }
         }
+
+        @Override
+        void setupSetter(Class targetClass) {
+            // Check new static hashmap<propName, int> for setter method
+            try {
+                mPropertyMapLock.writeLock().lock();
+                HashMap<String, Integer> propertyMap = sJNISetterPropertyMap.get(targetClass);
+                if (propertyMap != null) {
+                    Integer mJniSetterInteger = propertyMap.get(mPropertyName);
+                    if (mJniSetterInteger != null) {
+                        mJniSetter = mJniSetterInteger;
+                    }
+                }
+                if (mJniSetter == 0) {
+                    String methodName = getMethodName("set", mPropertyName);
+                    mJniSetter = nGetIntMethod(targetClass, methodName);
+                    if (mJniSetter != 0) {
+                        if (propertyMap == null) {
+                            propertyMap = new HashMap<String, Integer>();
+                            sJNISetterPropertyMap.put(targetClass, propertyMap);
+                        }
+                        propertyMap.put(mPropertyName, mJniSetter);
+                    }
+                }
+            } catch (NoSuchMethodError e) {
+                // System.out.println("Can't find native method using JNI, use reflection" + e);
+            } finally {
+                mPropertyMapLock.writeLock().unlock();
+            }
+            if (mJniSetter == 0) {
+                // Couldn't find method through fast JNI approach - just use reflection
+                super.setupSetter(targetClass);
+            }
+        }
     }
 
     static class FloatPropertyValuesHolder extends PropertyValuesHolder {
 
+        private static final HashMap<Class, HashMap<String, Integer>> sJNISetterPropertyMap =
+                new HashMap<Class, HashMap<String, Integer>>();
+        int mJniSetter;
+
         FloatKeyframeSet mFloatKeyframeSet;
         float mFloatAnimatedValue;
 
@@ -761,6 +821,10 @@ public class PropertyValuesHolder implements Cloneable {
          */
         @Override
         void setAnimatedValue(Object target) {
+            if (mJniSetter != 0) {
+                nCallFloatMethod(target, mJniSetter, mFloatAnimatedValue);
+                return;
+            }
             if (mSetter != null) {
                 try {
                     mTmpValueArray[0] = mFloatAnimatedValue;
@@ -773,5 +837,44 @@ public class PropertyValuesHolder implements Cloneable {
             }
         }
 
+        @Override
+        void setupSetter(Class targetClass) {
+            // Check new static hashmap<propName, int> for setter method
+            try {
+                mPropertyMapLock.writeLock().lock();
+                HashMap<String, Integer> propertyMap = sJNISetterPropertyMap.get(targetClass);
+                if (propertyMap != null) {
+                    Integer mJniSetterInteger = propertyMap.get(mPropertyName);
+                    if (mJniSetterInteger != null) {
+                        mJniSetter = mJniSetterInteger;
+                    }
+                }
+                if (mJniSetter == 0) {
+                    String methodName = getMethodName("set", mPropertyName);
+                    mJniSetter = nGetFloatMethod(targetClass, methodName);
+                    if (mJniSetter != 0) {
+                        if (propertyMap == null) {
+                            propertyMap = new HashMap<String, Integer>();
+                            sJNISetterPropertyMap.put(targetClass, propertyMap);
+                        }
+                        propertyMap.put(mPropertyName, mJniSetter);
+                    }
+                }
+            } catch (NoSuchMethodError e) {
+                // System.out.println("Can't find native method using JNI, use reflection" + e);
+            } finally {
+                mPropertyMapLock.writeLock().unlock();
+            }
+            if (mJniSetter == 0) {
+                // Couldn't find method through fast JNI approach - just use reflection
+                super.setupSetter(targetClass);
+            }
+        }
+
     }
+
+    native static private int nGetIntMethod(Class targetClass, String methodName);
+    native static private int nGetFloatMethod(Class targetClass, String methodName);
+    native static private void nCallIntMethod(Object target, int methodID, int arg);
+    native static private void nCallFloatMethod(Object target, int methodID, float arg);
 }
\ No newline at end of file
diff --git a/core/java/android/view/ViewTreeObserver.java b/core/java/android/view/ViewTreeObserver.java
index 26e5cbc..06a0fa6 100644
--- a/core/java/android/view/ViewTreeObserver.java
+++ b/core/java/android/view/ViewTreeObserver.java
@@ -18,6 +18,7 @@ package android.view;
 
 import android.graphics.Rect;
 
+import java.util.ArrayList;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
@@ -32,10 +33,10 @@ import java.util.concurrent.CopyOnWriteArrayList;
 public final class ViewTreeObserver {
     private CopyOnWriteArrayList<OnGlobalFocusChangeListener> mOnGlobalFocusListeners;
     private CopyOnWriteArrayList<OnGlobalLayoutListener> mOnGlobalLayoutListeners;
-    private CopyOnWriteArrayList<OnPreDrawListener> mOnPreDrawListeners;
     private CopyOnWriteArrayList<OnTouchModeChangeListener> mOnTouchModeChangeListeners;
     private CopyOnWriteArrayList<OnComputeInternalInsetsListener> mOnComputeInternalInsetsListeners;
     private CopyOnWriteArrayList<OnScrollChangedListener> mOnScrollChangedListeners;
+    private ArrayList<OnPreDrawListener> mOnPreDrawListeners;
 
     private boolean mAlive = true;
 
@@ -354,7 +355,7 @@ public final class ViewTreeObserver {
         checkIsAlive();
 
         if (mOnPreDrawListeners == null) {
-            mOnPreDrawListeners = new CopyOnWriteArrayList<OnPreDrawListener>();
+            mOnPreDrawListeners = new ArrayList<OnPreDrawListener>();
         }
 
         mOnPreDrawListeners.add(listener);
@@ -526,7 +527,7 @@ public final class ViewTreeObserver {
         // could mutate the list by calling the various add/remove methods. This prevents
         // the array from being modified while we iterate it.
         final CopyOnWriteArrayList<OnGlobalFocusChangeListener> listeners = mOnGlobalFocusListeners;
-        if (listeners != null) {
+        if (listeners != null && listeners.size() > 0) {
             for (OnGlobalFocusChangeListener listener : listeners) {
                 listener.onGlobalFocusChanged(oldFocus, newFocus);
             }
@@ -544,7 +545,7 @@ public final class ViewTreeObserver {
         // could mutate the list by calling the various add/remove methods. This prevents
         // the array from being modified while we iterate it.
         final CopyOnWriteArrayList<OnGlobalLayoutListener> listeners = mOnGlobalLayoutListeners;
-        if (listeners != null) {
+        if (listeners != null && listeners.size() > 0) {
             for (OnGlobalLayoutListener listener : listeners) {
                 listener.onGlobalLayout();
             }
@@ -560,15 +561,17 @@ public final class ViewTreeObserver {
      * @return True if the current draw should be canceled and resceduled, false otherwise.
      */
     public final boolean dispatchOnPreDraw() {
-        // NOTE: because of the use of CopyOnWriteArrayList, we *must* use an iterator to
-        // perform the dispatching. The iterator is a safe guard against listeners that
+        // NOTE: we *must* clone the listener list to perform the dispatching.
+        // The clone is a safe guard against listeners that
         // could mutate the list by calling the various add/remove methods. This prevents
-        // the array from being modified while we iterate it.
+        // the array from being modified while we process it.
         boolean cancelDraw = false;
-        final CopyOnWriteArrayList<OnPreDrawListener> listeners = mOnPreDrawListeners;
-        if (listeners != null) {
-            for (OnPreDrawListener listener : listeners) {
-                cancelDraw |= !listener.onPreDraw();
+        if (mOnPreDrawListeners != null && mOnPreDrawListeners.size() > 0) {
+            final ArrayList<OnPreDrawListener> listeners =
+                    (ArrayList<OnPreDrawListener>) mOnPreDrawListeners.clone();
+            int numListeners = listeners.size();
+            for (int i = 0; i < numListeners; ++i) {
+                cancelDraw |= !(listeners.get(i).onPreDraw());
             }
         }
         return cancelDraw;
@@ -580,13 +583,9 @@ public final class ViewTreeObserver {
      * @param inTouchMode True if the touch mode is now enabled, false otherwise.
      */
     final void dispatchOnTouchModeChanged(boolean inTouchMode) {
-        // NOTE: because of the use of CopyOnWriteArrayList, we *must* use an iterator to
-        // perform the dispatching. The iterator is a safe guard against listeners that
-        // could mutate the list by calling the various add/remove methods. This prevents
-        // the array from being modified while we iterate it.
         final CopyOnWriteArrayList<OnTouchModeChangeListener> listeners =
                 mOnTouchModeChangeListeners;
-        if (listeners != null) {
+        if (listeners != null && listeners.size() > 0) {
             for (OnTouchModeChangeListener listener : listeners) {
                 listener.onTouchModeChanged(inTouchMode);
             }
@@ -602,7 +601,7 @@ public final class ViewTreeObserver {
         // could mutate the list by calling the various add/remove methods. This prevents
         // the array from being modified while we iterate it.
         final CopyOnWriteArrayList<OnScrollChangedListener> listeners = mOnScrollChangedListeners;
-        if (listeners != null) {
+        if (listeners != null && listeners.size() > 0) {
             for (OnScrollChangedListener listener : listeners) {
                 listener.onScrollChanged();
             }
@@ -628,7 +627,7 @@ public final class ViewTreeObserver {
         // the array from being modified while we iterate it.
         final CopyOnWriteArrayList<OnComputeInternalInsetsListener> listeners =
                 mOnComputeInternalInsetsListeners;
-        if (listeners != null) {
+        if (listeners != null && listeners.size() > 0) {
             for (OnComputeInternalInsetsListener listener : listeners) {
                 listener.onComputeInternalInsets(inoutInfo);
             }
diff --git a/core/jni/Android.mk b/core/jni/Android.mk
index 755f694..bcd7bae 100644
--- a/core/jni/Android.mk
+++ b/core/jni/Android.mk
@@ -144,7 +144,8 @@ LOCAL_SRC_FILES:= \
 	android_backup_FileBackupHelperBase.cpp \
 	android_backup_BackupHelperDispatcher.cpp \
 	android_content_res_ObbScanner.cpp \
-    android_content_res_Configuration.cpp
+	android_content_res_Configuration.cpp \
+    android_animation_PropertyValuesHolder.cpp
 
 LOCAL_C_INCLUDES += \
 	$(JNI_H_INCLUDE) \
diff --git a/core/jni/AndroidRuntime.cpp b/core/jni/AndroidRuntime.cpp
index c38e001..1018ddb 100644
--- a/core/jni/AndroidRuntime.cpp
+++ b/core/jni/AndroidRuntime.cpp
@@ -171,6 +171,7 @@ extern int register_android_view_KeyEvent(JNIEnv* env);
 extern int register_android_view_MotionEvent(JNIEnv* env);
 extern int register_android_content_res_ObbScanner(JNIEnv* env);
 extern int register_android_content_res_Configuration(JNIEnv* env);
+extern int register_android_animation_PropertyValuesHolder(JNIEnv *env);
 
 static AndroidRuntime* gCurRuntime = NULL;
 
@@ -1291,6 +1292,8 @@ static const RegJNIRec gRegJNI[] = {
 
     REG_JNI(register_android_content_res_ObbScanner),
     REG_JNI(register_android_content_res_Configuration),
+
+    REG_JNI(register_android_animation_PropertyValuesHolder),
 };
 
 /*
diff --git a/core/jni/android_animation_PropertyValuesHolder.cpp b/core/jni/android_animation_PropertyValuesHolder.cpp
new file mode 100644
index 0000000..5991805
--- /dev/null
+++ b/core/jni/android_animation_PropertyValuesHolder.cpp
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <stdio.h>
+#include <assert.h>
+
+#include "jni.h"
+#include <android_runtime/AndroidRuntime.h>
+#include <utils/misc.h>
+
+// ----------------------------------------------------------------------------
+
+namespace android {
+
+// ----------------------------------------------------------------------------
+
+const char* const kClassPathName = "android/animation/PropertyValuesHolder";
+
+static jmethodID android_animation_PropertyValuesHolder_getIntMethod(
+        JNIEnv* env, jclass pvhClass, jclass targetClass, jstring methodName)
+{
+    const char *nativeString = env->GetStringUTFChars(methodName, 0);
+    jmethodID mid = env->GetMethodID(targetClass, nativeString, "(I)V");
+    env->ReleaseStringUTFChars(methodName, nativeString);
+    return mid;
+}
+
+static jmethodID android_animation_PropertyValuesHolder_getFloatMethod(
+        JNIEnv* env, jclass pvhClass, jclass targetClass, jstring methodName)
+{
+    const char *nativeString = env->GetStringUTFChars(methodName, 0);
+    jmethodID mid = env->GetMethodID(targetClass, nativeString, "(F)V");
+    env->ReleaseStringUTFChars(methodName, nativeString);
+    return mid;
+}
+
+static void android_animation_PropertyValuesHolder_callIntMethod(
+        JNIEnv* env, jclass pvhObject, jobject target, jmethodID methodID, int arg)
+{
+    env->CallVoidMethod(target, methodID, arg);
+}
+
+static void android_animation_PropertyValuesHolder_callFloatMethod(
+        JNIEnv* env, jclass pvhObject, jobject target, jmethodID methodID, float arg)
+{
+    env->CallVoidMethod(target, methodID, arg);
+}
+
+static JNINativeMethod gMethods[] = {
+    {   "nGetIntMethod", "(Ljava/lang/Class;Ljava/lang/String;)I",
+            (void*)android_animation_PropertyValuesHolder_getIntMethod },
+    {   "nGetFloatMethod", "(Ljava/lang/Class;Ljava/lang/String;)I",
+            (void*)android_animation_PropertyValuesHolder_getFloatMethod },
+    {   "nCallIntMethod", "(Ljava/lang/Object;II)V",
+            (void*)android_animation_PropertyValuesHolder_callIntMethod },
+    {   "nCallFloatMethod", "(Ljava/lang/Object;IF)V",
+            (void*)android_animation_PropertyValuesHolder_callFloatMethod }
+};
+
+int register_android_animation_PropertyValuesHolder(JNIEnv* env)
+{
+    return AndroidRuntime::registerNativeMethods(env,
+            kClassPathName, gMethods, NELEM(gMethods));
+}
+
+};