Skip to content

Commit

Permalink
Add option to make commit asynchronous
Browse files Browse the repository at this point in the history
Summary:
changelog: [internal]

`useLayoutEffect` has two guarantees which React Native breaks:
- Layout metrics are ready.
- Updates triggered inside `useLayoutEffect` are applied before paint. State between first commit and update is not shown on the screen.

React Native breaks the first guarantee because it uses Background Executor. Background executor moves Yoga layout to another thread. If user core reads layout metrics in `useLayoutEffect` hook, it is a race. The information might be there, or it might not. They can even read partially update information. This diff does not affect this. We already have a way to turn off Background Executor. We haven't done this because it introduces regressions on one screen but we have a solution for that.

React Native breaks the second guarantee. After Fabric's commit phase, Fabric moves to mounting the changes right away. In this diff, we queue the mounting phase and give React a chance to change what is committed to the screen. To do that, we schedule a task with user blocking priority in `RuntimeScheduler`. React, if there is an update in `useLayoutEffect`, schedules a task in the scheduler with higher priority and stops the mounting phase.
We are not delaying mounting, this just gives React a chance to interrupt it.

Fabric commit phase may be triggered by different mechanisms. C++ state update, surface tear down, template update (not used atm), setNativeProps, to name a few. Fabric only needs to block paint if commit originates from React. Otherwise the scheduling is wrong and we will get into undefined behaviour land.

Rollout:
This change is gated behind `react_fabric:block_paint_for_use_layout_effect` and will be rolled out incrementally.

Reviewed By: javache

Differential Revision: D43083051

fbshipit-source-id: bb494cf56a11763e38dce7ba0093c4dafdd8bf43
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Feb 9, 2023
1 parent 7535399 commit afec07a
Show file tree
Hide file tree
Showing 16 changed files with 84 additions and 27 deletions.
12 changes: 6 additions & 6 deletions React/Fabric/Mounting/RCTMountingManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,13 @@ - (void)scheduleTransaction:(MountingCoordinator::Shared)mountingCoordinator
// * No need to do a thread jump;
// * No need to do expensive copy of all mutations;
// * No need to allocate a block.
[self initiateTransaction:std::move(mountingCoordinator)];
[self initiateTransaction:*mountingCoordinator];
return;
}

RCTExecuteOnMainQueue(^{
RCTAssertMainQueue();
[self initiateTransaction:std::move(mountingCoordinator)];
[self initiateTransaction:*mountingCoordinator];
});
}

Expand Down Expand Up @@ -243,7 +243,7 @@ - (void)sendAccessibilityEvent:(ReactTag)reactTag eventType:(NSString *)eventTyp
});
}

- (void)initiateTransaction:(MountingCoordinator::Shared)mountingCoordinator
- (void)initiateTransaction:(MountingCoordinator const &)mountingCoordinator
{
SystraceSection s("-[RCTMountingManager initiateTransaction:]");
RCTAssertMainQueue();
Expand All @@ -261,14 +261,14 @@ - (void)initiateTransaction:(MountingCoordinator::Shared)mountingCoordinator
} while (_followUpTransactionRequired);
}

- (void)performTransaction:(MountingCoordinator::Shared const &)mountingCoordinator
- (void)performTransaction:(MountingCoordinator const &)mountingCoordinator
{
SystraceSection s("-[RCTMountingManager performTransaction:]");
RCTAssertMainQueue();

auto surfaceId = mountingCoordinator->getSurfaceId();
auto surfaceId = mountingCoordinator.getSurfaceId();

mountingCoordinator->getTelemetryController().pullTransaction(
mountingCoordinator.getTelemetryController().pullTransaction(
[&](MountingTransaction const &transaction, SurfaceTelemetry const &surfaceTelemetry) {
[self.delegate mountingManager:self willMountComponentsWithRootTag:surfaceId];
_observerCoordinator.notifyObserversMountingTransactionWillMount(transaction, surfaceTelemetry);
Expand Down
2 changes: 1 addition & 1 deletion React/Fabric/RCTScheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ NS_ASSUME_NONNULL_BEGIN
*/
@protocol RCTSchedulerDelegate

- (void)schedulerDidFinishTransaction:(facebook::react::MountingCoordinator::Shared const &)mountingCoordinator;
- (void)schedulerDidFinishTransaction:(facebook::react::MountingCoordinator::Shared)mountingCoordinator;

- (void)schedulerDidDispatchCommand:(facebook::react::ShadowView const &)shadowView
commandName:(std::string const &)commandName
Expand Down
2 changes: 1 addition & 1 deletion React/Fabric/RCTSurfacePresenter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ - (void)_applicationWillTerminate

#pragma mark - RCTSchedulerDelegate

- (void)schedulerDidFinishTransaction:(MountingCoordinator::Shared const &)mountingCoordinator
- (void)schedulerDidFinishTransaction:(MountingCoordinator::Shared)mountingCoordinator
{
[_mountingManager scheduleTransaction:mountingCoordinator];
}
Expand Down
1 change: 1 addition & 0 deletions ReactCommon/react/renderer/core/CoreFeatures.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ namespace react {

bool CoreFeatures::enablePropIteratorSetter = false;
bool CoreFeatures::enableMapBuffer = false;
bool CoreFeatures::blockPaintForUseLayoutEffect = false;

} // namespace react
} // namespace facebook
5 changes: 5 additions & 0 deletions ReactCommon/react/renderer/core/CoreFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ class CoreFeatures {
// its ShadowNode traits must set the MapBuffer trait; and this
// must be set to "true" globally.
static bool enableMapBuffer;

// When enabled, Fabric will block paint to allow for state updates in
// useLayoutEffect hooks to be processed. This changes affects scheduling of
// when a transaction is mounted.
static bool blockPaintForUseLayoutEffect;
};

} // namespace react
Expand Down
13 changes: 8 additions & 5 deletions ReactCommon/react/renderer/mounting/ShadowTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ void ShadowTree::setCommitMode(CommitMode commitMode) const {
// initial revision never contains any commits so mounting it here is
// incorrect
if (commitMode == CommitMode::Normal && revision.number != INITIAL_REVISION) {
mount(revision);
mount(revision, true);
}
}

Expand Down Expand Up @@ -402,7 +402,7 @@ CommitStatus ShadowTree::tryCommit(
emitLayoutEvents(affectedLayoutableNodes);

if (commitMode == CommitMode::Normal) {
mount(newRevision);
mount(newRevision, commitOptions.mountSynchronously);
}

return CommitStatus::Succeeded;
Expand All @@ -413,9 +413,12 @@ ShadowTreeRevision ShadowTree::getCurrentRevision() const {
return currentRevision_;
}

void ShadowTree::mount(ShadowTreeRevision const &revision) const {
void ShadowTree::mount(
ShadowTreeRevision const &revision,
bool mountSynchronously) const {
mountingCoordinator_->push(revision);
delegate_.shadowTreeDidFinishTransaction(mountingCoordinator_);
delegate_.shadowTreeDidFinishTransaction(
mountingCoordinator_, mountSynchronously);
}

void ShadowTree::commitEmptyTree() const {
Expand Down Expand Up @@ -458,7 +461,7 @@ void ShadowTree::emitLayoutEvents(
}

void ShadowTree::notifyDelegatesOfUpdates() const {
delegate_.shadowTreeDidFinishTransaction(mountingCoordinator_);
delegate_.shadowTreeDidFinishTransaction(mountingCoordinator_, true);
}

} // namespace facebook::react
9 changes: 8 additions & 1 deletion ReactCommon/react/renderer/mounting/ShadowTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ class ShadowTree final {
struct CommitOptions {
bool enableStateReconciliation{false};

// Indicates if mounting will be triggered synchronously and React will
// not get a chance to interrupt painting.
// This should be set to `false` when a commit is comming from React. It
// will then let React run layout effects and apply updates before paint.
// For all other commits, should be true.
bool mountSynchronously{true};

// Called during `tryCommit` phase. Returning true indicates current commit
// should yield to the next commit.
std::function<bool()> shouldYield;
Expand Down Expand Up @@ -128,7 +135,7 @@ class ShadowTree final {
private:
constexpr static ShadowTreeRevision::Number INITIAL_REVISION{0};

void mount(ShadowTreeRevision const &revision) const;
void mount(ShadowTreeRevision const &revision, bool mountSynchronously) const;

void emitLayoutEvents(
std::vector<LayoutableShadowNode const *> &affectedLayoutableNodes) const;
Expand Down
3 changes: 2 additions & 1 deletion ReactCommon/react/renderer/mounting/ShadowTreeDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ class ShadowTreeDelegate {
* Called right after Shadow Tree commit a new state of the tree.
*/
virtual void shadowTreeDidFinishTransaction(
MountingCoordinator::Shared mountingCoordinator) const = 0;
MountingCoordinator::Shared mountingCoordinator,
bool mountSynchronously) const = 0;

virtual ~ShadowTreeDelegate() noexcept = default;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class DummyShadowTreeDelegate : public ShadowTreeDelegate {
};

void shadowTreeDidFinishTransaction(
MountingCoordinator::Shared mountingCoordinator) const override{};
MountingCoordinator::Shared mountingCoordinator,
bool mountSynchronously) const override{};
};

inline ShadowNode const *findDescendantNode(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void RuntimeScheduler::scheduleWork(RawCallback callback) const {
std::shared_ptr<Task> RuntimeScheduler::scheduleTask(
SchedulerPriority priority,
jsi::Function callback) {
auto expirationTime = now() + timeoutForSchedulerPriority(priority);
auto expirationTime = now_() + timeoutForSchedulerPriority(priority);
auto task =
std::make_shared<Task>(priority, std::move(callback), expirationTime);
taskQueue_.push(task);
Expand All @@ -47,7 +47,7 @@ std::shared_ptr<Task> RuntimeScheduler::scheduleTask(
std::shared_ptr<Task> RuntimeScheduler::scheduleTask(
SchedulerPriority priority,
RawCallback callback) {
auto expirationTime = now() + timeoutForSchedulerPriority(priority);
auto expirationTime = now_() + timeoutForSchedulerPriority(priority);
auto task =
std::make_shared<Task>(priority, std::move(callback), expirationTime);
taskQueue_.push(task);
Expand Down
30 changes: 28 additions & 2 deletions ReactCommon/react/renderer/scheduler/Scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ Scheduler::Scheduler(
reduceDeleteCreateMutationLayoutAnimation_ = true;
#endif

CoreFeatures::blockPaintForUseLayoutEffect = reactNativeConfig_->getBool(
"react_fabric:block_paint_for_use_layout_effect");

if (animationDelegate != nullptr) {
animationDelegate->setComponentDescriptorRegistry(
componentDescriptorRegistry_);
Expand Down Expand Up @@ -299,11 +302,34 @@ void Scheduler::animationTick() const {
#pragma mark - UIManagerDelegate

void Scheduler::uiManagerDidFinishTransaction(
MountingCoordinator::Shared mountingCoordinator) {
MountingCoordinator::Shared mountingCoordinator,
bool mountSynchronously) {
SystraceSection s("Scheduler::uiManagerDidFinishTransaction");

if (delegate_ != nullptr) {
delegate_->schedulerDidFinishTransaction(std::move(mountingCoordinator));
if (CoreFeatures::blockPaintForUseLayoutEffect) {
auto weakRuntimeScheduler =
contextContainer_->find<std::weak_ptr<RuntimeScheduler>>(
"RuntimeScheduler");
auto runtimeScheduler = weakRuntimeScheduler.has_value()
? weakRuntimeScheduler.value().lock()
: nullptr;
if (runtimeScheduler && !mountSynchronously) {
runtimeScheduler->scheduleTask(
SchedulerPriority::UserBlockingPriority,
[delegate = delegate_,
mountingCoordinator =
std::move(mountingCoordinator)](jsi::Runtime &) {
delegate->schedulerDidFinishTransaction(
std::move(mountingCoordinator));
});
} else {
delegate_->schedulerDidFinishTransaction(
std::move(mountingCoordinator));
}
} else {
delegate_->schedulerDidFinishTransaction(std::move(mountingCoordinator));
}
}
}
void Scheduler::uiManagerDidCreateShadowNode(const ShadowNode &shadowNode) {
Expand Down
3 changes: 2 additions & 1 deletion ReactCommon/react/renderer/scheduler/Scheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ class Scheduler final : public UIManagerDelegate {
#pragma mark - UIManagerDelegate

void uiManagerDidFinishTransaction(
MountingCoordinator::Shared mountingCoordinator) override;
MountingCoordinator::Shared mountingCoordinator,
bool mountSynchronously) override;
void uiManagerDidCreateShadowNode(const ShadowNode &shadowNode) override;
void uiManagerDidDispatchCommand(
const ShadowNode::Shared &shadowNode,
Expand Down
6 changes: 4 additions & 2 deletions ReactCommon/react/renderer/uimanager/UIManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,11 +532,13 @@ RootShadowNode::Unshared UIManager::shadowTreeWillCommit(
}

void UIManager::shadowTreeDidFinishTransaction(
MountingCoordinator::Shared mountingCoordinator) const {
MountingCoordinator::Shared mountingCoordinator,
bool mountSynchronously) const {
SystraceSection s("UIManager::shadowTreeDidFinishTransaction");

if (delegate_ != nullptr) {
delegate_->uiManagerDidFinishTransaction(std::move(mountingCoordinator));
delegate_->uiManagerDidFinishTransaction(
std::move(mountingCoordinator), mountSynchronously);
}
}

Expand Down
3 changes: 2 additions & 1 deletion ReactCommon/react/renderer/uimanager/UIManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ class UIManager final : public ShadowTreeDelegate {
#pragma mark - ShadowTreeDelegate

void shadowTreeDidFinishTransaction(
MountingCoordinator::Shared mountingCoordinator) const override;
MountingCoordinator::Shared mountingCoordinator,
bool mountSynchronously) const override;

RootShadowNode::Unshared shadowTreeWillCommit(
ShadowTree const &shadowTree,
Expand Down
12 changes: 10 additions & 2 deletions ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,11 @@ jsi::Value UIManagerBinding::get(
auto shadowNodeList =
shadowNodeListFromWeakList(weakShadowNodeList);
if (shadowNodeList) {
uiManager->completeSurface(surfaceId, shadowNodeList, {true});
uiManager->completeSurface(
surfaceId,
shadowNodeList,
{/* .enableStateReconciliation = */ true,
/* .mountSynchronously = */ false});
}
} else {
auto weakShadowNodeList =
Expand All @@ -429,7 +433,11 @@ jsi::Value UIManagerBinding::get(
auto strongUIManager = weakUIManager.lock();
if (shadowNodeList && strongUIManager) {
strongUIManager->completeSurface(
surfaceId, shadowNodeList, {true, shouldYield});
surfaceId,
shadowNodeList,
{/* .enableStateReconciliation = */ true,
/* .mountSynchronously = */ false,
/* .shouldYield = */ shouldYield});
}
});
}
Expand Down
3 changes: 2 additions & 1 deletion ReactCommon/react/renderer/uimanager/UIManagerDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ class UIManagerDelegate {
* For this moment the tree is already laid out and sealed.
*/
virtual void uiManagerDidFinishTransaction(
MountingCoordinator::Shared mountingCoordinator) = 0;
MountingCoordinator::Shared mountingCoordinator,
bool mountSynchronously) = 0;

/*
* Called each time when UIManager constructs a new Shadow Node. Receiver
Expand Down

0 comments on commit afec07a

Please sign in to comment.