Skip to content

Commit 849da21

Browse files
javachefacebook-github-bot
authored andcommitted
Split scheduler commit and flush delegate methods (facebook#44188)
Summary: Pull Request resolved: facebook#44188 The current approach used for `batchRenderingUpdatesInEventLoop` is not compatible with Android due to limitations in its props processing model. The raw props changeset is passed through to Android, and must be available for the Android mounting layer to correctly apply changes. We have some logic to merge these payloads when multiple ShadowNode clones take place but were previously assuming that a ShadowTree commit was a safe state to synchronize. In the current implementation this means that two commits driven from layout effects (triggering states A → B → C) may cause Android to observe only the B → C props change, and miss out on any props changed in A → B. Changelog: [Android][Fixed] Cascading renders were not mounting correctly when `batchRenderingUpdatesInEventLoop` is enabled. Reviewed By: rubennorte Differential Revision: D56414689 fbshipit-source-id: 7c74d81620db0f8b7bd67e640168afc795c7a1d7
1 parent 9d1f951 commit 849da21

28 files changed

+236
-40
lines changed

packages/react-native/React/Fabric/RCTScheduler.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ NS_ASSUME_NONNULL_BEGIN
2828

2929
- (void)schedulerDidFinishTransaction:(facebook::react::MountingCoordinator::Shared)mountingCoordinator;
3030

31+
- (void)schedulerShouldRenderTransactions:(facebook::react::MountingCoordinator::Shared)mountingCoordinator;
32+
3133
- (void)schedulerDidDispatchCommand:(const facebook::react::ShadowView &)shadowView
3234
commandName:(const std::string &)commandName
3335
args:(const folly::dynamic &)args;

packages/react-native/React/Fabric/RCTScheduler.mm

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ void schedulerDidFinishTransaction(const MountingCoordinator::Shared &mountingCo
3131
[scheduler.delegate schedulerDidFinishTransaction:mountingCoordinator];
3232
}
3333

34+
void schedulerShouldRenderTransactions(const MountingCoordinator::Shared &mountingCoordinator) override
35+
{
36+
RCTScheduler *scheduler = (__bridge RCTScheduler *)scheduler_;
37+
[scheduler.delegate schedulerShouldRenderTransactions:mountingCoordinator];
38+
}
39+
3440
void schedulerDidRequestPreliminaryViewAllocation(SurfaceId surfaceId, const ShadowNode &shadowNode) override
3541
{
3642
// Does nothing.

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,11 @@ - (void)_applicationWillTerminate
333333
#pragma mark - RCTSchedulerDelegate
334334

335335
- (void)schedulerDidFinishTransaction:(MountingCoordinator::Shared)mountingCoordinator
336+
{
337+
// no-op, we will flush the transaction from schedulerShouldRenderTransactions
338+
}
339+
340+
- (void)schedulerShouldRenderTransactions:(MountingCoordinator::Shared)mountingCoordinator
336341
{
337342
[_mountingManager scheduleTransaction:mountingCoordinator];
338343
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<cf91b15910f94812cc0d4628fea91f97>>
7+
* @generated SignedSource<<f3d26e8e345068bc7ef4e156be460e09>>
88
*/
99

1010
/**
@@ -34,6 +34,12 @@ public object ReactNativeFeatureFlags {
3434
@JvmStatic
3535
public fun commonTestFlag(): Boolean = accessor.commonTestFlag()
3636

37+
/**
38+
* To be used with batchRenderingUpdatesInEventLoop. When enbled, the Android mounting layer will concatenate pending transactions to ensure they're applied atomatically
39+
*/
40+
@JvmStatic
41+
public fun androidEnablePendingFabricTransactions(): Boolean = accessor.androidEnablePendingFabricTransactions()
42+
3743
/**
3844
* When enabled, the RuntimeScheduler processing the event loop will batch all rendering updates and dispatch them together at the end of each iteration of the loop.
3945
*/

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<d23f1c4cd3f8960397455c495ed240ba>>
7+
* @generated SignedSource<<40668dcd951123da7c0b4ddde23f94c9>>
88
*/
99

1010
/**
@@ -21,6 +21,7 @@ package com.facebook.react.internal.featureflags
2121

2222
public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccessor {
2323
private var commonTestFlagCache: Boolean? = null
24+
private var androidEnablePendingFabricTransactionsCache: Boolean? = null
2425
private var batchRenderingUpdatesInEventLoopCache: Boolean? = null
2526
private var destroyFabricSurfacesInReactInstanceManagerCache: Boolean? = null
2627
private var enableBackgroundExecutorCache: Boolean? = null
@@ -47,6 +48,15 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
4748
return cached
4849
}
4950

51+
override fun androidEnablePendingFabricTransactions(): Boolean {
52+
var cached = androidEnablePendingFabricTransactionsCache
53+
if (cached == null) {
54+
cached = ReactNativeFeatureFlagsCxxInterop.androidEnablePendingFabricTransactions()
55+
androidEnablePendingFabricTransactionsCache = cached
56+
}
57+
return cached
58+
}
59+
5060
override fun batchRenderingUpdatesInEventLoop(): Boolean {
5161
var cached = batchRenderingUpdatesInEventLoopCache
5262
if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<4470db3808c33009365864c597ceaf16>>
7+
* @generated SignedSource<<c3f02dff409c10aa7922141cef3a6990>>
88
*/
99

1010
/**
@@ -30,6 +30,8 @@ public object ReactNativeFeatureFlagsCxxInterop {
3030

3131
@DoNotStrip @JvmStatic public external fun commonTestFlag(): Boolean
3232

33+
@DoNotStrip @JvmStatic public external fun androidEnablePendingFabricTransactions(): Boolean
34+
3335
@DoNotStrip @JvmStatic public external fun batchRenderingUpdatesInEventLoop(): Boolean
3436

3537
@DoNotStrip @JvmStatic public external fun destroyFabricSurfacesInReactInstanceManager(): Boolean

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<f9ed3dc67e050bc741efd3dc82f4b62c>>
7+
* @generated SignedSource<<d12888990ebca7c6199f4b51802ee59b>>
88
*/
99

1010
/**
@@ -25,6 +25,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi
2525

2626
override fun commonTestFlag(): Boolean = false
2727

28+
override fun androidEnablePendingFabricTransactions(): Boolean = false
29+
2830
override fun batchRenderingUpdatesInEventLoop(): Boolean = false
2931

3032
override fun destroyFabricSurfacesInReactInstanceManager(): Boolean = false

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<17d6ac9c31290ac19caa64d87ce7215b>>
7+
* @generated SignedSource<<c06b3b34cea24459f6ade0ec5665dae7>>
88
*/
99

1010
/**
@@ -25,6 +25,7 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
2525
private val accessedFeatureFlags = mutableSetOf<String>()
2626

2727
private var commonTestFlagCache: Boolean? = null
28+
private var androidEnablePendingFabricTransactionsCache: Boolean? = null
2829
private var batchRenderingUpdatesInEventLoopCache: Boolean? = null
2930
private var destroyFabricSurfacesInReactInstanceManagerCache: Boolean? = null
3031
private var enableBackgroundExecutorCache: Boolean? = null
@@ -52,6 +53,16 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
5253
return cached
5354
}
5455

56+
override fun androidEnablePendingFabricTransactions(): Boolean {
57+
var cached = androidEnablePendingFabricTransactionsCache
58+
if (cached == null) {
59+
cached = currentProvider.androidEnablePendingFabricTransactions()
60+
accessedFeatureFlags.add("androidEnablePendingFabricTransactions")
61+
androidEnablePendingFabricTransactionsCache = cached
62+
}
63+
return cached
64+
}
65+
5566
override fun batchRenderingUpdatesInEventLoop(): Boolean {
5667
var cached = batchRenderingUpdatesInEventLoopCache
5768
if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<5ab1ec5a95d9fd6b66b30c38c7f8f199>>
7+
* @generated SignedSource<<dceb20e62a9ddbb98c872a24fab9765c>>
88
*/
99

1010
/**
@@ -25,6 +25,8 @@ import com.facebook.proguard.annotations.DoNotStrip
2525
public interface ReactNativeFeatureFlagsProvider {
2626
@DoNotStrip public fun commonTestFlag(): Boolean
2727

28+
@DoNotStrip public fun androidEnablePendingFabricTransactions(): Boolean
29+
2830
@DoNotStrip public fun batchRenderingUpdatesInEventLoop(): Boolean
2931

3032
@DoNotStrip public fun destroyFabricSurfacesInReactInstanceManager(): Boolean

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

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,16 +462,51 @@ std::shared_ptr<FabricMountingManager> Binding::getMountingManager(
462462

463463
void Binding::schedulerDidFinishTransaction(
464464
const MountingCoordinator::Shared& mountingCoordinator) {
465-
auto mountingManager = getMountingManager("schedulerDidFinishTransaction");
466-
if (!mountingManager) {
465+
if (!ReactNativeFeatureFlags::androidEnablePendingFabricTransactions()) {
467466
return;
468467
}
469468

470469
auto mountingTransaction = mountingCoordinator->pullTransaction();
471470
if (!mountingTransaction.has_value()) {
472471
return;
473472
}
474-
mountingManager->executeMount(*mountingTransaction);
473+
474+
std::unique_lock<std::mutex> lock(pendingTransactionsMutex_);
475+
auto pendingTransaction = std::find_if(
476+
pendingTransactions_.begin(),
477+
pendingTransactions_.end(),
478+
[&](const auto& transaction) {
479+
return transaction.getSurfaceId() ==
480+
mountingTransaction->getSurfaceId();
481+
});
482+
483+
if (pendingTransaction != pendingTransactions_.end()) {
484+
pendingTransaction->mergeWith(std::move(*mountingTransaction));
485+
} else {
486+
pendingTransactions_.push_back(std::move(*mountingTransaction));
487+
}
488+
}
489+
490+
void Binding::schedulerShouldRenderTransactions(
491+
const MountingCoordinator::Shared& mountingCoordinator) {
492+
auto mountingManager =
493+
getMountingManager("schedulerShouldRenderTransactions");
494+
if (!mountingManager) {
495+
return;
496+
}
497+
498+
if (ReactNativeFeatureFlags::androidEnablePendingFabricTransactions()) {
499+
std::unique_lock<std::mutex> lock(pendingTransactionsMutex_);
500+
for (auto& transaction : pendingTransactions_) {
501+
mountingManager->executeMount(transaction);
502+
}
503+
pendingTransactions_.clear();
504+
} else {
505+
auto mountingTransaction = mountingCoordinator->pullTransaction();
506+
if (mountingTransaction.has_value()) {
507+
mountingManager->executeMount(*mountingTransaction);
508+
}
509+
}
475510
}
476511

477512
void Binding::schedulerDidRequestPreliminaryViewAllocation(

0 commit comments

Comments
 (0)