From a7adaf3062d51ed83ad66e86fb2093db74634e4a Mon Sep 17 00:00:00 2001 From: Andrei Shikov Date: Tue, 15 Jun 2021 05:44:09 -0700 Subject: [PATCH] Retry mounting items that were scheduled on view attach Summary: The mount items executed before the view attach can fail with the RetryableMountingLayerException, which we should either retry or skip the item. This change updates logic for such items to catch these exceptions. Instead of passing `MountingManager` to execute the items, this change now passes a reference to the `MountItemDispatcher` (abstracted through interface to resolve cycle between the manager and dispatcher). The dispatcher executes the queue directly and schedules retry together with the next batch. Changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D29068063 fbshipit-source-id: 04030b21db188d5617c3448322d25ba77d5fbb9f --- .../react/fabric/FabricUIManager.java | 15 +++++++++-- .../fabric/mounting/MountItemDispatcher.java | 27 +++++++++++++++++++ .../fabric/mounting/MountingManager.java | 26 +++++++++++++++--- .../mounting/SurfaceMountingManager.java | 16 +++++------ 4 files changed, 71 insertions(+), 13 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 d0092602efa..fb916e66c7d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -84,6 +84,7 @@ import com.facebook.react.views.text.TextLayoutManagerMapBuffer; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Queue; import java.util.concurrent.CopyOnWriteArrayList; /** @@ -145,6 +146,16 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { // from C++ to exceed 9,999 and it should be obvious what's going on when analyzing performance. private int mCurrentSynchronousCommitNumber = 10000; + private MountingManager.MountItemExecutor mMountItemExecutor = + new MountingManager.MountItemExecutor() { + @Override + public void executeItems(Queue items) { + // This executor can be technically accessed before the dispatcher is created, + // but if that happens, something is terribly wrong + mMountItemDispatcher.dispatchMountItems(items); + } + }; + // TODO T83943316: Deprecate and delete this constructor once StaticViewConfigs are enabled by // default public FabricUIManager( @@ -154,7 +165,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { EventBeatManager eventBeatManager) { mDispatchUIFrameCallback = new DispatchUIFrameCallback(reactContext); mReactApplicationContext = reactContext; - mMountingManager = new MountingManager(viewManagerRegistry); + mMountingManager = new MountingManager(viewManagerRegistry, mMountItemExecutor); mMountItemDispatcher = new MountItemDispatcher(mMountingManager, new MountItemDispatchListener()); mEventDispatcher = eventDispatcher; @@ -169,7 +180,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { EventBeatManager eventBeatManager) { mDispatchUIFrameCallback = new DispatchUIFrameCallback(reactContext); mReactApplicationContext = reactContext; - mMountingManager = new MountingManager(viewManagerRegistry); + mMountingManager = new MountingManager(viewManagerRegistry, mMountItemExecutor); mMountItemDispatcher = new MountItemDispatcher(mMountingManager, new MountItemDispatchListener()); mEventDispatcher = diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java index 2155755f451..dc9c51df04f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java @@ -30,6 +30,7 @@ import com.facebook.systrace.Systrace; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; public class MountItemDispatcher { @@ -145,6 +146,32 @@ public class MountItemDispatcher { return didDispatchItems; } + @UiThread + @ThreadConfined(UI) + public void dispatchMountItems(Queue mountItems) { + while (!mountItems.isEmpty()) { + MountItem item = mountItems.poll(); + try { + item.execute(mMountingManager); + } catch (RetryableMountingLayerException e) { + if (item instanceof DispatchCommandMountItem) { + // Only DispatchCommandMountItem supports retries + DispatchCommandMountItem mountItem = (DispatchCommandMountItem) item; + // Retrying exactly once + if (mountItem.getRetries() == 0) { + mountItem.incrementRetries(); + // In case we haven't retried executing this item yet, execute in the next batch of + // items + dispatchCommandMountItem(mountItem); + } + } else { + printMountItem( + item, "dispatchExternalMountItems: mounting failed with " + e.getMessage()); + } + } + } + } + @UiThread @ThreadConfined(UI) /** Nothing should call this directly except for `tryDispatchMountItems`. */ diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java index 97a8822d949..a853fe9ac2d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java @@ -8,6 +8,7 @@ package com.facebook.react.fabric.mounting; import static com.facebook.infer.annotation.ThreadConfined.ANY; +import static com.facebook.infer.annotation.ThreadConfined.UI; import android.text.Spannable; import android.view.View; @@ -26,6 +27,7 @@ import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.common.mapbuffer.ReadableMapBuffer; import com.facebook.react.fabric.FabricUIManager; import com.facebook.react.fabric.events.EventEmitterWrapper; +import com.facebook.react.fabric.mounting.mountitems.MountItem; import com.facebook.react.touch.JSResponderHandler; import com.facebook.react.uimanager.RootViewManager; import com.facebook.react.uimanager.ThemedReactContext; @@ -34,6 +36,7 @@ import com.facebook.react.views.text.ReactTextViewManagerCallback; import com.facebook.react.views.text.TextLayoutManagerMapBuffer; import com.facebook.yoga.YogaMeasureMode; import java.util.Map; +import java.util.Queue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; @@ -56,10 +59,20 @@ public class MountingManager { @NonNull private final JSResponderHandler mJSResponderHandler = new JSResponderHandler(); @NonNull private final ViewManagerRegistry mViewManagerRegistry; + @NonNull private final MountItemExecutor mMountItemExecutor; @NonNull private final RootViewManager mRootViewManager = new RootViewManager(); - public MountingManager(@NonNull ViewManagerRegistry viewManagerRegistry) { + public interface MountItemExecutor { + @UiThread + @ThreadConfined(UI) + void executeItems(Queue items); + } + + public MountingManager( + @NonNull ViewManagerRegistry viewManagerRegistry, + @NonNull MountItemExecutor mountItemExecutor) { mViewManagerRegistry = viewManagerRegistry; + mMountItemExecutor = mountItemExecutor; } /** Starts surface and attaches the root view. */ @@ -78,7 +91,11 @@ public class MountingManager { public SurfaceMountingManager startSurface(final int surfaceId) { SurfaceMountingManager surfaceMountingManager = new SurfaceMountingManager( - surfaceId, mJSResponderHandler, mViewManagerRegistry, mRootViewManager, this); + surfaceId, + mJSResponderHandler, + mViewManagerRegistry, + mRootViewManager, + mMountItemExecutor); // There could technically be a race condition here if addRootView is called twice from // different threads, though this is (probably) extremely unlikely, and likely an error. @@ -149,7 +166,10 @@ public class MountingManager { && mMostRecentSurfaceMountingManager.getSurfaceId() == surfaceId) { return mMostRecentSurfaceMountingManager; } - return mSurfaceIdToManager.get(surfaceId); + + SurfaceMountingManager surfaceMountingManager = mSurfaceIdToManager.get(surfaceId); + mLastQueriedSurfaceMountingManager = surfaceMountingManager; + return surfaceMountingManager; } @NonNull diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java index 41e1bc9f221..4047935cea4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java @@ -8,6 +8,7 @@ package com.facebook.react.fabric.mounting; import static com.facebook.infer.annotation.ThreadConfined.ANY; +import static com.facebook.infer.annotation.ThreadConfined.UI; import android.view.View; import android.view.ViewGroup; @@ -26,6 +27,7 @@ import com.facebook.react.bridge.SoftAssertions; import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.common.build.ReactBuildConfig; import com.facebook.react.fabric.events.EventEmitterWrapper; +import com.facebook.react.fabric.mounting.MountingManager.MountItemExecutor; import com.facebook.react.fabric.mounting.mountitems.MountItem; import com.facebook.react.touch.JSResponderHandler; import com.facebook.react.uimanager.IllegalViewOperationException; @@ -60,7 +62,7 @@ public class SurfaceMountingManager { private JSResponderHandler mJSResponderHandler; private ViewManagerRegistry mViewManagerRegistry; private RootViewManager mRootViewManager; - private MountingManager mMountingManager; + private MountItemExecutor mMountItemExecutor; // This is null *until* StopSurface is called. private Set mTagSetForStoppedSurface; @@ -72,13 +74,13 @@ public class SurfaceMountingManager { @NonNull JSResponderHandler jsResponderHandler, @NonNull ViewManagerRegistry viewManagerRegistry, @NonNull RootViewManager rootViewManager, - @NonNull MountingManager mountingManager) { + @NonNull MountItemExecutor mountItemExecutor) { mSurfaceId = surfaceId; mJSResponderHandler = jsResponderHandler; mViewManagerRegistry = viewManagerRegistry; mRootViewManager = rootViewManager; - mMountingManager = mountingManager; + mMountItemExecutor = mountItemExecutor; } public boolean isStopped() { @@ -206,11 +208,9 @@ public class SurfaceMountingManager { } @UiThread + @ThreadConfined(UI) private void executeViewAttachMountItems() { - while (!mOnViewAttachItems.isEmpty()) { - MountItem item = mOnViewAttachItems.poll(); - item.execute(mMountingManager); - } + mMountItemExecutor.executeItems(mOnViewAttachItems); } /** @@ -269,7 +269,7 @@ public class SurfaceMountingManager { mTagToViewState = null; mJSResponderHandler = null; mRootViewManager = null; - mMountingManager = null; + mMountItemExecutor = null; mOnViewAttachItems.clear(); } };