Generate isEquivalentTo for MountSpec and SectionSpecs. Otherwise use…

… reflective one Summary: After running the appropriate experiment, we concluded that it's still worth generating isEquivalentTo() for MountSpec and SectionSpecs for perf reasons. Instead, for every other Specs, we can rely on the reflective method ComponentUtils.hasEquivalentFields(). Reviewed By: muraziz Differential Revision: D13415220 fbshipit-source-id: 044a039ad1f5156b209751946dc8ba9fe6267cbe
facebook · Dec 12, 2018 · 2e27d99 · 2e27d99
1 parent 4465b3c
commit 2e27d99
Show file tree

Hide file tree

Showing 21 changed files with 83 additions and 176 deletions.
diff --git a/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.java b/litho-core/src/main/java/com/facebook/litho/config/ComponentsConfiguration.java
@@ -163,16 +163,6 @@ public class ComponentsConfiguration {
 
   public static boolean prewarmImageTexture = false;
 
-  public static boolean useNewIsEquivalentTo = false;
-
-  public static boolean useNewIsEquivalentToInLayoutSpec = false;
-
-  public static boolean useNewIsEquivalentToInMountSpec = false;
-
-  public static boolean useNewIsEquivalentToInSectionSpec = false;
-
-  public static boolean useNewIsEquivalentToInOtherSpec = false;
-
   /** Whether we should use the PlaceholderComponent instead of Column as MountSpec holder. */
   public static boolean usePlaceholderComponent = false;
 

diff --git a/...-it/src/test/java/com/facebook/litho/specmodels/generator/ComponentBodyGeneratorTest.java b/...-it/src/test/java/com/facebook/litho/specmodels/generator/ComponentBodyGeneratorTest.java
@@ -25,6 +25,8 @@
 import com.facebook.litho.ComponentContext;
 import com.facebook.litho.Row;
 import com.facebook.litho.annotations.LayoutSpec;
+import com.facebook.litho.annotations.MountSpec;
+import com.facebook.litho.annotations.OnBind;
 import com.facebook.litho.annotations.OnCreateLayout;
 import com.facebook.litho.annotations.OnEvent;
 import com.facebook.litho.annotations.OnUpdateState;
@@ -46,6 +48,7 @@
 import com.facebook.litho.specmodels.model.TypeSpec;
 import com.facebook.litho.specmodels.model.TypeSpec.DeclaredTypeSpec;
 import com.facebook.litho.specmodels.processor.LayoutSpecModelFactory;
+import com.facebook.litho.specmodels.processor.MountSpecModelFactory;
 import com.google.testing.compile.CompilationRule;
 import com.squareup.javapoet.ClassName;
 import java.util.List;
@@ -67,6 +70,7 @@ public class ComponentBodyGeneratorTest {
   @Mock private Messager mMessager;
 
   private final LayoutSpecModelFactory mLayoutSpecModelFactory = new LayoutSpecModelFactory();
+  private final MountSpecModelFactory mMountSpecModelFactory = new MountSpecModelFactory();
 
   @LayoutSpec
   static class TestSpec {
@@ -96,6 +100,34 @@ public void testEventMethod(
     public void testUpdateStateMethod() {}
   }
 
+  @MountSpec
+  static class MountTestSpec {
+    @PropDefault protected static boolean arg0 = true;
+
+    @OnBind
+    public void testDelegateMethod(
+        @Prop boolean arg0,
+        @Prop @Nullable Component arg4,
+        @Prop List<Component> arg5,
+        @Prop List<String> arg6,
+        @State int arg1,
+        @Param Object arg2,
+        @TreeProp long arg3,
+        @TreeProp Set<List<Row>> arg7,
+        @TreeProp Set<Integer> arg8) {}
+
+    @OnEvent(Object.class)
+    public void testEventMethod(
+        @Prop boolean arg0,
+        @Prop @Nullable Component arg4,
+        @State int arg1,
+        @Param Object arg2,
+        @TreeProp long arg3) {}
+
+    @OnUpdateState
+    public void testUpdateStateMethod() {}
+  }
+
   @LayoutSpec
   static class TestWithTransitionSpec {
     @PropDefault protected static boolean arg0 = true;
@@ -132,6 +164,7 @@ public final Component onCreateLayout(
   }
 
   private SpecModel mSpecModelDI;
+  private SpecModel mMountSpecModelDI;
   private SpecModel mSpecModelWithTransitionDI;
   private SpecModel mKotlinWildcardsSpecModel;
 
@@ -144,6 +177,17 @@ public void setUp() {
     mSpecModelDI =
         mLayoutSpecModelFactory.create(
             elements, types, typeElement, mMessager, RunMode.NORMAL, null, null);
+    // Here we are using the TestSpec that is declared as LayoutSpec but, because using
+    // the MountSpecModelFactory, is it going to be used as MountSpec anyway.
+    mMountSpecModelDI =
+        mMountSpecModelFactory.create(
+            elements,
+            types,
+            elements.getTypeElement(MountTestSpec.class.getCanonicalName()),
+            mMessager,
+            RunMode.NORMAL,
+            null,
+            null);
 
     TypeElement typeElementWithTransition =
         elements.getTypeElement(TestWithTransitionSpec.class.getCanonicalName());
@@ -309,58 +353,55 @@ public void testGenerateEventDeclarations() {
 
   @Test
   public void testGenerateIsEquivalentMethod() {
-    assertThat(ComponentBodyGenerator.generateIsEquivalentMethod(mSpecModelDI).toString())
+    assertThat(ComponentBodyGenerator.generateIsEquivalentMethod(mMountSpecModelDI).toString())
         .isEqualTo(
             "@java.lang.Override\n"
                 + "public boolean isEquivalentTo(com.facebook.litho.Component other) {\n"
-                + "  if (com.facebook.litho.config.ComponentsConfiguration.useNewIsEquivalentToInLayoutSpec) {\n"
-                + "    return super.isEquivalentTo(other);\n"
-                + "  }\n"
                 + "  if (this == other) {\n"
                 + "    return true;\n"
                 + "  }\n"
                 + "  if (other == null || getClass() != other.getClass()) {\n"
                 + "    return false;\n"
                 + "  }\n"
-                + "  Test testRef = (Test) other;\n"
-                + "  if (this.getId() == testRef.getId()) {\n"
+                + "  MountTest mountTestRef = (MountTest) other;\n"
+                + "  if (this.getId() == mountTestRef.getId()) {\n"
                 + "    return true;\n"
                 + "  }\n"
-                + "  if (arg0 != testRef.arg0) {\n"
+                + "  if (arg0 != mountTestRef.arg0) {\n"
                 + "    return false;\n"
                 + "  }\n"
-                + "  if (arg4 != null ? !arg4.isEquivalentTo(testRef.arg4) : testRef.arg4 != null) {\n"
+                + "  if (arg4 != null ? !arg4.isEquivalentTo(mountTestRef.arg4) : mountTestRef.arg4 != null) {\n"
                 + "    return false;\n"
                 + "  }\n"
                 + "  if (arg5 != null) {\n"
-                + "    if (testRef.arg5 == null || arg5.size() != testRef.arg5.size()) {\n"
+                + "    if (mountTestRef.arg5 == null || arg5.size() != mountTestRef.arg5.size()) {\n"
                 + "      return false;\n"
                 + "    }\n"
                 + "    java.util.Iterator<com.facebook.litho.Component> _e1_1 = arg5.iterator();\n"
-                + "    java.util.Iterator<com.facebook.litho.Component> _e2_1 = testRef.arg5.iterator();\n"
+                + "    java.util.Iterator<com.facebook.litho.Component> _e2_1 = mountTestRef.arg5.iterator();\n"
                 + "    while (_e1_1.hasNext() && _e2_1.hasNext()) {\n"
                 + "      if (!_e1_1.next().isEquivalentTo(_e2_1.next())) {\n"
                 + "        return false;\n"
                 + "      }\n"
                 + "    }\n"
-                + "  } else if (testRef.arg5 != null) {\n"
+                + "  } else if (mountTestRef.arg5 != null) {\n"
                 + "    return false;\n"
                 + "  }\n"
-                + "  if (arg6 != null ? !arg6.equals(testRef.arg6) : testRef.arg6 != null) {\n"
+                + "  if (arg6 != null ? !arg6.equals(mountTestRef.arg6) : mountTestRef.arg6 != null) {\n"
                 + "    return false;\n"
                 + "  }\n"
-                + "  if (mStateContainer.arg1 != testRef.mStateContainer.arg1) {\n"
+                + "  if (mStateContainer.arg1 != mountTestRef.mStateContainer.arg1) {\n"
                 + "    return false;\n"
                 + "  }\n"
-                + "  if (arg3 != testRef.arg3) {\n"
+                + "  if (arg3 != mountTestRef.arg3) {\n"
                 + "    return false;\n"
                 + "  }\n"
                 + "  if (arg7 != null) {\n"
-                + "    if (testRef.arg7 == null || arg7.size() != testRef.arg7.size()) {\n"
+                + "    if (mountTestRef.arg7 == null || arg7.size() != mountTestRef.arg7.size()) {\n"
                 + "      return false;\n"
                 + "    }\n"
                 + "    java.util.Iterator<java.util.List<com.facebook.litho.Row>> _e1_2 = arg7.iterator();\n"
-                + "    java.util.Iterator<java.util.List<com.facebook.litho.Row>> _e2_2 = testRef.arg7.iterator();\n"
+                + "    java.util.Iterator<java.util.List<com.facebook.litho.Row>> _e2_2 = mountTestRef.arg7.iterator();\n"
                 + "    while (_e1_2.hasNext() && _e2_2.hasNext()) {\n"
                 + "      if (_e1_2.next().size() != _e2_2.next().size()) {\n"
                 + "        return false;\n"
@@ -373,10 +414,10 @@ public void testGenerateIsEquivalentMethod() {
                 + "        }\n"
                 + "      }\n"
                 + "    }\n"
-                + "  } else if (testRef.arg7 != null) {\n"
+                + "  } else if (mountTestRef.arg7 != null) {\n"
                 + "    return false;\n"
                 + "  }\n"
-                + "  if (arg8 != null ? !arg8.equals(testRef.arg8) : testRef.arg8 != null) {\n"
+                + "  if (arg8 != null ? !arg8.equals(mountTestRef.arg8) : mountTestRef.arg8 != null) {\n"
                 + "    return false;\n"
                 + "  }\n"
                 + "  return true;\n"

diff --git a/litho-it/src/test/resources/processor/SimpleLayout.java b/litho-it/src/test/resources/processor/SimpleLayout.java
@@ -18,32 +18,12 @@
 
 import com.facebook.litho.Component;
 import com.facebook.litho.ComponentContext;
-import com.facebook.litho.config.ComponentsConfiguration;
 
 /** @see com.facebook.litho.processor.integration.resources.SimpleLayoutSpec */
 public final class SimpleLayout extends Component {
   private SimpleLayout() {
     super("SimpleLayout");
   }
-
-  @Override
-  public boolean isEquivalentTo(Component other) {
-    if (ComponentsConfiguration.useNewIsEquivalentToInLayoutSpec) {
-      return super.isEquivalentTo(other);
-    }
-    if (this == other) {
-      return true;
-    }
-    if (other == null || getClass() != other.getClass()) {
-      return false;
-    }
-    SimpleLayout simpleLayoutRef = (SimpleLayout) other;
-    if (this.getId() == simpleLayoutRef.getId()) {
-      return true;
-    }
-    return true;
-  }
-
   @Override
   protected Component onCreateLayout(ComponentContext context) {
     Component _result = (Component) SimpleLayoutSpec.onCreateLayout((ComponentContext) context);

diff --git a/litho-it/src/test/resources/processor/SimpleMount.java b/litho-it/src/test/resources/processor/SimpleMount.java
@@ -25,7 +25,6 @@
 import com.facebook.litho.annotations.Comparable;
 import com.facebook.litho.annotations.Prop;
 import com.facebook.litho.annotations.ResType;
-import com.facebook.litho.config.ComponentsConfiguration;
 import java.util.BitSet;
 
 /**
@@ -48,9 +47,6 @@ private SimpleMount() {
 
   @Override
   public boolean isEquivalentTo(Component other) {
-    if (ComponentsConfiguration.useNewIsEquivalentToInMountSpec) {
-      return super.isEquivalentTo(other);
-    }
     if (this == other) {
       return true;
     }

diff --git a/litho-it/src/test/resources/processor/TestLayout.java b/litho-it/src/test/resources/processor/TestLayout.java
@@ -44,9 +44,7 @@
 import com.facebook.litho.annotations.ResType;
 import com.facebook.litho.annotations.State;
 import com.facebook.litho.annotations.TreeProp;
-import com.facebook.litho.config.ComponentsConfiguration;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.BitSet;
 import java.util.List;
 
@@ -136,71 +134,6 @@ protected StateContainer getStateContainer() {
     return mStateContainer;
   }
 
-  @Override
-  public boolean isEquivalentTo(Component other) {
-    if (ComponentsConfiguration.useNewIsEquivalentToInLayoutSpec) {
-      return super.isEquivalentTo(other);
-    }
-    if (this == other) {
-      return true;
-    }
-    if (other == null || getClass() != other.getClass()) {
-      return false;
-    }
-    TestLayout testLayoutRef = (TestLayout) other;
-    if (this.getId() == testLayoutRef.getId()) {
-      return true;
-    }
-    if (Float.compare(aspectRatio, testLayoutRef.aspectRatio) != 0) {
-      return false;
-    }
-    if (child != null ? !child.isEquivalentTo(testLayoutRef.child) : testLayoutRef.child != null) {
-      return false;
-    }
-    if (focusable != testLayoutRef.focusable) {
-      return false;
-    }
-    if (handler != null
-        ? !handler.isEquivalentTo(testLayoutRef.handler)
-        : testLayoutRef.handler != null) {
-      return false;
-    }
-    if (names != null ? !names.equals(testLayoutRef.names) : testLayoutRef.names != null) {
-      return false;
-    }
-    if (prop1 != testLayoutRef.prop1) {
-      return false;
-    }
-    if (prop2 != testLayoutRef.prop2) {
-      return false;
-    }
-    if (prop3 != null ? !prop3.equals(testLayoutRef.prop3) : testLayoutRef.prop3 != null) {
-      return false;
-    }
-    if (!Arrays.equals(prop4, testLayoutRef.prop4)) {
-      return false;
-    }
-    if (prop5 != testLayoutRef.prop5) {
-      return false;
-    }
-    if (prop6 != testLayoutRef.prop6) {
-      return false;
-    }
-    if (mStateContainer.state1 != testLayoutRef.mStateContainer.state1) {
-      return false;
-    }
-    if (mStateContainer.state2 != null ? !mStateContainer.state2.equals(testLayoutRef.mStateContainer.state2) : testLayoutRef.mStateContainer.state2 != null) {
-      return false;
-    }
-    if (mStateContainer.state3 != testLayoutRef.mStateContainer.state3) {
-      return false;
-    }
-    if (treeProp != null ? !treeProp.equals(testLayoutRef.treeProp) : testLayoutRef.treeProp != null) {
-      return false;
-    }
-    return true;
-  }
-
   private UpdateCurrentStateStateUpdate createUpdateCurrentStateStateUpdate(int someParam) {
     return new UpdateCurrentStateStateUpdate(someParam);
   }

diff --git a/litho-it/src/test/resources/processor/TestMount.java b/litho-it/src/test/resources/processor/TestMount.java
@@ -47,7 +47,6 @@
 import com.facebook.litho.annotations.ResType;
 import com.facebook.litho.annotations.State;
 import com.facebook.litho.annotations.TreeProp;
-import com.facebook.litho.config.ComponentsConfiguration;
 import java.util.Arrays;
 import java.util.BitSet;
 import javax.annotation.Nullable;
@@ -127,9 +126,6 @@ protected StateContainer getStateContainer() {
 
   @Override
   public boolean isEquivalentTo(Component other) {
-    if (ComponentsConfiguration.useNewIsEquivalentToInMountSpec) {
-      return super.isEquivalentTo(other);
-    }
     if (this == other) {
       return true;
     }

diff --git a/litho-it/src/test/resources/processor/sections/FullDiffSection.java b/litho-it/src/test/resources/processor/sections/FullDiffSection.java
@@ -30,7 +30,6 @@
 import com.facebook.litho.annotations.Prop;
 import com.facebook.litho.annotations.ResType;
 import com.facebook.litho.annotations.State;
-import com.facebook.litho.config.ComponentsConfiguration;
 import com.facebook.litho.sections.ChangeSet;
 import com.facebook.litho.sections.LoadingEvent;
 import com.facebook.litho.sections.Section;
@@ -85,9 +84,6 @@ protected StateContainer getStateContainer() {
 
   @Override
   public boolean isEquivalentTo(Section other) {
-    if (ComponentsConfiguration.useNewIsEquivalentToInSectionSpec) {
-      return super.isEquivalentTo(other);
-    }
     if (this == other) {
       return true;
     }

diff --git a/litho-it/src/test/resources/processor/sections/FullGroupSection.java b/litho-it/src/test/resources/processor/sections/FullGroupSection.java
@@ -34,7 +34,6 @@
 import com.facebook.litho.annotations.ResType;
 import com.facebook.litho.annotations.State;
 import com.facebook.litho.annotations.TreeProp;
-import com.facebook.litho.config.ComponentsConfiguration;
 import com.facebook.litho.sections.Children;
 import com.facebook.litho.sections.LoadingEvent;
 import com.facebook.litho.sections.Section;
@@ -101,9 +100,6 @@ protected StateContainer getStateContainer() {
 
   @Override
   public boolean isEquivalentTo(Section other) {
-    if (ComponentsConfiguration.useNewIsEquivalentToInSectionSpec) {
-      return super.isEquivalentTo(other);
-    }
     if (this == other) {
       return true;
     }

diff --git a/litho-it/src/test/resources/processor/sections/SimpleDiffSection.java b/litho-it/src/test/resources/processor/sections/SimpleDiffSection.java
@@ -17,7 +17,6 @@
 package com.facebook.litho.sections.processor.integration.resources;
 
 import com.facebook.litho.EventHandler;
-import com.facebook.litho.config.ComponentsConfiguration;
 import com.facebook.litho.sections.ChangeSet;
 import com.facebook.litho.sections.LoadingEvent;
 import com.facebook.litho.sections.Section;
@@ -31,9 +30,6 @@ private SimpleDiffSection() {
 
   @Override
   public boolean isEquivalentTo(Section other) {
-    if (ComponentsConfiguration.useNewIsEquivalentToInSectionSpec) {
-      return super.isEquivalentTo(other);
-    }
     if (this == other) {
       return true;
     }