Skip to content

Commit

Permalink
Split scheduler commit and flush delegate methods (#44188)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #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
  • Loading branch information
javache authored and cipolleschi committed Apr 30, 2024
1 parent b440672 commit ff4b20e
Show file tree
Hide file tree
Showing 28 changed files with 572 additions and 36 deletions.
2 changes: 2 additions & 0 deletions packages/react-native/React/Fabric/RCTScheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ NS_ASSUME_NONNULL_BEGIN

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

- (void)schedulerShouldRenderTransactions:(facebook::react::MountingCoordinator::Shared)mountingCoordinator;

- (void)schedulerDidDispatchCommand:(const facebook::react::ShadowView &)shadowView
commandName:(const std::string &)commandName
args:(const folly::dynamic &)args;
Expand Down
6 changes: 6 additions & 0 deletions packages/react-native/React/Fabric/RCTScheduler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ void schedulerDidFinishTransaction(const MountingCoordinator::Shared &mountingCo
[scheduler.delegate schedulerDidFinishTransaction:mountingCoordinator];
}

void schedulerShouldRenderTransactions(const MountingCoordinator::Shared &mountingCoordinator) override
{
RCTScheduler *scheduler = (__bridge RCTScheduler *)scheduler_;
[scheduler.delegate schedulerShouldRenderTransactions:mountingCoordinator];
}

void schedulerDidRequestPreliminaryViewAllocation(SurfaceId surfaceId, const ShadowNode &shadowNode) override
{
// Does nothing.
Expand Down
5 changes: 5 additions & 0 deletions packages/react-native/React/Fabric/RCTSurfacePresenter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,11 @@ - (void)_applicationWillTerminate
#pragma mark - RCTSchedulerDelegate

- (void)schedulerDidFinishTransaction:(MountingCoordinator::Shared)mountingCoordinator
{
// no-op, we will flush the transaction from schedulerShouldRenderTransactions
}

- (void)schedulerShouldRenderTransactions:(MountingCoordinator::Shared)mountingCoordinator
{
[_mountingManager scheduleTransaction:mountingCoordinator];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<c526fb1c44f00f5b032684396246e4d4>>
* @generated SignedSource<<f3d26e8e345068bc7ef4e156be460e09>>
*/

/**
Expand Down Expand Up @@ -34,6 +34,24 @@ public object ReactNativeFeatureFlags {
@JvmStatic
public fun commonTestFlag(): Boolean = accessor.commonTestFlag()

/**
* To be used with batchRenderingUpdatesInEventLoop. When enbled, the Android mounting layer will concatenate pending transactions to ensure they're applied atomatically
*/
@JvmStatic
public fun androidEnablePendingFabricTransactions(): Boolean = accessor.androidEnablePendingFabricTransactions()

/**
* 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.
*/
@JvmStatic
public fun batchRenderingUpdatesInEventLoop(): Boolean = accessor.batchRenderingUpdatesInEventLoop()

/**
* When enabled, ReactInstanceManager will clean up Fabric surfaces on destroy().
*/
@JvmStatic
public fun destroyFabricSurfacesInReactInstanceManager(): Boolean = accessor.destroyFabricSurfacesInReactInstanceManager()

/**
* Enables the use of a background executor to compute layout and commit updates on Fabric (this system is deprecated and should not be used).
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<11824621ee7ca5dbdf2f09bdf1a1f983>>
* @generated SignedSource<<40668dcd951123da7c0b4ddde23f94c9>>
*/

/**
Expand All @@ -21,6 +21,9 @@ package com.facebook.react.internal.featureflags

public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccessor {
private var commonTestFlagCache: Boolean? = null
private var androidEnablePendingFabricTransactionsCache: Boolean? = null
private var batchRenderingUpdatesInEventLoopCache: Boolean? = null
private var destroyFabricSurfacesInReactInstanceManagerCache: Boolean? = null
private var enableBackgroundExecutorCache: Boolean? = null
private var useModernRuntimeSchedulerCache: Boolean? = null
private var enableMicrotasksCache: Boolean? = null
Expand All @@ -40,6 +43,55 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
return cached
}

<<<<<<< HEAD
||||||| parent of 849da2146ca (Split scheduler commit and flush delegate methods (#44188))
override fun batchRenderingUpdatesInEventLoop(): Boolean {
var cached = batchRenderingUpdatesInEventLoopCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.batchRenderingUpdatesInEventLoop()
batchRenderingUpdatesInEventLoopCache = cached
}
return cached
}

override fun destroyFabricSurfacesInReactInstanceManager(): Boolean {
var cached = destroyFabricSurfacesInReactInstanceManagerCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.destroyFabricSurfacesInReactInstanceManager()
destroyFabricSurfacesInReactInstanceManagerCache = cached
}
return cached
}

=======
override fun androidEnablePendingFabricTransactions(): Boolean {
var cached = androidEnablePendingFabricTransactionsCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.androidEnablePendingFabricTransactions()
androidEnablePendingFabricTransactionsCache = cached
}
return cached
}

override fun batchRenderingUpdatesInEventLoop(): Boolean {
var cached = batchRenderingUpdatesInEventLoopCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.batchRenderingUpdatesInEventLoop()
batchRenderingUpdatesInEventLoopCache = cached
}
return cached
}

override fun destroyFabricSurfacesInReactInstanceManager(): Boolean {
var cached = destroyFabricSurfacesInReactInstanceManagerCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.destroyFabricSurfacesInReactInstanceManager()
destroyFabricSurfacesInReactInstanceManagerCache = cached
}
return cached
}

>>>>>>> 849da2146ca (Split scheduler commit and flush delegate methods (#44188))
override fun enableBackgroundExecutor(): Boolean {
var cached = enableBackgroundExecutorCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<dfbe9bcab657e4c66dd104788639448d>>
* @generated SignedSource<<c3f02dff409c10aa7922141cef3a6990>>
*/

/**
Expand All @@ -30,6 +30,12 @@ public object ReactNativeFeatureFlagsCxxInterop {

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

@DoNotStrip @JvmStatic public external fun androidEnablePendingFabricTransactions(): Boolean

@DoNotStrip @JvmStatic public external fun batchRenderingUpdatesInEventLoop(): Boolean

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

@DoNotStrip @JvmStatic public external fun enableBackgroundExecutor(): Boolean

@DoNotStrip @JvmStatic public external fun useModernRuntimeScheduler(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<cc8e437bf2f486949f256a19d3d73a1e>>
* @generated SignedSource<<d12888990ebca7c6199f4b51802ee59b>>
*/

/**
Expand All @@ -25,6 +25,12 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi

override fun commonTestFlag(): Boolean = false

override fun androidEnablePendingFabricTransactions(): Boolean = false

override fun batchRenderingUpdatesInEventLoop(): Boolean = false

override fun destroyFabricSurfacesInReactInstanceManager(): Boolean = false

override fun enableBackgroundExecutor(): Boolean = false

override fun useModernRuntimeScheduler(): Boolean = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<63356ad414e641eae11ca07b1a876fd3>>
* @generated SignedSource<<c06b3b34cea24459f6ade0ec5665dae7>>
*/

/**
Expand All @@ -25,6 +25,9 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
private val accessedFeatureFlags = mutableSetOf<String>()

private var commonTestFlagCache: Boolean? = null
private var androidEnablePendingFabricTransactionsCache: Boolean? = null
private var batchRenderingUpdatesInEventLoopCache: Boolean? = null
private var destroyFabricSurfacesInReactInstanceManagerCache: Boolean? = null
private var enableBackgroundExecutorCache: Boolean? = null
private var useModernRuntimeSchedulerCache: Boolean? = null
private var enableMicrotasksCache: Boolean? = null
Expand All @@ -45,6 +48,60 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
return cached
}

<<<<<<< HEAD
||||||| parent of 849da2146ca (Split scheduler commit and flush delegate methods (#44188))
override fun batchRenderingUpdatesInEventLoop(): Boolean {
var cached = batchRenderingUpdatesInEventLoopCache
if (cached == null) {
cached = currentProvider.batchRenderingUpdatesInEventLoop()
accessedFeatureFlags.add("batchRenderingUpdatesInEventLoop")
batchRenderingUpdatesInEventLoopCache = cached
}
return cached
}

override fun destroyFabricSurfacesInReactInstanceManager(): Boolean {
var cached = destroyFabricSurfacesInReactInstanceManagerCache
if (cached == null) {
cached = currentProvider.destroyFabricSurfacesInReactInstanceManager()
accessedFeatureFlags.add("destroyFabricSurfacesInReactInstanceManager")
destroyFabricSurfacesInReactInstanceManagerCache = cached
}
return cached
}

=======
override fun androidEnablePendingFabricTransactions(): Boolean {
var cached = androidEnablePendingFabricTransactionsCache
if (cached == null) {
cached = currentProvider.androidEnablePendingFabricTransactions()
accessedFeatureFlags.add("androidEnablePendingFabricTransactions")
androidEnablePendingFabricTransactionsCache = cached
}
return cached
}

override fun batchRenderingUpdatesInEventLoop(): Boolean {
var cached = batchRenderingUpdatesInEventLoopCache
if (cached == null) {
cached = currentProvider.batchRenderingUpdatesInEventLoop()
accessedFeatureFlags.add("batchRenderingUpdatesInEventLoop")
batchRenderingUpdatesInEventLoopCache = cached
}
return cached
}

override fun destroyFabricSurfacesInReactInstanceManager(): Boolean {
var cached = destroyFabricSurfacesInReactInstanceManagerCache
if (cached == null) {
cached = currentProvider.destroyFabricSurfacesInReactInstanceManager()
accessedFeatureFlags.add("destroyFabricSurfacesInReactInstanceManager")
destroyFabricSurfacesInReactInstanceManagerCache = cached
}
return cached
}

>>>>>>> 849da2146ca (Split scheduler commit and flush delegate methods (#44188))
override fun enableBackgroundExecutor(): Boolean {
var cached = enableBackgroundExecutorCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<c5388e841a06044520f6c89fcca27286>>
* @generated SignedSource<<dceb20e62a9ddbb98c872a24fab9765c>>
*/

/**
Expand All @@ -25,6 +25,12 @@ import com.facebook.proguard.annotations.DoNotStrip
public interface ReactNativeFeatureFlagsProvider {
@DoNotStrip public fun commonTestFlag(): Boolean

@DoNotStrip public fun androidEnablePendingFabricTransactions(): Boolean

@DoNotStrip public fun batchRenderingUpdatesInEventLoop(): Boolean

@DoNotStrip public fun destroyFabricSurfacesInReactInstanceManager(): Boolean

@DoNotStrip public fun enableBackgroundExecutor(): Boolean

@DoNotStrip public fun useModernRuntimeScheduler(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -474,16 +474,51 @@ std::shared_ptr<FabricMountingManager> Binding::getMountingManager(

void Binding::schedulerDidFinishTransaction(
const MountingCoordinator::Shared& mountingCoordinator) {
auto mountingManager = getMountingManager("schedulerDidFinishTransaction");
if (!mountingManager) {
if (!ReactNativeFeatureFlags::androidEnablePendingFabricTransactions()) {
return;
}

auto mountingTransaction = mountingCoordinator->pullTransaction();
if (!mountingTransaction.has_value()) {
return;
}
mountingManager->executeMount(*mountingTransaction);

std::unique_lock<std::mutex> lock(pendingTransactionsMutex_);
auto pendingTransaction = std::find_if(
pendingTransactions_.begin(),
pendingTransactions_.end(),
[&](const auto& transaction) {
return transaction.getSurfaceId() ==
mountingTransaction->getSurfaceId();
});

if (pendingTransaction != pendingTransactions_.end()) {
pendingTransaction->mergeWith(std::move(*mountingTransaction));
} else {
pendingTransactions_.push_back(std::move(*mountingTransaction));
}
}

void Binding::schedulerShouldRenderTransactions(
const MountingCoordinator::Shared& mountingCoordinator) {
auto mountingManager =
getMountingManager("schedulerShouldRenderTransactions");
if (!mountingManager) {
return;
}

if (ReactNativeFeatureFlags::androidEnablePendingFabricTransactions()) {
std::unique_lock<std::mutex> lock(pendingTransactionsMutex_);
for (auto& transaction : pendingTransactions_) {
mountingManager->executeMount(transaction);
}
pendingTransactions_.clear();
} else {
auto mountingTransaction = mountingCoordinator->pullTransaction();
if (mountingTransaction.has_value()) {
mountingManager->executeMount(*mountingTransaction);
}
}
}

void Binding::schedulerDidRequestPreliminaryViewAllocation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#pragma once

#include <memory>
#include <mutex>
#include <shared_mutex>
#include <unordered_map>

Expand Down Expand Up @@ -101,6 +102,9 @@ class Binding : public jni::HybridClass<Binding, JBinding>,
void schedulerDidFinishTransaction(
const MountingCoordinator::Shared& mountingCoordinator) override;

void schedulerShouldRenderTransactions(
const MountingCoordinator::Shared& mountingCoordinator) override;

void schedulerDidRequestPreliminaryViewAllocation(
const SurfaceId surfaceId,
const ShadowNode& shadowNode) override;
Expand Down Expand Up @@ -146,6 +150,10 @@ class Binding : public jni::HybridClass<Binding, JBinding>,
std::shared_mutex
surfaceHandlerRegistryMutex_; // Protects `surfaceHandlerRegistry_`.

// Track pending transactions, one per surfaceId
std::mutex pendingTransactionsMutex_;
std::vector<MountingTransaction> pendingTransactions_;

float pointScaleFactor_ = 1;

std::shared_ptr<const ReactNativeConfig> reactNativeConfig_{nullptr};
Expand Down
Loading

0 comments on commit ff4b20e

Please sign in to comment.