Skip to content

Commit f494a77

Browse files
j-piaseckifacebook-github-bot
authored andcommitted
Update branching implementation to default to the JS thread merge (#58615)
Summary: Changelog: [Internal] The current behavior, merge on the main thread, is kept behind the new `enableFabricCommitBranchingMergeOnMainThread` flag. Reviewed By: rubennorte Differential Revision: D120981616
1 parent baee2f3 commit f494a77

14 files changed

Lines changed: 32 additions & 55 deletions

File tree

‎packages/react-native/React/Fabric/RCTSurfacePresenter.mm‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ - (instancetype)initWithContextContainer:(std::shared_ptr<const ContextContainer
101101
_mountingManager.contextContainer = contextContainer;
102102
_mountingManager.delegate = self;
103103

104-
if (ReactNativeFeatureFlags::enableFabricCommitBranching()) {
104+
if (ReactNativeFeatureFlags::enableFabricCommitBranchingMergeOnMainThread()) {
105105
_mergeRunLoopObserverDelegate = std::make_shared<ReactRevisionMergeRunLoopObserverDelegate>(self);
106106
_mergeRunLoopObserver = std::make_unique<const MainRunLoopObserver>(
107107
RunLoopObserver::Activity::BeforeWaiting, _mergeRunLoopObserverDelegate);
@@ -350,7 +350,7 @@ - (void)schedulerShouldRenderTransactions:(std::shared_ptr<const MountingCoordin
350350

351351
- (void)schedulerShouldMergeReactRevision:(SurfaceId)surfaceId
352352
{
353-
if (RCTIsMainQueue()) {
353+
if (RCTIsMainQueue() || !ReactNativeFeatureFlags::enableFabricCommitBranchingMergeOnMainThread()) {
354354
[self _mergeReactRevisionForSurfaceId:surfaceId];
355355
return;
356356
}
@@ -369,7 +369,6 @@ - (void)schedulerShouldMergeReactRevision:(SurfaceId)surfaceId
369369

370370
- (void)_mergeReactRevisionForSurfaceId:(SurfaceId)surfaceId
371371
{
372-
RCTAssertMainQueue();
373372
RCTScheduler *scheduler = [self scheduler];
374373
if (!scheduler) {
375374
return;

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1628,7 +1628,7 @@ public void doFrameGuarded(long frameTimeNanos) {
16281628

16291629
// Drain pending React revision merges first so that animations,
16301630
// preallocation, and mount items operate against the latest revision.
1631-
if (ReactNativeFeatureFlags.enableFabricCommitBranching()) {
1631+
if (ReactNativeFeatureFlags.enableFabricCommitBranchingMergeOnMainThread()) {
16321632
FabricUIManagerBinding binding = mBinding;
16331633
if (binding != null) {
16341634
Integer mergeSurfaceId;

‎packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricUIManagerBinding.cpp‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -767,17 +767,24 @@ void FabricUIManagerBinding::schedulerShouldRenderTransactions(
767767

768768
void FabricUIManagerBinding::schedulerShouldMergeReactRevision(
769769
SurfaceId surfaceId) {
770-
std::shared_lock lock(installMutex_);
771-
auto mountingManager =
772-
getMountingManager("schedulerShouldMergeReactRevision");
773-
if (mountingManager) {
774-
mountingManager->scheduleReactRevisionMerge(surfaceId);
770+
if (ReactNativeFeatureFlags::enableFabricCommitBranchingMergeOnMainThread()) {
771+
auto mountingManager =
772+
getMountingManager("schedulerShouldMergeReactRevision");
773+
if (mountingManager) {
774+
mountingManager->scheduleReactRevisionMerge(surfaceId);
775+
}
776+
} else {
777+
mergeReactRevision(surfaceId);
775778
}
776779
}
777780

778781
void FabricUIManagerBinding::mergeReactRevision(SurfaceId surfaceId) {
779-
std::shared_lock lock(installMutex_);
780-
scheduler_->getUIManager()->getShadowTreeRegistry().visit(
782+
auto scheduler = getScheduler();
783+
if (!scheduler) {
784+
return;
785+
}
786+
787+
scheduler->getUIManager()->getShadowTreeRegistry().visit(
781788
surfaceId,
782789
[](const ShadowTree& shadowTree) { shadowTree.mergeReactRevision(); });
783790
}

‎packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,7 @@ void ShadowTree::mount(ShadowTreeRevision revision, bool mountSynchronously)
492492
}
493493

494494
void ShadowTree::mergeReactRevision() const {
495+
TraceSection s("ShadowTree::mergeReactRevision");
495496
ShadowTreeRevision promotedRevision;
496497
std::vector<ShadowTreeRevision> promotedRevisions;
497498
// If props updates accumulation is guaranteed, we can merge the promoted
@@ -568,7 +569,7 @@ void ShadowTree::mergeReactRevision() const {
568569
}
569570
}
570571

571-
void ShadowTree::promoteReactRevision() const {
572+
bool ShadowTree::promoteReactRevision() const {
572573
// Promote only when props updates accumulation is guaranteed. Otherwise,
573574
// queuedReactRevisions_ will be used instead.
574575
if (isPropsUpdatesAccumulationGuaranteed()) {
@@ -579,7 +580,7 @@ void ShadowTree::promoteReactRevision() const {
579580
// have more than one promotion in a row. In this case, all but the first
580581
// one should no-op.
581582
if (!currentReactRevision_.has_value()) {
582-
return;
583+
return false;
583584
}
584585
currentReactRevision = currentReactRevision_.value();
585586
}
@@ -592,7 +593,7 @@ void ShadowTree::promoteReactRevision() const {
592593
UniqueLock lock = uniqueRevisionLock(false);
593594

594595
if (queuedReactRevisions_.empty()) {
595-
return;
596+
return false;
596597
}
597598

598599
// Move all queued revisions to the promoted revisions.
@@ -603,7 +604,7 @@ void ShadowTree::promoteReactRevision() const {
603604
queuedReactRevisions_.clear();
604605
}
605606

606-
delegate_.shadowTreeDidPromoteReactRevision(*this);
607+
return true;
607608
}
608609

609610
void ShadowTree::scheduleReactRevisionPromotion() const {

‎packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,9 @@ class ShadowTree final {
148148

149149
/**
150150
* Promotes the current React revision to be merged into the main branch of the
151-
* ShadowTree.
151+
* ShadowTree. Returns `true` if a revision was promoted.
152152
*/
153-
void promoteReactRevision() const;
153+
bool promoteReactRevision() const;
154154

155155
/**
156156
* Commits the currently promoted React revision to the "main" branch of the

‎packages/react-native/ReactCommon/react/renderer/mounting/ShadowTreeDelegate.h‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,6 @@ class ShadowTreeDelegate {
4646
*/
4747
virtual void shadowTreeDidFinishReactCommit(const ShadowTree &shadowTree) const = 0;
4848

49-
/*
50-
* Called right after Shadow Tree promotes a React revision of the tree to
51-
* be merged.
52-
*/
53-
virtual void shadowTreeDidPromoteReactRevision(const ShadowTree &shadowTree) const = 0;
54-
5549
/*
5650
* Called right after a Shadow Tree commits a new tree, reporting the nodes
5751
* whose layout changed in this commit.

‎packages/react-native/ReactCommon/react/renderer/mounting/tests/ShadowTreeReactBranchingTest.cpp‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,6 @@ class DummyShadowTreeDelegate : public ShadowTreeDelegate {
4646

4747
void shadowTreeDidFinishReactCommit(
4848
const ShadowTree& /*shadowTree*/) const override {}
49-
50-
void shadowTreeDidPromoteReactRevision(
51-
const ShadowTree& /*shadowTree*/) const override {}
5249
};
5350

5451
} // namespace

‎packages/react-native/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,6 @@ class DummyShadowTreeDelegate : public ShadowTreeDelegate {
4040

4141
void shadowTreeDidFinishReactCommit(
4242
const ShadowTree& /*shadowTree*/) const override {}
43-
44-
void shadowTreeDidPromoteReactRevision(
45-
const ShadowTree& /*shadowTree*/) const override {}
4643
};
4744

4845
namespace {

‎packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -419,17 +419,17 @@ void Scheduler::uiManagerShouldRemoveEventListener(
419419
void Scheduler::uiManagerDidFinishReactCommit(const ShadowTree& shadowTree) {
420420
auto surfaceId = shadowTree.getSurfaceId();
421421
runtimeScheduler_->scheduleRenderingUpdate(
422-
surfaceId, [surfaceId, uiManager = uiManager_]() {
422+
surfaceId, [surfaceId, uiManager = uiManager_, this]() {
423+
bool promoted = false;
424+
423425
uiManager->getShadowTreeRegistry().visit(
424426
surfaceId,
425-
[](const ShadowTree& tree) { tree.promoteReactRevision(); });
426-
});
427-
}
427+
[&](const ShadowTree& tree) { promoted = tree.promoteReactRevision(); });
428428

429-
void Scheduler::uiManagerDidPromoteReactRevision(const ShadowTree& shadowTree) {
430-
if (delegate_ != nullptr) {
431-
delegate_->schedulerShouldMergeReactRevision(shadowTree.getSurfaceId());
432-
}
429+
if (promoted && delegate_ != nullptr) {
430+
delegate_->schedulerShouldMergeReactRevision(surfaceId);
431+
}
432+
});
433433
}
434434

435435
void Scheduler::uiManagerDidStartSurface(const ShadowTree& shadowTree) {

‎packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ class Scheduler final : public UIManagerDelegate {
107107
void uiManagerShouldAddEventListener(std::shared_ptr<const EventListener> listener) final;
108108
void uiManagerShouldRemoveEventListener(const std::shared_ptr<const EventListener> &listener) final;
109109
void uiManagerDidFinishReactCommit(const ShadowTree &shadowTree) override;
110-
void uiManagerDidPromoteReactRevision(const ShadowTree &shadowTree) override;
111110
void uiManagerDidStartSurface(const ShadowTree &shadowTree) override;
112111

113112
#pragma mark - ContextContainer

0 commit comments

Comments
 (0)