From 0cef464fd25e4856b18dd10c650267863feb12b5 Mon Sep 17 00:00:00 2001
From: Joshua Gross <joshuagross@fb.com>
Date: Tue, 30 Jun 2020 22:03:38 -0700
Subject: [PATCH] StopSurface optimizations

Summary:
(1) As soon as we know we can StopSurface, stop executing all mountitems by clearing out the root tag.
(2) If a surface has been stopped and there's a batch of MountItems to execute against it, execute only the "delete" operations to clear views from memory.

Both of these should reduce memory usage and improve speed slightly around navigation pops.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22321389

fbshipit-source-id: 96a8292a8442528f1bba50d35208885cc4168170
---
 .../react/fabric/FabricUIManager.java         |  3 +-
 .../mounting/mountitems/BatchMountItem.java   | 38 +++++++++++++++----
 .../RemoveDeleteMultiMountItem.java           | 21 +++++++++-
 3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
index 1edbe4229cb..3f3eb338d13 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
@@ -253,8 +253,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
   @ThreadConfined(ANY)
   @Override
   public void stopSurface(int surfaceID) {
-    mBinding.stopSurface(surfaceID);
     mReactContextForRootTag.remove(surfaceID);
+    mBinding.stopSurface(surfaceID);
   }
 
   @Override
@@ -853,6 +853,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
           BatchMountItem batchMountItem = (BatchMountItem) mountItem;
           if (!surfaceActiveForExecution(
               batchMountItem.getRootTag(), "dispatchMountItems BatchMountItem")) {
+            batchMountItem.executeDeletes(mMountingManager);
             continue;
           }
         }
diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/BatchMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/BatchMountItem.java
index 91e736d5335..7aebdd3d071 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/BatchMountItem.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/BatchMountItem.java
@@ -45,21 +45,18 @@ public class BatchMountItem implements MountItem {
     mCommitNumber = commitNumber;
   }
 
-  @Override
-  public void execute(@NonNull MountingManager mountingManager) {
+  private void beginMarkers(String reason) {
     Systrace.beginSection(
-        Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "FabricUIManager::mountViews - " + mSize + " items");
+        Systrace.TRACE_TAG_REACT_JAVA_BRIDGE,
+        "FabricUIManager::" + reason + " - " + mSize + " items");
 
     if (mCommitNumber > 0) {
       ReactMarker.logFabricMarker(
           ReactMarkerConstants.FABRIC_BATCH_EXECUTION_START, null, mCommitNumber);
     }
+  }
 
-    for (int mountItemIndex = 0; mountItemIndex < mSize; mountItemIndex++) {
-      MountItem mountItem = mMountItems[mountItemIndex];
-      mountItem.execute(mountingManager);
-    }
-
+  private void endMarkers() {
     if (mCommitNumber > 0) {
       ReactMarker.logFabricMarker(
           ReactMarkerConstants.FABRIC_BATCH_EXECUTION_END, null, mCommitNumber);
@@ -68,6 +65,31 @@ public class BatchMountItem implements MountItem {
     Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
   }
 
+  @Override
+  public void execute(@NonNull MountingManager mountingManager) {
+    beginMarkers("mountViews");
+
+    for (int mountItemIndex = 0; mountItemIndex < mSize; mountItemIndex++) {
+      MountItem mountItem = mMountItems[mountItemIndex];
+      mountItem.execute(mountingManager);
+    }
+
+    endMarkers();
+  }
+
+  public void executeDeletes(@NonNull MountingManager mountingManager) {
+    beginMarkers("deleteViews");
+
+    for (int mountItemIndex = 0; mountItemIndex < mSize; mountItemIndex++) {
+      MountItem mountItem = mMountItems[mountItemIndex];
+      if (mountItem instanceof RemoveDeleteMultiMountItem) {
+        ((RemoveDeleteMultiMountItem) mountItem).executeDeletes(mountingManager, true);
+      }
+    }
+
+    endMarkers();
+  }
+
   public int getRootTag() {
     return mRootTag;
   }
diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveDeleteMultiMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveDeleteMultiMountItem.java
index daf3ed5ca75..8a25f84b743 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveDeleteMultiMountItem.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveDeleteMultiMountItem.java
@@ -56,11 +56,30 @@ public class RemoveDeleteMultiMountItem implements MountItem {
     }
 
     // After removing all views, delete all views marked for deletion.
+    executeDeletes(mountingManager, false);
+  }
+
+  /**
+   * Execute only deletion operations. When being executed as part of shutdown/stopping surface,
+   * deletion failures can be ignored. For example: if there is a batch of MountItems being executed
+   * as part of stopSurface, a "Create" that is not executed may have a matching "Delete". The
+   * Delete will fail but we can safely ignore it in those cases.
+   *
+   * @param mountingManager
+   * @param ignoreFailures
+   */
+  public void executeDeletes(@NonNull MountingManager mountingManager, boolean ignoreFailures) {
     for (int i = 0; i < mMetadata.length; i += 4) {
       int flags = mMetadata[i + FLAGS_INDEX];
       if ((flags & DELETE_FLAG) != 0) {
         int tag = mMetadata[i + TAG_INDEX];
-        mountingManager.deleteView(tag);
+        try {
+          mountingManager.deleteView(tag);
+        } catch (IllegalStateException e) {
+          if (!ignoreFailures) {
+            throw e;
+          }
+        }
       }
     }
   }