RemoveDeleteTree mount instruction

Summary:
TL;DR: For applications using JS navigation, save 50-95% of CPU during mounting phase in N>2 navigations that replace ~most of screen.

During investigation of performance on the UI thread of React Native applications, I noticed that the /initial/ render of an screen for an application using JS navigation is /mostly/ consumed (on the UI thread) by tearing-down the previous View hierarchy. In one 185ms segment on the UI thread in production, 95% of the CPU time was Remove/Delete instructions and only 5% of CPU time was consumed by actually displaying the new hierarchy (this is specific to Android and also assumes that View Preallocation is being used, so post-commit work consists of Insert and UpdateLayout mutations primarily).

There are /some/ cases where the C++ differ knows that we are deleting an entire subtree and therefore we could communicate this to the mounting layer. All that matters is that these Views are removed from the View hierarchy immediately; and secondarily that their memory is cleaned up ASAP, but that doesn't need to happen immediately.

Some additional constraints and notes:

1) As noted in the comments, we cannot simply stop producing Remove and Delete instructions. We need to produce /both/ the new RemoveDeleteTree instruction, /and/ produce all the Remove/Delete instructions, primarily because LayoutAnimations relies heavily on these Remove/Delete instructions and certain things would break if we removed those instructions entirely. However, we can mark those Remove/Delete instructions as redundant, process them only in LayoutAnimations, and not send them to the Android mounting layer.
2) We want to make sure that View Recycling is not impacted. Since Android cannot take advantage of View Recycling until /after/ the second major render (preallocation of views will happen before any views are recycled), this doesn't impact View Recycling and we'll make sure Views are recycled whenever they are deleted.

Thus, we do two things:

1) Introduce a new RemoveDeleteTree operation that can delete an entire subtree recursively as part of one operation. This allows us to avoid serializing hundreds or thousands of instructions and prevents JNI traffic.
2) Besides removing the topmost View from the View hierarchy, and ensuring it's not drawn, the full teardown and recycling of the tree can happen /after/ the paint.

In some flows with JS navigation this saves us 95% of CPU during the mount phase. In the general case it is probably closer to 25-50% of CPU time that is saved and/or deferred.

Changelog: [Android][Changed] Significant perf optimization to Fabric Remove/Delete operations

Reviewed By: ryancat

Differential Revision: D37257864

fbshipit-source-id: a7d33fc74683939965cfb98be4db7890644110b2

React/Fabric/Mounting/RCTMountingManager.mm

@@ -104,6 +104,11 @@ static void RCTPerformMountInstructions(
         break;
       }
 
+      case ShadowViewMutation::RemoveDeleteTree: {
+        // TODO - not supported yet
+        break;
+      }
+
       case ShadowViewMutation::Update: {
         auto &oldChildShadowView = mutation.oldChildShadowView;
         auto &newChildShadowView = mutation.newChildShadowView;

ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java

@@ -135,4 +135,9 @@ public class ReactFeatureFlags {
    * Enable prop iterator setter-style construction of Props in C++ (this flag is not used in Java).
    */
   public static boolean enableCppPropsIteratorSetter = false;
+
+  /**
+   * Allow Differentiator.cpp and FabricMountingManager.cpp to generate a RemoveDeleteTree mega-op.
+   */
+  public static boolean enableRemoveDeleteTreeInstruction = false;
 }

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp

@@ -451,6 +451,10 @@ void Binding::installFabricUIManager(
       Props::enablePropIteratorSetter;
   BaseTextProps::enablePropIteratorSetter = Props::enablePropIteratorSetter;
 
+  // RemoveDelete mega-op
+  ShadowViewMutation::PlatformSupportsRemoveDeleteTreeInstruction =
+      getFeatureFlagValue("enableRemoveDeleteTreeInstruction");
+
   auto toolbox = SchedulerToolbox{};
   toolbox.contextContainer = contextContainer;
   toolbox.componentRegistryFactory = componentsRegistry->buildRegistryFunction;

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountItem.cpp

@@ -28,6 +28,13 @@ CppMountItem CppMountItem::RemoveMountItem(
     int index) {
   return {CppMountItem::Type::Remove, parentView, shadowView, {}, index};
 }
+CppMountItem CppMountItem::RemoveDeleteTreeMountItem(
+    ShadowView const &parentView,
+    ShadowView const &shadowView,
+    int index) {
+  return {
+      CppMountItem::Type::RemoveDeleteTree, parentView, shadowView, {}, index};
+}
 CppMountItem CppMountItem::UpdatePropsMountItem(
     ShadowView const &oldShadowView,
     ShadowView const &newShadowView) {

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountItem.h

@@ -35,6 +35,11 @@ struct CppMountItem final {
       ShadowView const &shadowView,
       int index);
 
+  static CppMountItem RemoveDeleteTreeMountItem(
+      ShadowView const &parentView,
+      ShadowView const &shadowView,
+      int index);
+
   static CppMountItem UpdatePropsMountItem(
       ShadowView const &oldShadowView,
       ShadowView const &newShadowView);
@@ -64,7 +69,8 @@ struct CppMountItem final {
     UpdateLayout = 128,
     UpdateEventEmitter = 256,
     UpdatePadding = 512,
-    UpdateOverflowInset = 1024
+    UpdateOverflowInset = 1024,
+    RemoveDeleteTree = 2048,
   };
 
 #pragma mark - Fields

ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.cpp

@@ -69,6 +69,8 @@ static inline int getIntBufferSizeForType(CppMountItem::Type mountItemType) {
     case CppMountItem::Type::Insert:
     case CppMountItem::Type::Remove:
       return 3; // tag, parentTag, index
+    case CppMountItem::Type::RemoveDeleteTree:
+      return 3; // tag, parentTag, index
     case CppMountItem::Type::Delete:
     case CppMountItem::Type::UpdateProps:
     case CppMountItem::Type::UpdateState:
@@ -332,15 +334,25 @@ void FabricMountingManager::executeMount(
           break;
         }
         case ShadowViewMutation::Remove: {
-          if (!isVirtual) {
+          if (!isVirtual && !mutation.isRedundantOperation) {
             cppCommonMountItems.push_back(CppMountItem::RemoveMountItem(
                 parentShadowView, oldChildShadowView, index));
           }
           break;
         }
+        case ShadowViewMutation::RemoveDeleteTree: {
+          if (!isVirtual) {
+            cppCommonMountItems.push_back(
+                CppMountItem::RemoveDeleteTreeMountItem(
+                    parentShadowView, oldChildShadowView, index));
+          }
+          break;
+        }
         case ShadowViewMutation::Delete: {
-          cppDeleteMountItems.push_back(
-              CppMountItem::DeleteMountItem(oldChildShadowView));
+          if (!mutation.isRedundantOperation) {
+            cppDeleteMountItems.push_back(
+                CppMountItem::DeleteMountItem(oldChildShadowView));
+          }
           break;
         }
         case ShadowViewMutation::Update: {
@@ -609,6 +621,12 @@ void FabricMountingManager::executeMount(
       temp[2] = mountItem.index;
       env->SetIntArrayRegion(intBufferArray, intBufferPosition, 3, temp);
       intBufferPosition += 3;
+    } else if (mountItemType == CppMountItem::RemoveDeleteTree) {
+      temp[0] = mountItem.oldChildShadowView.tag;
+      temp[1] = mountItem.parentShadowView.tag;
+      temp[2] = mountItem.index;
+      env->SetIntArrayRegion(intBufferArray, intBufferPosition, 3, temp);
+      intBufferPosition += 3;
     } else {
       LOG(ERROR) << "Unexpected CppMountItem type";
     }

ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java

@@ -51,6 +51,7 @@
 import java.util.LinkedList;
 import java.util.Queue;
 import java.util.Set;
+import java.util.Stack;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import javax.annotation.Nullable;
@@ -74,6 +75,11 @@ public class SurfaceMountingManager {
   private RootViewManager mRootViewManager;
   private MountItemExecutor mMountItemExecutor;
 
+  // Stack of deferred-removal tags for Views that can be
+  // removed asynchronously. Guaranteed to be disconnected
+  // from the viewport and these tags will not be reused in the future.
+  private final Stack<Integer> mReactTagsToRemove = new Stack<>();
+
   // This is null *until* StopSurface is called.
   private Set<Integer> mTagSetForStoppedSurface;
 
@@ -561,6 +567,223 @@ public void run() {
     }
   }
 
+  @UiThread
+  public void removeDeleteTreeAt(final int tag, final int parentTag, int index) {
+    if (isStopped()) {
+      return;
+    }
+
+    UiThreadUtil.assertOnUiThread();
+    ViewState parentViewState = getNullableViewState(parentTag);
+
+    // TODO: throw exception here?
+    if (parentViewState == null) {
+      ReactSoftExceptionLogger.logSoftException(
+          MountingManager.TAG,
+          new IllegalStateException(
+              "Unable to find viewState for tag: [" + parentTag + "] for removeDeleteTreeAt"));
+      return;
+    }
+
+    if (!(parentViewState.mView instanceof ViewGroup)) {
+      String message =
+          "Unable to remove+delete a view from a view that is not a ViewGroup. ParentTag: "
+              + parentTag
+              + " - Tag: "
+              + tag
+              + " - Index: "
+              + index;
+      FLog.e(TAG, message);
+      throw new IllegalStateException(message);
+    }
+
+    final ViewGroup parentView = (ViewGroup) parentViewState.mView;
+
+    if (parentView == null) {
+      throw new IllegalStateException("Unable to find view for tag [" + parentTag + "]");
+    }
+
+    if (SHOW_CHANGED_VIEW_HIERARCHIES) {
+      // Display children before deleting any
+      FLog.e(
+          TAG,
+          "removeDeleteTreeAt: [" + tag + "] -> [" + parentTag + "] idx: " + index + " BEFORE");
+      logViewHierarchy(parentView, false);
+    }
+
+    ViewGroupManager<ViewGroup> viewGroupManager = getViewGroupManager(parentViewState);
+
+    // Verify that the view we're about to remove has the same tag we expect
+    View view = viewGroupManager.getChildAt(parentView, index);
+    int actualTag = (view != null ? view.getId() : -1);
+    if (actualTag != tag) {
+      int tagActualIndex = -1;
+      int parentChildrenCount = parentView.getChildCount();
+      for (int i = 0; i < parentChildrenCount; i++) {
+        if (parentView.getChildAt(i).getId() == tag) {
+          tagActualIndex = i;
+          break;
+        }
+      }
+
+      // TODO T74425739: previously, we did not do this check and `removeViewAt` would be executed
+      // below, sometimes crashing there. *However*, interestingly enough, `removeViewAt` would not
+      // complain if you removed views from an already-empty parent. This seems necessary currently
+      // for certain ViewManagers that remove their own children - like BottomSheet?
+      // This workaround seems not-great, but for now, we just return here for
+      // backwards-compatibility. Essentially, if a view has already been removed from the
+      // hierarchy, we treat it as a noop.
+      if (tagActualIndex == -1) {
+        FLog.e(
+            TAG,
+            "removeDeleteTreeAt: ["
+                + tag
+                + "] -> ["
+                + parentTag
+                + "] @"
+                + index
+                + ": view already removed from parent! Children in parent: "
+                + parentChildrenCount);
+        return;
+      }
+
+      // Here we are guaranteed that the view is still in the View hierarchy, just
+      // at a different index. In debug mode we'll crash here; in production, we'll remove
+      // the child from the parent and move on.
+      // This is an issue that is safely recoverable 95% of the time. If this allows corruption
+      // of the view hierarchy and causes bugs or a crash after this point, there will be logs
+      // indicating that this happened.
+      // This is likely *only* necessary because of Fabric's LayoutAnimations implementation.
+      // If we can fix the bug there, or remove the need for LayoutAnimation index adjustment
+      // entirely, we can just throw this exception without regression user experience.
+      logViewHierarchy(parentView, true);
+      ReactSoftExceptionLogger.logSoftException(
+          TAG,
+          new IllegalStateException(
+              "Tried to remove+delete view ["
+                  + tag
+                  + "] of parent ["
+                  + parentTag
+                  + "] at index "
+                  + index
+                  + ", but got view tag "
+                  + actualTag
+                  + " - actual index of view: "
+                  + tagActualIndex));
+      index = tagActualIndex;
+    }
+
+    try {
+      viewGroupManager.removeViewAt(parentView, index);
+    } catch (RuntimeException e) {
+      // Note: `getChildCount` may not always be accurate!
+      // We don't currently have a good explanation other than, in situations where you
+      // would empirically expect to see childCount > 0, the childCount is reported as 0.
+      // This is likely due to a ViewManager overriding getChildCount or some other methods
+      // in a way that is strictly incorrect, but potentially only visible here.
+      // The failure mode is actually that in `removeViewAt`, a NullPointerException is
+      // thrown when we try to perform an operation on a View that doesn't exist, and
+      // is therefore null.
+      // We try to add some extra diagnostics here, but we always try to remove the View
+      // from the hierarchy first because detecting by looking at childCount will not work.
+      //
+      // Note that the lesson here is that `getChildCount` is not /required/ to adhere to
+      // any invariants. If you add 9 children to a parent, the `getChildCount` of the parent
+      // may not be equal to 9. This apparently causes no issues with Android and is common
+      // enough that we shouldn't try to change this invariant, without a lot of thought.
+      int childCount = viewGroupManager.getChildCount(parentView);
+
+      logViewHierarchy(parentView, true);
+
+      throw new IllegalStateException(
+          "Cannot remove child at index "
+              + index
+              + " from parent ViewGroup ["
+              + parentView.getId()
+              + "], only "
+              + childCount
+              + " children in parent. Warning: childCount may be incorrect!",
+          e);
+    }
+
+    // Display children after deleting any
+    if (SHOW_CHANGED_VIEW_HIERARCHIES) {
+      final int finalIndex = index;
+      UiThreadUtil.runOnUiThread(
+          new Runnable() {
+            @Override
+            public void run() {
+              FLog.e(
+                  TAG,
+                  "removeViewAt: ["
+                      + tag
+                      + "] -> ["
+                      + parentTag
+                      + "] idx: "
+                      + finalIndex
+                      + " AFTER");
+              logViewHierarchy(parentView, false);
+            }
+          });
+    }
+
+    // The View has been removed from the View hierarchy; now it
+    // and all of its children, if any, need to be deleted, recursively.
+    // We want to maintain the legacy ordering: delete (and call onViewStateDeleted)
+    // for leaf nodes, and then parents, recursively.
+    mReactTagsToRemove.push(tag);
+    runDeferredTagRemovalAndDeletion();
+  }
+
+  @UiThread
+  private void runDeferredTagRemovalAndDeletion() {
+    UiThreadUtil.runOnUiThread(
+        new Runnable() {
+          @Override
+          public void run() {
+            int deletedViews = 1;
+            while (!mReactTagsToRemove.empty()) {
+              int reactTag = mReactTagsToRemove.pop();
+              ViewState thisViewState = getNullableViewState(reactTag);
+              if (thisViewState != null) {
+                View thisView = thisViewState.mView;
+                int numChildren = 0;
+                if (thisView instanceof ViewGroup) {
+                  View nextChild = null;
+                  // For reasons documented elsewhere in this class, getChildCount is not
+                  // necessarily
+                  // reliable, and so we rely instead on requesting children directly.
+                  while ((nextChild = ((ViewGroup) thisView).getChildAt(numChildren)) != null) {
+                    if (numChildren == 0) {
+                      // Push tag onto the stack so we reprocess it after all children
+                      mReactTagsToRemove.push(reactTag);
+                    }
+                    mReactTagsToRemove.push(nextChild.getId());
+                    numChildren++;
+                  }
+                  // Removing all at once is more efficient than removing one-by-one
+                  ((ViewGroup) thisView).removeAllViews();
+                }
+                if (numChildren == 0) {
+                  deletedViews++;
+                  mTagToViewState.remove(reactTag);
+                  onViewStateDeleted(thisViewState);
+                }
+                // circuit breaker
+                // TODO: check frame time
+                if (deletedViews > 200) {
+                  break;
+                }
+              }
+            }
+
+            if (!mReactTagsToRemove.empty()) {
+              runDeferredTagRemovalAndDeletion();
+            }
+          }
+        });
+  }
+
   @UiThread
   public void createView(
       @NonNull String componentName,

ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.java

@@ -49,6 +49,7 @@ public class IntBufferBatchMountItem implements MountItem {
   static final int INSTRUCTION_UPDATE_EVENT_EMITTER = 256;
   static final int INSTRUCTION_UPDATE_PADDING = 512;
   static final int INSTRUCTION_UPDATE_OVERFLOW_INSET = 1024;
+  static final int INSTRUCTION_REMOVE_DELETE_TREE = 2048;
 
   private final int mSurfaceId;
   private final int mCommitNumber;
@@ -139,6 +140,9 @@ public void execute(@NonNull MountingManager mountingManager) {
           surfaceMountingManager.addViewAt(parentTag, tag, mIntBuffer[i++]);
         } else if (type == INSTRUCTION_REMOVE) {
           surfaceMountingManager.removeViewAt(mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]);
+        } else if (type == INSTRUCTION_REMOVE_DELETE_TREE) {
+          surfaceMountingManager.removeDeleteTreeAt(
+              mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]);
         } else if (type == INSTRUCTION_UPDATE_PROPS) {
           surfaceMountingManager.updateProps(mIntBuffer[i++], mObjBuffer[j++]);
         } else if (type == INSTRUCTION_UPDATE_STATE) {
@@ -221,6 +225,11 @@ public String toString() {
             s.append(
                 String.format(
                     "REMOVE [%d]->[%d] @%d\n", mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]));
+          } else if (type == INSTRUCTION_REMOVE_DELETE_TREE) {
+            s.append(
+                String.format(
+                    "REMOVE+DELETE TREE [%d]->[%d] @%d\n",
+                    mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]));
           } else if (type == INSTRUCTION_UPDATE_PROPS) {
             Object props = mObjBuffer[j++];
             String propsString =