From 3552ff056263cd2d77224b909cc6fc2f81ad66cf Mon Sep 17 00:00:00 2001 From: Felipe Perez Date: Tue, 1 Feb 2022 15:15:19 -0800 Subject: [PATCH] Back out "Delete RuntimeScheduler yielding mobile config" Summary: D33740360 (https://github.com/facebook/react-native/commit/16ed62a850dd81bd9cc7f77ab3e77f42ed64b177) broke Explore VR on React Native. The app would go into a loop on boot and not finish mounting. This is probably a product code issue, but it's not a trivial issue to solve. Unlanding to unblock the RN migration. Changelog: [internal] internal Reviewed By: mdvacca Differential Revision: D33918026 fbshipit-source-id: cc77c70ece9994d82c91f7ae8783e959629e9cfb --- React/Fabric/RCTSurfacePresenter.mm | 2 ++ .../com/facebook/react/fabric/jni/Binding.cpp | 2 ++ .../runtimescheduler/RuntimeScheduler.cpp | 24 +++++++++++++------ .../runtimescheduler/RuntimeScheduler.h | 10 ++++++++ .../tests/RuntimeSchedulerTest.cpp | 9 ++++++- 5 files changed, 39 insertions(+), 8 deletions(-) diff --git a/React/Fabric/RCTSurfacePresenter.mm b/React/Fabric/RCTSurfacePresenter.mm index cc16148690f..17ca2900ad3 100644 --- a/React/Fabric/RCTSurfacePresenter.mm +++ b/React/Fabric/RCTSurfacePresenter.mm @@ -271,6 +271,8 @@ static BackgroundExecutor RCTGetBackgroundExecutor() auto weakRuntimeScheduler = _contextContainer->find>("RuntimeScheduler"); auto runtimeScheduler = weakRuntimeScheduler.hasValue() ? weakRuntimeScheduler.value().lock() : nullptr; if (runtimeScheduler) { + runtimeScheduler->setEnableYielding( + reactNativeConfig->getBool("react_native_new_architecture:runtimescheduler_enable_yielding_ios")); runtimeExecutor = [runtimeScheduler](std::function &&callback) { runtimeScheduler->scheduleWork(std::move(callback)); }; 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 c8dde14b528..d22dfb384e2 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 @@ -369,6 +369,8 @@ void Binding::installFabricUIManager( if (runtimeSchedulerHolder) { auto runtimeScheduler = runtimeSchedulerHolder->cthis()->get().lock(); if (runtimeScheduler) { + runtimeScheduler->setEnableYielding(config->getBool( + "react_native_new_architecture:runtimescheduler_enable_yielding_android")); runtimeExecutor = [runtimeScheduler]( std::function &&callback) { diff --git a/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp b/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp index 82a9f7c2012..f1d114cc9d6 100644 --- a/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp +++ b/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp @@ -22,13 +22,19 @@ RuntimeScheduler::RuntimeScheduler( void RuntimeScheduler::scheduleWork( std::function callback) const { - shouldYield_ = true; - runtimeExecutor_( - [this, callback = std::move(callback)](jsi::Runtime &runtime) { - shouldYield_ = false; - callback(runtime); - startWorkLoop(runtime); - }); + if (enableYielding_) { + shouldYield_ = true; + runtimeExecutor_( + [this, callback = std::move(callback)](jsi::Runtime &runtime) { + shouldYield_ = false; + callback(runtime); + startWorkLoop(runtime); + }); + } else { + runtimeExecutor_([callback = std::move(callback)](jsi::Runtime &runtime) { + callback(runtime); + }); + } } std::shared_ptr RuntimeScheduler::scheduleTask( @@ -64,6 +70,10 @@ RuntimeSchedulerTimePoint RuntimeScheduler::now() const noexcept { return now_(); } +void RuntimeScheduler::setEnableYielding(bool enableYielding) { + enableYielding_ = enableYielding; +} + void RuntimeScheduler::executeNowOnTheSameThread( std::function callback) { shouldYield_ = true; diff --git a/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h b/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h index 63ed4ab031b..d5d01727842 100644 --- a/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h +++ b/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h @@ -106,6 +106,7 @@ class RuntimeScheduler final { * Thread synchronization must be enforced externally. */ void callImmediates(jsi::Runtime &runtime); + void setEnableYielding(bool enableYielding); private: mutable std::priority_queue< @@ -139,6 +140,15 @@ class RuntimeScheduler final { */ mutable std::atomic_bool isWorkLoopScheduled_{false}; + /* + * Flag indicating if yielding is enabled. + * + * If set to true and Concurrent Mode is enabled on the surface, + * React Native will ask React to yield in case any work has been scheduled. + * Default value is false + */ + bool enableYielding_{false}; + /* * This flag is set while performing work, to prevent re-entrancy. */ diff --git a/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp b/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp index ad7c33a1e9a..7b88581360e 100644 --- a/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp +++ b/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp @@ -323,7 +323,7 @@ TEST_F(RuntimeSchedulerTest, scheduleWork) { EXPECT_FALSE(wasCalled); - EXPECT_TRUE(runtimeScheduler_->getShouldYield()); + EXPECT_FALSE(runtimeScheduler_->getShouldYield()); EXPECT_EQ(stubQueue_->size(), 1); @@ -334,6 +334,7 @@ TEST_F(RuntimeSchedulerTest, scheduleWork) { } TEST_F(RuntimeSchedulerTest, scheduleWorkWithYielding) { + runtimeScheduler_->setEnableYielding(true); bool wasCalled = false; runtimeScheduler_->scheduleWork( [&](jsi::Runtime const &) { wasCalled = true; }); @@ -352,6 +353,8 @@ TEST_F(RuntimeSchedulerTest, scheduleWorkWithYielding) { } TEST_F(RuntimeSchedulerTest, normalTaskYieldsToPlatformEvent) { + runtimeScheduler_->setEnableYielding(true); + bool didRunJavaScriptTask = false; bool didRunPlatformWork = false; @@ -379,6 +382,8 @@ TEST_F(RuntimeSchedulerTest, normalTaskYieldsToPlatformEvent) { } TEST_F(RuntimeSchedulerTest, expiredTaskDoesntYieldToPlatformEvent) { + runtimeScheduler_->setEnableYielding(true); + bool didRunJavaScriptTask = false; bool didRunPlatformWork = false; @@ -407,6 +412,8 @@ TEST_F(RuntimeSchedulerTest, expiredTaskDoesntYieldToPlatformEvent) { } TEST_F(RuntimeSchedulerTest, immediateTaskDoesntYieldToPlatformEvent) { + runtimeScheduler_->setEnableYielding(true); + bool didRunJavaScriptTask = false; bool didRunPlatformWork = false;