Restructure ViewManagersPropertyCache.

Differential Revision: D2532365

fb-gh-sync-id: e5cbb55093310b47b6525839d7c5929264d158ad

ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java

@@ -35,7 +35,7 @@ public abstract class ViewManager<T extends View, C extends ReactShadowNode> {
 
   public final void updateProperties(T viewToUpdate, CatalystStylesDiffMap props) {
     Map<String, ViewManagersPropertyCache.PropSetter> propSetters =
-        ViewManagersPropertyCache.getNativePropSettersForClass(getClass());
+        ViewManagersPropertyCache.getNativePropSettersForViewManagerClass(getClass());
     ReadableMap propMap = props.mBackingMap;
     ReadableMapKeySeyIterator iterator = propMap.keySetIterator();
     // TODO(krzysztof): Remove missingSetters code once all views are migrated to @ReactProp
@@ -44,7 +44,7 @@ public abstract class ViewManager<T extends View, C extends ReactShadowNode> {
       String key = iterator.nextKey();
       ViewManagersPropertyCache.PropSetter setter = propSetters.get(key);
       if (setter != null) {
-        setter.updateProp(this, viewToUpdate, props);
+        setter.updateViewProp(this, viewToUpdate, props);
       } else {
         missingSetters = true;
       }
@@ -219,7 +219,7 @@ public abstract class ViewManager<T extends View, C extends ReactShadowNode> {
     // TODO(krzysztof): This method will just delegate to ViewManagersPropertyRegistry once
     // refactoring is finished
     Class cls = getClass();
-    Map<String, String> nativeProps = ViewManagersPropertyCache.getNativePropsForClass(cls);
+    Map<String, String> nativeProps = ViewManagersPropertyCache.getNativePropsForView(cls);
     while (cls.getSuperclass() != null) {
       Map<String, UIProp.Type> props = getNativePropsForClass(cls);
       for (Map.Entry<String, UIProp.Type> entry : props.entrySet()) {

ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagersPropertyCache.java

@@ -2,7 +2,8 @@
 
 package com.facebook.react.uimanager;
 
-import java.lang.reflect.InvocationTargetException;
+import javax.annotation.Nullable;
+
 import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.Map;
@@ -28,16 +29,20 @@ import com.facebook.react.bridge.ReadableMap;
     protected final String mPropName;
     protected final String mPropType;
     protected final Method mSetter;
+    protected final @Nullable Integer mIndex; /* non-null only for group setters */
+
+    // The following Object arrays are used to prevent extra allocations from varargs when we call
+    // Method.invoke. It's safe for those objects to be static as we update properties in a single
+    // thread sequentially
+    private static final Object[] VIEW_MGR_ARGS = new Object[2];
+    private static final Object[] VIEW_MGR_GROUP_ARGS = new Object[3];
 
-    // The following two constructors make it easy to reuse code responsible for setting property
-    // type. It's probably not a best design but this API is not exposed and since we can't use
-    // inheritance for annotation classes it's the easiest way to avoid creating an extra base class
-    // just to support group and non-group setters.
     private PropSetter(ReactProp prop, String defaultType, Method setter) {
       mPropName = prop.name();
       mPropType = ReactProp.USE_DEFAULT_TYPE.equals(prop.customType()) ?
           defaultType : prop.customType();
       mSetter = setter;
+      mIndex = null;
     }
 
     private PropSetter(ReactPropGroup prop, String defaultType, Method setter, int index) {
@@ -45,6 +50,7 @@ import com.facebook.react.bridge.ReadableMap;
       mPropType = ReactPropGroup.USE_DEFAULT_TYPE.equals(prop.customType()) ?
           defaultType : prop.customType();
       mSetter = setter;
+      mIndex = index;
     }
 
     public String getPropName() {
@@ -55,12 +61,21 @@ import com.facebook.react.bridge.ReadableMap;
       return mPropType;
     }
 
-    public void updateProp(
+    public void updateViewProp(
         ViewManager viewManager,
         View viewToUpdate,
         CatalystStylesDiffMap props) {
       try {
-        updateProperty(viewManager, viewToUpdate, props);
+        if (mIndex == null) {
+          VIEW_MGR_ARGS[0] = viewToUpdate;
+          VIEW_MGR_ARGS[1] = extractProperty(props);
+          mSetter.invoke(viewManager, VIEW_MGR_ARGS);
+        } else {
+          VIEW_MGR_GROUP_ARGS[0] = viewToUpdate;
+          VIEW_MGR_GROUP_ARGS[1] = mIndex;
+          VIEW_MGR_GROUP_ARGS[2] = extractProperty(props);
+          mSetter.invoke(viewManager, VIEW_MGR_GROUP_ARGS);
+        }
       } catch (Throwable t) {
         FLog.e(ViewManager.class, "Error while updating prop " + mPropName, t);
         throw new JSApplicationIllegalArgumentException("Error while updating property '" +
@@ -68,10 +83,7 @@ import com.facebook.react.bridge.ReadableMap;
       }
     }
 
-    protected abstract void updateProperty(
+    protected abstract @Nullable Object extractProperty(CatalystStylesDiffMap props);
-        ViewManager viewManager,
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException;
   }
 
   private static class IntPropSetter extends PropSetter {
@@ -83,12 +95,14 @@ import com.facebook.react.bridge.ReadableMap;
       mDefaultValue = defaultValue;
     }
 
+    public IntPropSetter(ReactPropGroup prop, Method setter, int index, int defaultValue) {
+      super(prop, "number", setter, index);
+      mDefaultValue = defaultValue;
+    }
+
     @Override
-    protected void updateProperty(
+    protected Object extractProperty(CatalystStylesDiffMap props) {
-        ViewManager viewManager,
+      return props.getInt(mPropName, mDefaultValue);
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      mSetter.invoke(viewManager, viewToUpdate, props.getInt(mPropName, mDefaultValue));
     }
   }
 
@@ -102,11 +116,8 @@ import com.facebook.react.bridge.ReadableMap;
     }
 
     @Override
-    protected void updateProperty(
+    protected Object extractProperty(CatalystStylesDiffMap props) {
-        ViewManager viewManager,
+      return props.getDouble(mPropName, mDefaultValue);
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      mSetter.invoke(viewManager, viewToUpdate, props.getDouble(mPropName, mDefaultValue));
     }
   }
 
@@ -120,11 +131,8 @@ import com.facebook.react.bridge.ReadableMap;
     }
 
     @Override
-    protected void updateProperty(
+    protected Object extractProperty(CatalystStylesDiffMap props) {
-        ViewManager viewManager,
+      return props.getBoolean(mPropName, mDefaultValue) ? Boolean.TRUE : Boolean.FALSE;
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      mSetter.invoke(viewManager, viewToUpdate, props.getBoolean(mPropName, mDefaultValue));
     }
   }
 
@@ -137,12 +145,14 @@ import com.facebook.react.bridge.ReadableMap;
       mDefaultValue = defaultValue;
     }
 
+    public FloatPropSetter(ReactPropGroup prop, Method setter, int index, float defaultValue) {
+      super(prop, "number", setter, index);
+      mDefaultValue = defaultValue;
+    }
+
     @Override
-    protected void updateProperty(
+    protected Object extractProperty(CatalystStylesDiffMap props) {
-        ViewManager viewManager,
+      return props.getFloat(mPropName, mDefaultValue);
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      mSetter.invoke(viewManager, viewToUpdate, props.getFloat(mPropName, mDefaultValue));
     }
   }
 
@@ -153,11 +163,8 @@ import com.facebook.react.bridge.ReadableMap;
     }
 
     @Override
-    protected void updateProperty(
+    protected @Nullable Object extractProperty(CatalystStylesDiffMap props) {
-        ViewManager viewManager,
+      return props.getArray(mPropName);
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      mSetter.invoke(viewManager, viewToUpdate, props.getArray(mPropName));
     }
   }
 
@@ -168,11 +175,8 @@ import com.facebook.react.bridge.ReadableMap;
     }
 
     @Override
-    protected void updateProperty(
+    protected @Nullable Object extractProperty(CatalystStylesDiffMap props) {
-        ViewManager viewManager,
+      return props.getMap(mPropName);
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      mSetter.invoke(viewManager, viewToUpdate, props.getMap(mPropName));
     }
   }
 
@@ -183,11 +187,8 @@ import com.facebook.react.bridge.ReadableMap;
     }
 
     @Override
-    protected void updateProperty(
+    protected @Nullable Object extractProperty(CatalystStylesDiffMap props) {
-        ViewManager viewManager,
+      return props.getString(mPropName);
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      mSetter.invoke(viewManager, viewToUpdate, props.getString(mPropName));
     }
   }
 
@@ -198,15 +199,11 @@ import com.facebook.react.bridge.ReadableMap;
     }
 
     @Override
-    protected void updateProperty(
+    protected @Nullable Object extractProperty(CatalystStylesDiffMap props) {
-        ViewManager viewManager,
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      Boolean value = null;
       if (!props.isNull(mPropName)) {
-        value = props.getBoolean(mPropName, false) ? Boolean.TRUE : Boolean.FALSE;
+        return props.getBoolean(mPropName, /* ignored */ false) ? Boolean.TRUE : Boolean.FALSE;
       }
-      mSetter.invoke(viewManager, viewToUpdate, value);
+      return null;
     }
   }
 
@@ -216,95 +213,38 @@ import com.facebook.react.bridge.ReadableMap;
       super(prop, "number", setter);
     }
 
+    public BoxedIntPropSetter(ReactPropGroup prop, Method setter, int index) {
+      super(prop, "number", setter, index);
+    }
+
     @Override
-    protected void updateProperty(
+    protected @Nullable Object extractProperty(CatalystStylesDiffMap props) {
-        ViewManager viewManager,
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      Integer value = null;
       if (!props.isNull(mPropName)) {
-        value = props.getInt(mPropName, /* ignored */ 0);
+        return props.getInt(mPropName, /* ignored */ 0);
       }
-      mSetter.invoke(viewManager, viewToUpdate, value);
+      return null;
     }
   }
 
-  private static abstract class GroupSetter extends PropSetter {
+  /*package*/ static Map<String, String> getNativePropsForView(
-
+      Class<? extends ViewManager> viewManagerTopClass) {
-    protected final int mIndex;
-
-    protected GroupSetter(ReactPropGroup prop, String defaultType, Method setter, int index) {
-      super(prop, defaultType, setter, index);
-      mIndex = index;
-    }
-  }
-
-  private static class GroupIntSetter extends GroupSetter {
-
-    private final int mDefaultValue;
-
-    public GroupIntSetter(ReactPropGroup prop, Method setter, int index, int defaultValue) {
-      super(prop, "number", setter, index);
-      mDefaultValue = defaultValue;
-    }
-
-    @Override
-    protected void updateProperty(
-        ViewManager viewManager,
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      mSetter.invoke(viewManager, viewToUpdate, mIndex, props.getInt(mPropName, mDefaultValue));
-    }
-  }
-
-  private static class GroupFloatSetter extends GroupSetter {
-
-    private final float mDefaultValue;
-
-    public GroupFloatSetter(ReactPropGroup prop, Method setter, int index, float defaultValue) {
-      super(prop, "number", setter, index);
-      mDefaultValue = defaultValue;
-    }
-
-    @Override
-    protected void updateProperty(
-        ViewManager viewManager,
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      mSetter.invoke(viewManager, viewToUpdate, mIndex, props.getFloat(mPropName, mDefaultValue));
-    }
-  }
-
-  private static class GroupBoxedIntSetter extends GroupSetter {
-
-    protected GroupBoxedIntSetter(ReactPropGroup prop, Method setter, int index) {
-      super(prop, "number", setter, index);
-    }
-
-    @Override
-    protected void updateProperty(
-        ViewManager viewManager,
-        View viewToUpdate,
-        CatalystStylesDiffMap props) throws InvocationTargetException, IllegalAccessException {
-      mSetter.invoke(
-          viewManager,
-          viewToUpdate,
-          mIndex,
-          props.isNull(mPropName) ? null : props.getInt(mPropName, /* unused */ 0));
-    }
-  }
-
-  /*package*/ static Map<String, String> getNativePropsForClass(
-      Class<? extends ViewManager> topLevelClass) {
     Map<String, String> nativeProps = new HashMap<>();
-    Map<String, PropSetter> props = getNativePropSettersForClass(topLevelClass);
+
-    for (PropSetter setter : props.values()) {
+    Map<String, PropSetter> viewManagerProps =
+        getNativePropSettersForViewManagerClass(viewManagerTopClass);
+    for (PropSetter setter : viewManagerProps.values()) {
       nativeProps.put(setter.getPropName(), setter.getPropType());
     }
+
     return nativeProps;
   }
 
-  /*package*/ static Map<String, PropSetter> getNativePropSettersForClass(
+  /**
+   * Returns map from property name to setter instances for all the property setters annotated with
+   * {@link ReactProp} in the given {@link ViewManager} plus all the setter declared by it's
+   * parent classes.
+   */
+  /*package*/ static Map<String, PropSetter> getNativePropSettersForViewManagerClass(
       Class<? extends ViewManager> cls) {
     if (cls == ViewManager.class) {
       return EMPTY_PROPS_MAP;
@@ -313,93 +253,111 @@ import com.facebook.react.bridge.ReadableMap;
     if (props != null) {
       return props;
     }
+    // This is to include all the setters from parent classes. Once calculated the result will be
+    // stored in CLASS_PROPS_CACHE so that we only scan for @ReactProp annotations once per class.
     props = new HashMap<>(
-        getNativePropSettersForClass((Class<? extends ViewManager>) cls.getSuperclass()));
+        getNativePropSettersForViewManagerClass(
-    for (Method method : cls.getDeclaredMethods()) {
+            (Class<? extends ViewManager>) cls.getSuperclass()));
-      {
+    extractPropSettersFromViewManagerClassDefinition(cls, props);
-        ReactProp annotation = method.getAnnotation(ReactProp.class);
-        if (annotation != null) {
-          Class<?>[] paramTypes = method.getParameterTypes();
-          if (paramTypes.length != 2) {
-            throw new RuntimeException("Wrong number of args for prop setter: " +
-                cls.getName() + "#" + method.getName());
-          }
-          if (!View.class.isAssignableFrom(paramTypes[0])) {
-            throw new RuntimeException("First param should be a view subclass to be updated: " +
-                cls.getName() + "#" + method.getName());
-          }
-          Class<?> propTypeClass = paramTypes[1];
-          PropSetter propSetter;
-          if (propTypeClass == boolean.class) {
-            propSetter =
-                new BooleanPropSetter(annotation, method, annotation.defaultBoolean());
-          } else if (propTypeClass == int.class) {
-            propSetter = new IntPropSetter(annotation, method, annotation.defaultInt());
-          } else if (propTypeClass == float.class) {
-            propSetter = new FloatPropSetter(annotation, method, annotation.defaultFloat());
-          } else if (propTypeClass == double.class) {
-            propSetter =
-                new DoublePropSetter(annotation, method, annotation.defaultDouble());
-          } else if (propTypeClass == String.class) {
-            propSetter = new StringPropSetter(annotation, method);
-          } else if (propTypeClass == Boolean.class) {
-            propSetter = new BoxedBooleanPropSetter(annotation, method);
-          } else if (propTypeClass == Integer.class) {
-            propSetter = new BoxedIntPropSetter(annotation, method);
-          } else if (propTypeClass == ReadableArray.class) {
-            propSetter = new ArrayPropSetter(annotation, method);
-          } else if (propTypeClass == ReadableMap.class) {
-            propSetter = new MapPropSetter(annotation, method);
-          } else {
-            throw new RuntimeException("Unrecognized type");
-          }
-          props.put(annotation.name(), propSetter);
-        }
-      }
-      {
-        ReactPropGroup annotation = method.getAnnotation(ReactPropGroup.class);
-        if (annotation != null) {
-          Class<?> [] paramTypes = method.getParameterTypes();
-          if (paramTypes.length != 3) {
-            throw new RuntimeException("Wrong number of args for group prop setter: " +
-                cls.getName() + "#" + method.getName());
-          }
-          if (!View.class.isAssignableFrom(paramTypes[0])) {
-            throw new RuntimeException("First param should be a view subclass to be updated: " +
-                cls.getName() + "#" + method.getName());
-          }
-          if (paramTypes[1] != int.class) {
-            throw new RuntimeException("Second argument should be property index: " +
-                cls.getName() + "#" + method.getName());
-          }
-          Class<?> propTypeClass = paramTypes[2];
-          String[] names = annotation.names();
-          if (propTypeClass == int.class) {
-            for (int i = 0; i < names.length; i++) {
-              props.put(
-                  names[i],
-                  new GroupIntSetter(annotation, method, i, annotation.defaultInt()));
-            }
-          } else if (propTypeClass == float.class) {
-            for (int i = 0; i < names.length; i++) {
-              props.put(
-                  names[i],
-                  new GroupFloatSetter(annotation, method, i, annotation.defaultFloat()));
-            }
-          } else if (propTypeClass == Integer.class) {
-            for (int i = 0; i < names.length; i++) {
-              props.put(
-                  names[i],
-                  new GroupBoxedIntSetter(annotation, method, i));
-            }
-          } else {
-            throw new RuntimeException("Unrecognized type: " + paramTypes[2] + " for method: " +
-                cls.getName() + "#" + method.getName());
-          }
-        }
-      }
-    }
     CLASS_PROPS_CACHE.put(cls, props);
     return props;
   }
+
+  private static PropSetter createPropSetter(
+      ReactProp annotation,
+      Method method,
+      Class<?> propTypeClass) {
+    if (propTypeClass == boolean.class) {
+      return new BooleanPropSetter(annotation, method, annotation.defaultBoolean());
+    } else if (propTypeClass == int.class) {
+      return new IntPropSetter(annotation, method, annotation.defaultInt());
+    } else if (propTypeClass == float.class) {
+      return new FloatPropSetter(annotation, method, annotation.defaultFloat());
+    } else if (propTypeClass == double.class) {
+      return new DoublePropSetter(annotation, method, annotation.defaultDouble());
+    } else if (propTypeClass == String.class) {
+      return new StringPropSetter(annotation, method);
+    } else if (propTypeClass == Boolean.class) {
+      return new BoxedBooleanPropSetter(annotation, method);
+    } else if (propTypeClass == Integer.class) {
+      return new BoxedIntPropSetter(annotation, method);
+    } else if (propTypeClass == ReadableArray.class) {
+      return new ArrayPropSetter(annotation, method);
+    } else if (propTypeClass == ReadableMap.class) {
+      return new MapPropSetter(annotation, method);
+    } else {
+      throw new RuntimeException("Unrecognized type: " + propTypeClass + " for method: " +
+          method.getDeclaringClass().getName() + "#" + method.getName());
+    }
+  }
+
+  private static void createPropSetters(
+      ReactPropGroup annotation,
+      Method method,
+      Class<?> propTypeClass,
+      Map<String, PropSetter> props) {
+    String[] names = annotation.names();
+    if (propTypeClass == int.class) {
+      for (int i = 0; i < names.length; i++) {
+        props.put(
+            names[i],
+            new IntPropSetter(annotation, method, i, annotation.defaultInt()));
+      }
+    } else if (propTypeClass == float.class) {
+      for (int i = 0; i < names.length; i++) {
+        props.put(
+            names[i],
+            new FloatPropSetter(annotation, method, i, annotation.defaultFloat()));
+      }
+    } else if (propTypeClass == Integer.class) {
+      for (int i = 0; i < names.length; i++) {
+        props.put(
+            names[i],
+            new BoxedIntPropSetter(annotation, method, i));
+      }
+    } else {
+      throw new RuntimeException("Unrecognized type: " + propTypeClass + " for method: " +
+          method.getDeclaringClass().getName() + "#" + method.getName());
+    }
+  }
+
+  private static void extractPropSettersFromViewManagerClassDefinition(
+      Class<? extends ViewManager> cls,
+      Map<String, PropSetter> props) {
+    Method[] declaredMethods = cls.getDeclaredMethods();
+    for (int i = 0; i < declaredMethods.length; i++) {
+      Method method = declaredMethods[i];
+      ReactProp annotation = method.getAnnotation(ReactProp.class);
+      if (annotation != null) {
+        Class<?>[] paramTypes = method.getParameterTypes();
+        if (paramTypes.length != 2) {
+          throw new RuntimeException("Wrong number of args for prop setter: " +
+              cls.getName() + "#" + method.getName());
+        }
+        if (!View.class.isAssignableFrom(paramTypes[0])) {
+          throw new RuntimeException("First param should be a view subclass to be updated: " +
+              cls.getName() + "#" + method.getName());
+        }
+        props.put(annotation.name(), createPropSetter(annotation, method, paramTypes[1]));
+      }
+
+      ReactPropGroup groupAnnotation = method.getAnnotation(ReactPropGroup.class);
+      if (groupAnnotation != null) {
+        Class<?> [] paramTypes = method.getParameterTypes();
+        if (paramTypes.length != 3) {
+          throw new RuntimeException("Wrong number of args for group prop setter: " +
+              cls.getName() + "#" + method.getName());
+        }
+        if (!View.class.isAssignableFrom(paramTypes[0])) {
+          throw new RuntimeException("First param should be a view subclass to be updated: " +
+              cls.getName() + "#" + method.getName());
+        }
+        if (paramTypes[1] != int.class) {
+          throw new RuntimeException("Second argument should be property index: " +
+              cls.getName() + "#" + method.getName());
+        }
+        createPropSetters(groupAnnotation, method, paramTypes[2], props);
+      }
+    }
+  }
 }