From 86561ddf507e4f82b6db5f050228d0589331c17f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Wed, 13 Mar 2024 03:31:41 -0700 Subject: [PATCH] Remove getIsSynchronous method from RuntimeScheduler (#43441) Summary: `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. Differential Revision: D54804805 --- .../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 79be477d2c24..4f2cf5d8646d 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 26c2c336202d..115bc3ec3d99 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 8bcb0420df97..22312c98ca7f 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 5058cb40252b..ea892b043bc4 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 812eac2456d8..325de9f44c27 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 d4ad98d59a1f..fc436b5d2283 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 aa737c68310e..76011d0a272f 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 0759d8470bd5..d31707f5217c 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 ef98135ee474..bff617dc5adc 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 b57ecc3d38f5..212184bc023f 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();