Skip to content

Commit

Permalink
Retry mounting items that were scheduled on view attach
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Andrei Shikov authored and facebook-github-bot committed Jun 15, 2021
1 parent f78526c commit a7adaf3
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 13 deletions.
Expand Up @@ -84,6 +84,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Queue;
import java.util.concurrent.CopyOnWriteArrayList;

/**
Expand Down Expand Up @@ -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<MountItem> 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(
Expand All @@ -154,7 +165,7 @@ public FabricUIManager(
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;
Expand All @@ -169,7 +180,7 @@ public FabricUIManager(
EventBeatManager eventBeatManager) {
mDispatchUIFrameCallback = new DispatchUIFrameCallback(reactContext);
mReactApplicationContext = reactContext;
mMountingManager = new MountingManager(viewManagerRegistry);
mMountingManager = new MountingManager(viewManagerRegistry, mMountItemExecutor);
mMountItemDispatcher =
new MountItemDispatcher(mMountingManager, new MountItemDispatchListener());
mEventDispatcher =
Expand Down
Expand Up @@ -30,6 +30,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;

public class MountItemDispatcher {
Expand Down Expand Up @@ -145,6 +146,32 @@ public boolean tryDispatchMountItems() {
return didDispatchItems;
}

@UiThread
@ThreadConfined(UI)
public void dispatchMountItems(Queue<MountItem> 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`. */
Expand Down
Expand Up @@ -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;
Expand All @@ -26,6 +27,7 @@
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;
Expand All @@ -34,6 +36,7 @@
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;

Expand All @@ -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<MountItem> items);
}

public MountingManager(
@NonNull ViewManagerRegistry viewManagerRegistry,
@NonNull MountItemExecutor mountItemExecutor) {
mViewManagerRegistry = viewManagerRegistry;
mMountItemExecutor = mountItemExecutor;
}

/** Starts surface and attaches the root view. */
Expand All @@ -78,7 +91,11 @@ public void startSurface(
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.
Expand Down Expand Up @@ -149,7 +166,10 @@ public SurfaceMountingManager getSurfaceManager(int surfaceId) {
&& mMostRecentSurfaceMountingManager.getSurfaceId() == surfaceId) {
return mMostRecentSurfaceMountingManager;
}
return mSurfaceIdToManager.get(surfaceId);

SurfaceMountingManager surfaceMountingManager = mSurfaceIdToManager.get(surfaceId);
mLastQueriedSurfaceMountingManager = surfaceMountingManager;
return surfaceMountingManager;
}

@NonNull
Expand Down
Expand Up @@ -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;
Expand All @@ -26,6 +27,7 @@
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;
Expand Down Expand Up @@ -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<Integer> mTagSetForStoppedSurface;
Expand All @@ -72,13 +74,13 @@ public 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() {
Expand Down Expand Up @@ -206,11 +208,9 @@ public void run() {
}

@UiThread
@ThreadConfined(UI)
private void executeViewAttachMountItems() {
while (!mOnViewAttachItems.isEmpty()) {
MountItem item = mOnViewAttachItems.poll();
item.execute(mMountingManager);
}
mMountItemExecutor.executeItems(mOnViewAttachItems);
}

/**
Expand Down Expand Up @@ -269,7 +269,7 @@ public void run() {
mTagToViewState = null;
mJSResponderHandler = null;
mRootViewManager = null;
mMountingManager = null;
mMountItemExecutor = null;
mOnViewAttachItems.clear();
}
};
Expand Down

0 comments on commit a7adaf3

Please sign in to comment.