From 387e79f8aadc076808ce57aa8d2f9212211ab607 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 2 Dec 2021 15:24:45 -0800 Subject: [PATCH] Remove background_executor flag Summary: changelog: [internal] Background executor has been shipped on both platforms for a long time. I've kept the flag around because I wanted to run tests and compare Concurrent Mode vs Background Executor. The intention was to see if we can get rid of Background Executor to simplify the threading model. Since then, React team has moved away from Concurrent Mode towards more gradual rollout of concurrent rendering and it no longer makes sense to do this comparison. Right now, we don't have a concern with concurrent rendering and Background Executor. If we ever want to run the an experiment, this gating will need to be added again. Reviewed By: javache Differential Revision: D32674798 fbshipit-source-id: a1e51c9c5b8e48efa4cb0f25379d58e7eb80ccd9 --- React/Fabric/RCTSurfacePresenter.mm | 4 +- .../com/facebook/react/fabric/jni/Binding.cpp | 7 +- .../renderer/uimanager/UIManagerBinding.cpp | 103 +++++++----------- 3 files changed, 44 insertions(+), 70 deletions(-) diff --git a/React/Fabric/RCTSurfacePresenter.mm b/React/Fabric/RCTSurfacePresenter.mm index c05c806b48e..81ec1d5cca1 100644 --- a/React/Fabric/RCTSurfacePresenter.mm +++ b/React/Fabric/RCTSurfacePresenter.mm @@ -285,9 +285,7 @@ static BackgroundExecutor RCTGetBackgroundExecutor() return std::make_unique(activities, owner); }; - if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:enable_background_executor_ios")) { - toolbox.backgroundExecutor = RCTGetBackgroundExecutor(); - } + toolbox.backgroundExecutor = RCTGetBackgroundExecutor(); toolbox.synchronousEventBeatFactory = [runtimeExecutor, runtimeScheduler = runtimeScheduler](EventBeat::SharedOwnerBox const &ownerBox) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp index eb9082b854b..2871394179b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp @@ -606,11 +606,8 @@ void Binding::installFabricUIManager( toolbox.synchronousEventBeatFactory = synchronousBeatFactory; toolbox.asynchronousEventBeatFactory = asynchronousBeatFactory; - if (reactNativeConfig_->getBool( - "react_fabric:enable_background_executor_android")) { - backgroundExecutor_ = std::make_unique(); - toolbox.backgroundExecutor = backgroundExecutor_->get(); - } + backgroundExecutor_ = std::make_unique(); + toolbox.backgroundExecutor = backgroundExecutor_->get(); animationDriver_ = std::make_shared( runtimeExecutor, contextContainer, this); diff --git a/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp b/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp index cbc68035ad8..e76348b74f6 100644 --- a/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp +++ b/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp @@ -531,69 +531,48 @@ jsi::Value UIManagerBinding::get( } if (methodName == "completeRoot") { - if (uiManager->backgroundExecutor_) { - std::weak_ptr weakUIManager = uiManager_; - // Enhanced version of the method that uses `backgroundExecutor` and - // captures a shared pointer to `UIManager`. - return jsi::Function::createFromHostFunction( - runtime, - name, - 2, - [weakUIManager, uiManager]( - jsi::Runtime &runtime, - jsi::Value const &thisValue, - jsi::Value const *arguments, - size_t count) noexcept -> jsi::Value { - auto surfaceId = surfaceIdFromValue(runtime, arguments[0]); - auto weakShadowNodeList = - weakShadowNodeListFromValue(runtime, arguments[1]); - static std::atomic_uint_fast8_t completeRootEventCounter{0}; - static std::atomic_uint_fast32_t mostRecentSurfaceId{0}; - completeRootEventCounter += 1; - mostRecentSurfaceId = surfaceId; - uiManager->backgroundExecutor_( - [weakUIManager, - weakShadowNodeList, - surfaceId, - eventCount = completeRootEventCounter.load()] { - auto shouldYield = [=]() -> bool { - // If `completeRootEventCounter` was incremented, another - // `completeSurface` call has been scheduled and current - // `completeSurface` should yield to it. - return completeRootEventCounter > eventCount && - mostRecentSurfaceId == surfaceId; - }; - auto shadowNodeList = - shadowNodeListFromWeakList(weakShadowNodeList); - auto strongUIManager = weakUIManager.lock(); - if (shadowNodeList && strongUIManager) { - strongUIManager->completeSurface( - surfaceId, shadowNodeList, {true, shouldYield}); - } - }); + std::weak_ptr weakUIManager = uiManager_; + // Enhanced version of the method that uses `backgroundExecutor` and + // captures a shared pointer to `UIManager`. + return jsi::Function::createFromHostFunction( + runtime, + name, + 2, + [weakUIManager, uiManager]( + jsi::Runtime &runtime, + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) noexcept -> jsi::Value { + auto surfaceId = surfaceIdFromValue(runtime, arguments[0]); + auto weakShadowNodeList = + weakShadowNodeListFromValue(runtime, arguments[1]); + static std::atomic_uint_fast8_t completeRootEventCounter{0}; + static std::atomic_uint_fast32_t mostRecentSurfaceId{0}; + completeRootEventCounter += 1; + mostRecentSurfaceId = surfaceId; + uiManager->backgroundExecutor_( + [weakUIManager, + weakShadowNodeList, + surfaceId, + eventCount = completeRootEventCounter.load()] { + auto shouldYield = [=]() -> bool { + // If `completeRootEventCounter` was incremented, another + // `completeSurface` call has been scheduled and current + // `completeSurface` should yield to it. + return completeRootEventCounter > eventCount && + mostRecentSurfaceId == surfaceId; + }; + auto shadowNodeList = + shadowNodeListFromWeakList(weakShadowNodeList); + auto strongUIManager = weakUIManager.lock(); + if (shadowNodeList && strongUIManager) { + strongUIManager->completeSurface( + surfaceId, shadowNodeList, {true, shouldYield}); + } + }); - return jsi::Value::undefined(); - }); - } else { - // Basic version of the method that does *not* use `backgroundExecutor` - // and does *not* capture a shared pointer to `UIManager`. - return jsi::Function::createFromHostFunction( - runtime, - name, - 2, - [uiManager]( - jsi::Runtime &runtime, - jsi::Value const &thisValue, - jsi::Value const *arguments, - size_t count) noexcept -> jsi::Value { - uiManager->completeSurface( - surfaceIdFromValue(runtime, arguments[0]), - shadowNodeListFromValue(runtime, arguments[1]), - {true, {}}); - - return jsi::Value::undefined(); - }); - } + return jsi::Value::undefined(); + }); } if (methodName == "registerEventHandler") {