Skip to content

Commit

Permalink
only schedule frame callback when there are mount items in FabricUIMa…
Browse files Browse the repository at this point in the history
…nager

Summary:
changelog: [internal]

It is redundant to schedule frame callback if there is no work to do. Let's remove it.

Reviewed By: javache

Differential Revision: D50494928

fbshipit-source-id: fce7d9a84eb2486dc01d4bff98540c128b91969d
  • Loading branch information
sammy-SC authored and Othinn committed Jan 9, 2024
1 parent 0983617 commit baaf673
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,10 @@ public class ReactFeatureFlags {
* longer work as they won't subscribe to ReactChoregrapher for updates.
*/
public static boolean enableFabricRendererExclusively = false;

/*
* When enabled, uses of ReactChoreographer (e.g. FabricUIManager) will only post callback
* when there is work to do.
*/
public static boolean enableOnDemandReactChoreographer = false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import com.facebook.react.bridge.UIManagerListener;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.bridge.WritableMap;
import com.facebook.react.common.LifecycleState;
import com.facebook.react.common.build.ReactBuildConfig;
import com.facebook.react.common.mapbuffer.ReadableMapBuffer;
import com.facebook.react.config.ReactFeatureFlags;
Expand Down Expand Up @@ -188,6 +189,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
private volatile boolean mDestroyed = false;

private boolean mDriveCxxAnimations = false;
private final AtomicBoolean mIsFrameCallbackScheduled = new AtomicBoolean(false);

private long mDispatchViewUpdatesTime = 0l;
private long mCommitStartTime = 0l;
Expand Down Expand Up @@ -693,7 +695,7 @@ public String toString() {
// If the reactTag exists, we assume that it might at the end of the next
// batch of MountItems. Otherwise, we try to execute immediately.
if (!mMountingManager.getViewExists(reactTag)) {
mMountItemDispatcher.addMountItem(synchronousMountItem);
addMountItem(synchronousMountItem);
return;
}

Expand Down Expand Up @@ -726,7 +728,7 @@ private void preallocateView(
@Nullable Object eventEmitterWrapper,
boolean isLayoutable) {

mMountItemDispatcher.addPreAllocateMountItem(
addPreAllocateMountItem(
MountItemFactory.createPreAllocateViewMountItem(
rootTag,
reactTag,
Expand Down Expand Up @@ -799,6 +801,8 @@ public void runGuarded() {
};
if (UiThreadUtil.isOnUiThread()) {
runnable.run();
} else {
postChoreographerCallbackIfNecessary();
}
}

Expand Down Expand Up @@ -959,10 +963,50 @@ public void receiveEvent(
}
}

private void addPreAllocateMountItem(final MountItem mountItem) {
mMountItemDispatcher.addPreAllocateMountItem(mountItem);
postChoreographerCallbackIfNecessary();
}

private void addMountItem(final MountItem mountItem) {
mMountItemDispatcher.addMountItem(mountItem);
postChoreographerCallbackIfNecessary();
}

private void addViewCommandMountItem(final DispatchCommandMountItem mountItem) {
mMountItemDispatcher.addViewCommandMountItem(mountItem);
postChoreographerCallbackIfNecessary();
}

private void postChoreographerCallbackIfNecessary() {
if (ReactFeatureFlags.enableOnDemandReactChoreographer == false) {
return;
}

if (mIsFrameCallbackScheduled.get()) {
return;
}

boolean isWorkPending = mMountItemDispatcher.hasMountItems() || mDriveCxxAnimations;
boolean isAppActive = mReactApplicationContext.getLifecycleState() == LifecycleState.RESUMED;

if (isWorkPending && isAppActive) {
// It is important to only schedule one frame callback at a time.
// Otherwise, with LayoutAnimations, deadlock may occur.
mIsFrameCallbackScheduled.set(true);
ReactChoreographer.getInstance()
.postFrameCallback(ReactChoreographer.CallbackType.DISPATCH_UI, mDispatchUIFrameCallback);
}
}

@Override
public void onHostResume() {
ReactChoreographer.getInstance()
.postFrameCallback(ReactChoreographer.CallbackType.DISPATCH_UI, mDispatchUIFrameCallback);
if (ReactFeatureFlags.enableOnDemandReactChoreographer) {
postChoreographerCallbackIfNecessary();
} else {
ReactChoreographer.getInstance()
.postFrameCallback(ReactChoreographer.CallbackType.DISPATCH_UI, mDispatchUIFrameCallback);
}
}

@Override
Expand Down Expand Up @@ -1009,7 +1053,7 @@ public void dispatchCommand(
final int reactTag,
final int commandId,
@Nullable final ReadableArray commandArgs) {
mMountItemDispatcher.addViewCommandMountItem(
addViewCommandMountItem(
MountItemFactory.createDispatchCommandMountItem(
surfaceId, reactTag, commandId, commandArgs));
}
Expand All @@ -1025,10 +1069,10 @@ public void dispatchCommand(
// For Fabric Interop, we check if the commandId is an integer. If it is, we use the integer
// overload of dispatchCommand. Otherwise, we use the string overload.
// and the events won't be correctly dispatched.
mMountItemDispatcher.addViewCommandMountItem(
addViewCommandMountItem(
createDispatchCommandMountItemForInterop(surfaceId, reactTag, commandId, commandArgs));
} else {
mMountItemDispatcher.addViewCommandMountItem(
addViewCommandMountItem(
MountItemFactory.createDispatchCommandMountItem(
surfaceId, reactTag, commandId, commandArgs));
}
Expand All @@ -1040,7 +1084,7 @@ public void dispatchCommand(
public void sendAccessibilityEvent(int reactTag, int eventType) {
// Can be called from native, not just JS - we need to migrate the native callsites
// before removing this entirely.
mMountItemDispatcher.addMountItem(
addMountItem(
MountItemFactory.createSendAccessibilityEventMountItem(View.NO_ID, reactTag, eventType));
}

Expand All @@ -1060,7 +1104,7 @@ public void sendAccessibilityEventFromJS(int surfaceId, int reactTag, String eve
throw new IllegalArgumentException(
"sendAccessibilityEventFromJS: invalid eventType " + eventTypeJS);
}
mMountItemDispatcher.addMountItem(
addMountItem(
MountItemFactory.createSendAccessibilityEventMountItem(surfaceId, reactTag, eventType));
}

Expand All @@ -1076,7 +1120,7 @@ public void setJSResponder(
final int reactTag,
final int initialReactTag,
final boolean blockNativeResponder) {
mMountItemDispatcher.addMountItem(
addMountItem(
new MountItem() {
@Override
public void execute(@NonNull MountingManager mountingManager) {
Expand Down Expand Up @@ -1110,7 +1154,7 @@ public String toString() {
* the touch events are going to be handled by JS.
*/
public void clearJSResponder() {
mMountItemDispatcher.addMountItem(
addMountItem(
new MountItem() {
@Override
public void execute(@NonNull MountingManager mountingManager) {
Expand Down Expand Up @@ -1151,6 +1195,7 @@ public String resolveCustomDirectEventName(@Nullable String eventName) {
@AnyThread
public void onAnimationStarted() {
mDriveCxxAnimations = true;
postChoreographerCallbackIfNecessary();
}

// Called from Binding.cpp
Expand Down Expand Up @@ -1266,6 +1311,7 @@ void stop() {
@UiThread
@ThreadConfined(UI)
public void doFrameGuarded(long frameTimeNanos) {
mIsFrameCallbackScheduled.set(false);
if (!mIsMountingEnabled || mDestroyed) {
FLog.w(TAG, "Not flushing pending UI operations because of previously thrown Exception");
return;
Expand All @@ -1292,9 +1338,13 @@ public void doFrameGuarded(long frameTimeNanos) {
stop();
throw ex;
} finally {
ReactChoreographer.getInstance()
.postFrameCallback(
ReactChoreographer.CallbackType.DISPATCH_UI, mDispatchUIFrameCallback);
if (ReactFeatureFlags.enableOnDemandReactChoreographer == false) {
ReactChoreographer.getInstance()
.postFrameCallback(
ReactChoreographer.CallbackType.DISPATCH_UI, mDispatchUIFrameCallback);
} else {
postChoreographerCallbackIfNecessary();
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,10 @@ private boolean dispatchMountItems() {
return true;
}

public boolean hasMountItems() {
return !mPreMountItems.isEmpty() || !mMountItems.isEmpty() || !mViewCommandMountItems.isEmpty();
}

/*
* Executes pre mount items. Pre mount items are operations that can be executed before the mount items come. For example view preallocation.
* This is a performance optimisation to do as much work ahead of time as possible.
Expand Down

0 comments on commit baaf673

Please sign in to comment.