From 6c50418474603b2b7fe9478a3b79ae64333e9bd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Wed, 13 Mar 2024 08:07:32 -0700 Subject: [PATCH] Remove getIsSynchronous method from RuntimeScheduler (#43441) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/43441 `RuntimeScheduler::getIsSynchronous` is currently used to check if the current task is being executed on the main thread, to avoid using the background executor. This was part of a test to dispatch events synchronously in an app using background executor. We're not running that test anymore and this method doesn't make a lot of sense in the first place (it's not checking if the current task is running on the main thread, only if the caller of the task scheduled it synchronously from whatever thread they called from), so we can remove the method. If this is necessary in the future we should create a method that actually does something useful (like `isCurrentTaskOnMainThread()`). Changelog: [internal] I consider this change not to be a breaking change because runtime scheduler wasn't an official public API, and what we want to make public doesn't include this method. Reviewed By: sammy-SC Differential Revision: D54804805 fbshipit-source-id: 54b3a6586e08bccc201df848b942fb1fcae29f30 --- .../runtimescheduler/RuntimeScheduler.cpp | 4 ---- .../runtimescheduler/RuntimeScheduler.h | 9 -------- .../RuntimeSchedulerBinding.cpp | 4 ---- .../RuntimeSchedulerBinding.h | 2 -- .../RuntimeScheduler_Legacy.cpp | 6 ----- .../RuntimeScheduler_Legacy.h | 8 ------- .../RuntimeScheduler_Modern.cpp | 8 ------- .../RuntimeScheduler_Modern.h | 10 --------- .../tests/RuntimeSchedulerTest.cpp | 4 +--- .../renderer/uimanager/UIManagerBinding.cpp | 22 +++++++++---------- 10 files changed, 11 insertions(+), 66 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp index 79be477d2c240a..4f2cf5d8646d20 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp @@ -58,10 +58,6 @@ bool RuntimeScheduler::getShouldYield() const noexcept { return runtimeSchedulerImpl_->getShouldYield(); } -bool RuntimeScheduler::getIsSynchronous() const noexcept { - return runtimeSchedulerImpl_->getIsSynchronous(); -} - void RuntimeScheduler::cancelTask(Task& task) noexcept { return runtimeSchedulerImpl_->cancelTask(task); } diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h index 26c2c336202df2..115bc3ec3d9930 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.h @@ -30,7 +30,6 @@ class RuntimeSchedulerBase { RawCallback&& callback) noexcept = 0; virtual void cancelTask(Task& task) noexcept = 0; virtual bool getShouldYield() const noexcept = 0; - virtual bool getIsSynchronous() const noexcept = 0; virtual SchedulerPriority getCurrentPriorityLevel() const noexcept = 0; virtual RuntimeSchedulerTimePoint now() const noexcept = 0; virtual void callExpiredTasks(jsi::Runtime& runtime) = 0; @@ -100,14 +99,6 @@ class RuntimeScheduler final : RuntimeSchedulerBase { */ bool getShouldYield() const noexcept override; - /* - * Return value informs if the current task is executed inside synchronous - * block. - * - * Can be called from any thread. - */ - bool getIsSynchronous() const noexcept override; - /* * Returns value of currently executed task. Designed to be called from React. * diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerBinding.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerBinding.cpp index 8bcb0420df9779..22312c98ca7f9a 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerBinding.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerBinding.cpp @@ -61,10 +61,6 @@ RuntimeSchedulerBinding::RuntimeSchedulerBinding( std::shared_ptr runtimeScheduler) : runtimeScheduler_(std::move(runtimeScheduler)) {} -bool RuntimeSchedulerBinding::getIsSynchronous() const { - return runtimeScheduler_->getIsSynchronous(); -} - jsi::Value RuntimeSchedulerBinding::get( jsi::Runtime& runtime, const jsi::PropNameID& name) { diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerBinding.h b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerBinding.h index 5058cb40252b60..ea892b043bc4b3 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerBinding.h +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeSchedulerBinding.h @@ -42,8 +42,6 @@ class RuntimeSchedulerBinding : public jsi::HostObject { */ jsi::Value get(jsi::Runtime& runtime, const jsi::PropNameID& name) override; - bool getIsSynchronous() const; - private: std::shared_ptr runtimeScheduler_; }; diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp index 812eac2456d8c0..325de9f44c2700 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp @@ -79,10 +79,6 @@ bool RuntimeScheduler_Legacy::getShouldYield() const noexcept { return runtimeAccessRequests_ > 0; } -bool RuntimeScheduler_Legacy::getIsSynchronous() const noexcept { - return isSynchronous_; -} - void RuntimeScheduler_Legacy::cancelTask(Task& task) noexcept { task.callback.reset(); } @@ -108,9 +104,7 @@ void RuntimeScheduler_Legacy::executeNowOnTheSameThread( "RuntimeScheduler::executeNowOnTheSameThread callback"); runtimeAccessRequests_ -= 1; - isSynchronous_ = true; callback(runtime); - isSynchronous_ = false; }); // Resume work loop if needed. In synchronous mode diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h index d4ad98d59a1f43..fc436b5d2283a6 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h @@ -77,14 +77,6 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase { */ bool getShouldYield() const noexcept override; - /* - * Return value informs if the current task is executed inside synchronous - * block. - * - * Can be called from any thread. - */ - bool getIsSynchronous() const noexcept override; - /* * Returns value of currently executed task. Designed to be called from React. * diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp index aa737c68310ed1..76011d0a272fb6 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp @@ -74,10 +74,6 @@ bool RuntimeScheduler_Modern::getShouldYield() const noexcept { (!taskQueue_.empty() && taskQueue_.top() != currentTask_); } -bool RuntimeScheduler_Modern::getIsSynchronous() const noexcept { - return isSynchronous_; -} - void RuntimeScheduler_Modern::cancelTask(Task& task) noexcept { task.callback.reset(); } @@ -105,8 +101,6 @@ void RuntimeScheduler_Modern::executeNowOnTheSameThread( syncTaskRequests_--; - isSynchronous_ = true; - auto currentTime = now_(); auto priority = SchedulerPriority::ImmediatePriority; auto expirationTime = @@ -115,8 +109,6 @@ void RuntimeScheduler_Modern::executeNowOnTheSameThread( priority, std::move(callback), expirationTime); executeTask(runtime, task, currentTime); - - isSynchronous_ = false; }); bool shouldScheduleWorkLoop = false; diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h index 0759d8470bd54b..d31707f5217c96 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h @@ -86,14 +86,6 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase { */ bool getShouldYield() const noexcept override; - /* - * Return value informs if the current task is executed inside synchronous - * block. - * - * Can be called from any thread. - */ - bool getIsSynchronous() const noexcept override; - /* * Returns value of currently executed task. Designed to be called from React. * @@ -149,8 +141,6 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase { const RuntimeExecutor runtimeExecutor_; SchedulerPriority currentPriority_{SchedulerPriority::NormalPriority}; - std::atomic_bool isSynchronous_{false}; - void scheduleWorkLoop(); void startWorkLoop(jsi::Runtime& runtime, bool onlyExpired); diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp index ef98135ee47482..bff617dc5adc93 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp @@ -777,11 +777,9 @@ TEST_P(RuntimeSchedulerTest, basicSameThreadExecution) { bool didRunSynchronousTask = false; std::thread t1([this, &didRunSynchronousTask]() { runtimeScheduler_->executeNowOnTheSameThread( - [this, &didRunSynchronousTask](jsi::Runtime& /*rt*/) { - EXPECT_TRUE(runtimeScheduler_->getIsSynchronous()); + [&didRunSynchronousTask](jsi::Runtime& /*rt*/) { didRunSynchronousTask = true; }); - EXPECT_FALSE(runtimeScheduler_->getIsSynchronous()); }); auto hasTask = stubQueue_->waitForTask(); diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp index b57ecc3d38f5dc..212184bc023f19 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp @@ -500,18 +500,7 @@ jsi::Value UIManagerBinding::get( RuntimeSchedulerBinding::getBinding(runtime); auto surfaceId = surfaceIdFromValue(runtime, arguments[0]); - if (!uiManager->backgroundExecutor_ || - (runtimeSchedulerBinding && - runtimeSchedulerBinding->getIsSynchronous())) { - auto shadowNodeList = - shadowNodeListFromValue(runtime, arguments[1]); - uiManager->completeSurface( - surfaceId, - shadowNodeList, - {.enableStateReconciliation = true, - .mountSynchronously = false, - .shouldYield = nullptr}); - } else { + if (uiManager->backgroundExecutor_) { auto weakShadowNodeList = weakShadowNodeListFromValue(runtime, arguments[1]); static std::atomic_uint_fast8_t completeRootEventCounter{0}; @@ -542,6 +531,15 @@ jsi::Value UIManagerBinding::get( /* .shouldYield = */ shouldYield}); } }); + } else { + auto shadowNodeList = + shadowNodeListFromValue(runtime, arguments[1]); + uiManager->completeSurface( + surfaceId, + shadowNodeList, + {.enableStateReconciliation = true, + .mountSynchronously = false, + .shouldYield = nullptr}); } return jsi::Value::undefined();