Skip to content

Commit afec07a

Browse files
sammy-SCfacebook-github-bot
authored andcommitted
Add option to make commit asynchronous
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
1 parent 7535399 commit afec07a

File tree

16 files changed

+84
-27
lines changed

16 files changed

+84
-27
lines changed

React/Fabric/Mounting/RCTMountingManager.mm

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -203,13 +203,13 @@ - (void)scheduleTransaction:(MountingCoordinator::Shared)mountingCoordinator
203203
// * No need to do a thread jump;
204204
// * No need to do expensive copy of all mutations;
205205
// * No need to allocate a block.
206-
[self initiateTransaction:std::move(mountingCoordinator)];
206+
[self initiateTransaction:*mountingCoordinator];
207207
return;
208208
}
209209

210210
RCTExecuteOnMainQueue(^{
211211
RCTAssertMainQueue();
212-
[self initiateTransaction:std::move(mountingCoordinator)];
212+
[self initiateTransaction:*mountingCoordinator];
213213
});
214214
}
215215

@@ -243,7 +243,7 @@ - (void)sendAccessibilityEvent:(ReactTag)reactTag eventType:(NSString *)eventTyp
243243
});
244244
}
245245

246-
- (void)initiateTransaction:(MountingCoordinator::Shared)mountingCoordinator
246+
- (void)initiateTransaction:(MountingCoordinator const &)mountingCoordinator
247247
{
248248
SystraceSection s("-[RCTMountingManager initiateTransaction:]");
249249
RCTAssertMainQueue();
@@ -261,14 +261,14 @@ - (void)initiateTransaction:(MountingCoordinator::Shared)mountingCoordinator
261261
} while (_followUpTransactionRequired);
262262
}
263263

264-
- (void)performTransaction:(MountingCoordinator::Shared const &)mountingCoordinator
264+
- (void)performTransaction:(MountingCoordinator const &)mountingCoordinator
265265
{
266266
SystraceSection s("-[RCTMountingManager performTransaction:]");
267267
RCTAssertMainQueue();
268268

269-
auto surfaceId = mountingCoordinator->getSurfaceId();
269+
auto surfaceId = mountingCoordinator.getSurfaceId();
270270

271-
mountingCoordinator->getTelemetryController().pullTransaction(
271+
mountingCoordinator.getTelemetryController().pullTransaction(
272272
[&](MountingTransaction const &transaction, SurfaceTelemetry const &surfaceTelemetry) {
273273
[self.delegate mountingManager:self willMountComponentsWithRootTag:surfaceId];
274274
_observerCoordinator.notifyObserversMountingTransactionWillMount(transaction, surfaceTelemetry);

React/Fabric/RCTScheduler.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ NS_ASSUME_NONNULL_BEGIN
2828
*/
2929
@protocol RCTSchedulerDelegate
3030

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

3333
- (void)schedulerDidDispatchCommand:(facebook::react::ShadowView const &)shadowView
3434
commandName:(std::string const &)commandName

React/Fabric/RCTSurfacePresenter.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ - (void)_applicationWillTerminate
356356

357357
#pragma mark - RCTSchedulerDelegate
358358

359-
- (void)schedulerDidFinishTransaction:(MountingCoordinator::Shared const &)mountingCoordinator
359+
- (void)schedulerDidFinishTransaction:(MountingCoordinator::Shared)mountingCoordinator
360360
{
361361
[_mountingManager scheduleTransaction:mountingCoordinator];
362362
}

ReactCommon/react/renderer/core/CoreFeatures.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ namespace react {
1212

1313
bool CoreFeatures::enablePropIteratorSetter = false;
1414
bool CoreFeatures::enableMapBuffer = false;
15+
bool CoreFeatures::blockPaintForUseLayoutEffect = false;
1516

1617
} // namespace react
1718
} // namespace facebook

ReactCommon/react/renderer/core/CoreFeatures.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ class CoreFeatures {
2525
// its ShadowNode traits must set the MapBuffer trait; and this
2626
// must be set to "true" globally.
2727
static bool enableMapBuffer;
28+
29+
// When enabled, Fabric will block paint to allow for state updates in
30+
// useLayoutEffect hooks to be processed. This changes affects scheduling of
31+
// when a transaction is mounted.
32+
static bool blockPaintForUseLayoutEffect;
2833
};
2934

3035
} // namespace react

ReactCommon/react/renderer/mounting/ShadowTree.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ void ShadowTree::setCommitMode(CommitMode commitMode) const {
281281
// initial revision never contains any commits so mounting it here is
282282
// incorrect
283283
if (commitMode == CommitMode::Normal && revision.number != INITIAL_REVISION) {
284-
mount(revision);
284+
mount(revision, true);
285285
}
286286
}
287287

@@ -402,7 +402,7 @@ CommitStatus ShadowTree::tryCommit(
402402
emitLayoutEvents(affectedLayoutableNodes);
403403

404404
if (commitMode == CommitMode::Normal) {
405-
mount(newRevision);
405+
mount(newRevision, commitOptions.mountSynchronously);
406406
}
407407

408408
return CommitStatus::Succeeded;
@@ -413,9 +413,12 @@ ShadowTreeRevision ShadowTree::getCurrentRevision() const {
413413
return currentRevision_;
414414
}
415415

416-
void ShadowTree::mount(ShadowTreeRevision const &revision) const {
416+
void ShadowTree::mount(
417+
ShadowTreeRevision const &revision,
418+
bool mountSynchronously) const {
417419
mountingCoordinator_->push(revision);
418-
delegate_.shadowTreeDidFinishTransaction(mountingCoordinator_);
420+
delegate_.shadowTreeDidFinishTransaction(
421+
mountingCoordinator_, mountSynchronously);
419422
}
420423

421424
void ShadowTree::commitEmptyTree() const {
@@ -458,7 +461,7 @@ void ShadowTree::emitLayoutEvents(
458461
}
459462

460463
void ShadowTree::notifyDelegatesOfUpdates() const {
461-
delegate_.shadowTreeDidFinishTransaction(mountingCoordinator_);
464+
delegate_.shadowTreeDidFinishTransaction(mountingCoordinator_, true);
462465
}
463466

464467
} // namespace facebook::react

ReactCommon/react/renderer/mounting/ShadowTree.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ class ShadowTree final {
5959
struct CommitOptions {
6060
bool enableStateReconciliation{false};
6161

62+
// Indicates if mounting will be triggered synchronously and React will
63+
// not get a chance to interrupt painting.
64+
// This should be set to `false` when a commit is comming from React. It
65+
// will then let React run layout effects and apply updates before paint.
66+
// For all other commits, should be true.
67+
bool mountSynchronously{true};
68+
6269
// Called during `tryCommit` phase. Returning true indicates current commit
6370
// should yield to the next commit.
6471
std::function<bool()> shouldYield;
@@ -128,7 +135,7 @@ class ShadowTree final {
128135
private:
129136
constexpr static ShadowTreeRevision::Number INITIAL_REVISION{0};
130137

131-
void mount(ShadowTreeRevision const &revision) const;
138+
void mount(ShadowTreeRevision const &revision, bool mountSynchronously) const;
132139

133140
void emitLayoutEvents(
134141
std::vector<LayoutableShadowNode const *> &affectedLayoutableNodes) const;

ReactCommon/react/renderer/mounting/ShadowTreeDelegate.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ class ShadowTreeDelegate {
3434
* Called right after Shadow Tree commit a new state of the tree.
3535
*/
3636
virtual void shadowTreeDidFinishTransaction(
37-
MountingCoordinator::Shared mountingCoordinator) const = 0;
37+
MountingCoordinator::Shared mountingCoordinator,
38+
bool mountSynchronously) const = 0;
3839

3940
virtual ~ShadowTreeDelegate() noexcept = default;
4041
};

ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ class DummyShadowTreeDelegate : public ShadowTreeDelegate {
3232
};
3333

3434
void shadowTreeDidFinishTransaction(
35-
MountingCoordinator::Shared mountingCoordinator) const override{};
35+
MountingCoordinator::Shared mountingCoordinator,
36+
bool mountSynchronously) const override{};
3637
};
3738

3839
inline ShadowNode const *findDescendantNode(

ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ void RuntimeScheduler::scheduleWork(RawCallback callback) const {
3434
std::shared_ptr<Task> RuntimeScheduler::scheduleTask(
3535
SchedulerPriority priority,
3636
jsi::Function callback) {
37-
auto expirationTime = now() + timeoutForSchedulerPriority(priority);
37+
auto expirationTime = now_() + timeoutForSchedulerPriority(priority);
3838
auto task =
3939
std::make_shared<Task>(priority, std::move(callback), expirationTime);
4040
taskQueue_.push(task);
@@ -47,7 +47,7 @@ std::shared_ptr<Task> RuntimeScheduler::scheduleTask(
4747
std::shared_ptr<Task> RuntimeScheduler::scheduleTask(
4848
SchedulerPriority priority,
4949
RawCallback callback) {
50-
auto expirationTime = now() + timeoutForSchedulerPriority(priority);
50+
auto expirationTime = now_() + timeoutForSchedulerPriority(priority);
5151
auto task =
5252
std::make_shared<Task>(priority, std::move(callback), expirationTime);
5353
taskQueue_.push(task);

0 commit comments

Comments
 (0)