From d53fc8a3cd44c7ec7e6eade985daf3d4feb2d736 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Fri, 24 Jul 2020 23:50:50 -0700 Subject: [PATCH] Fabric: Moving layout calculation off JavaScript thread Summary: In the current Fabric model, we compute layout during the commit phase on the caller thread synchronously. Even though, in general, it's by design the correct way to do it, there are cases where it's *not* a requirement. In such cases, it's more optimal to yield early and continue execution of the commit process on a different thread asynchronously. One of such cases potentially is `completeRoot` call. There we don't need to return anything and can resume JavaScript execution immediately. The performance implications of that are not clear but there is a hope that it can free up to ~100ms of JavaScript execution time which is currently spent waiting for layout calculation (and other aspects of the commit phase). This is an implementation in the core. The plan is to test that on iOS first and then, in case if the results of the experiment are positive, to implement it on Android as well. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D22743723 fbshipit-source-id: 846a13152c5a419de0eaef0e6283ea4277c907dc --- .../fabric/mounting/MountingCoordinator.cpp | 1 - ReactCommon/fabric/scheduler/Scheduler.cpp | 1 + .../fabric/scheduler/SchedulerToolbox.h | 11 ++++ ReactCommon/fabric/uimanager/UIManager.cpp | 5 ++ ReactCommon/fabric/uimanager/UIManager.h | 4 ++ .../fabric/uimanager/UIManagerBinding.cpp | 60 ++++++++++++++----- ReactCommon/fabric/uimanager/primitives.h | 3 + 7 files changed, 70 insertions(+), 15 deletions(-) diff --git a/ReactCommon/fabric/mounting/MountingCoordinator.cpp b/ReactCommon/fabric/mounting/MountingCoordinator.cpp index f4d5f3ab7b8..3ffc83ac1cc 100644 --- a/ReactCommon/fabric/mounting/MountingCoordinator.cpp +++ b/ReactCommon/fabric/mounting/MountingCoordinator.cpp @@ -39,7 +39,6 @@ void MountingCoordinator::push(ShadowTreeRevision &&revision) const { { std::lock_guard lock(mutex_); - assert(revision.getNumber() > baseRevision_.getNumber()); assert( !lastRevision_.has_value() || revision.getNumber() != lastRevision_->getNumber()); diff --git a/ReactCommon/fabric/scheduler/Scheduler.cpp b/ReactCommon/fabric/scheduler/Scheduler.cpp index 14a4f77086c..74c849f77d8 100644 --- a/ReactCommon/fabric/scheduler/Scheduler.cpp +++ b/ReactCommon/fabric/scheduler/Scheduler.cpp @@ -79,6 +79,7 @@ Scheduler::Scheduler( rootComponentDescriptor_ = std::make_unique( ComponentDescriptorParameters{eventDispatcher, nullptr, nullptr}); + uiManager->setBackgroundExecutor(schedulerToolbox.backgroundExecutor); uiManager->setDelegate(this); uiManager->setComponentDescriptorRegistry(componentDescriptorRegistry_); diff --git a/ReactCommon/fabric/scheduler/SchedulerToolbox.h b/ReactCommon/fabric/scheduler/SchedulerToolbox.h index 0f56dbcccb4..f94f410a3c3 100644 --- a/ReactCommon/fabric/scheduler/SchedulerToolbox.h +++ b/ReactCommon/fabric/scheduler/SchedulerToolbox.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -49,6 +50,16 @@ struct SchedulerToolbox final { */ EventBeat::Factory asynchronousEventBeatFactory; EventBeat::Factory synchronousEventBeatFactory; + + /* + * General-purpose executor that is used to dispatch work on some utility + * queue (mostly) asynchronously to avoid unnecessary blocking the caller + * queue. + * The concrete implementation can use a serial or concurrent queue. + * Due to architectural constraints, the concrete implementation *must* call + * the call back synchronously if the executor is invoked on the main thread. + */ + BackgroundExecutor backgroundExecutor; }; } // namespace react diff --git a/ReactCommon/fabric/uimanager/UIManager.cpp b/ReactCommon/fabric/uimanager/UIManager.cpp index f7172002c4c..a5f2554f72f 100644 --- a/ReactCommon/fabric/uimanager/UIManager.cpp +++ b/ReactCommon/fabric/uimanager/UIManager.cpp @@ -290,6 +290,11 @@ UIManagerDelegate *UIManager::getDelegate() { return delegate_; } +void UIManager::setBackgroundExecutor( + BackgroundExecutor const &backgroundExecutor) { + backgroundExecutor_ = backgroundExecutor; +} + void UIManager::visitBinding( std::function callback) const { diff --git a/ReactCommon/fabric/uimanager/UIManager.h b/ReactCommon/fabric/uimanager/UIManager.h index ab6fdbcca6a..29dda87d431 100644 --- a/ReactCommon/fabric/uimanager/UIManager.h +++ b/ReactCommon/fabric/uimanager/UIManager.h @@ -20,6 +20,7 @@ #include #include #include +#include namespace facebook { namespace react { @@ -41,6 +42,8 @@ class UIManager final : public ShadowTreeDelegate { void setDelegate(UIManagerDelegate *delegate); UIManagerDelegate *getDelegate(); + void setBackgroundExecutor(BackgroundExecutor const &backgroundExecutor); + /** * Sets and gets the UIManager's Animation APIs delegate. * The delegate is stored as a raw pointer, so the owner must null @@ -143,6 +146,7 @@ class UIManager final : public ShadowTreeDelegate { UIManagerAnimationDelegate *animationDelegate_{nullptr}; UIManagerBinding *uiManagerBinding_; ShadowTreeRegistry shadowTreeRegistry_{}; + BackgroundExecutor backgroundExecutor_{}; }; } // namespace react diff --git a/ReactCommon/fabric/uimanager/UIManagerBinding.cpp b/ReactCommon/fabric/uimanager/UIManagerBinding.cpp index 92d20a915cd..5d9c128d3e8 100644 --- a/ReactCommon/fabric/uimanager/UIManagerBinding.cpp +++ b/ReactCommon/fabric/uimanager/UIManagerBinding.cpp @@ -407,20 +407,52 @@ jsi::Value UIManagerBinding::get( } if (methodName == "completeRoot") { - return jsi::Function::createFromHostFunction( - runtime, - name, - 2, - [uiManager]( - jsi::Runtime &runtime, - const jsi::Value &thisValue, - const jsi::Value *arguments, - size_t count) -> jsi::Value { - uiManager->completeSurface( - surfaceIdFromValue(runtime, arguments[0]), - shadowNodeListFromValue(runtime, arguments[1])); - return jsi::Value::undefined(); - }); + if (uiManager->backgroundExecutor_) { + // Enhanced version of the method that uses `backgroundExecutor` and + // captures a shared pointer to `UIManager`. + return jsi::Function::createFromHostFunction( + runtime, + name, + 2, + [uiManager, sharedUIManager = uiManager_]( + jsi::Runtime &runtime, + jsi::Value const &thisValue, + jsi::Value const *arguments, + size_t count) -> jsi::Value { + auto surfaceId = surfaceIdFromValue(runtime, arguments[0]); + auto shadowNodeList = + shadowNodeListFromValue(runtime, arguments[1]); + + if (sharedUIManager->backgroundExecutor_) { + sharedUIManager->backgroundExecutor_( + [sharedUIManager, surfaceId, shadowNodeList] { + sharedUIManager->completeSurface(surfaceId, shadowNodeList); + }); + } else { + uiManager->completeSurface(surfaceId, shadowNodeList); + } + + 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) -> jsi::Value { + uiManager->completeSurface( + surfaceIdFromValue(runtime, arguments[0]), + shadowNodeListFromValue(runtime, arguments[1])); + + return jsi::Value::undefined(); + }); + } } if (methodName == "registerEventHandler") { diff --git a/ReactCommon/fabric/uimanager/primitives.h b/ReactCommon/fabric/uimanager/primitives.h index f02e8b7ffd6..f5d29a34bb2 100644 --- a/ReactCommon/fabric/uimanager/primitives.h +++ b/ReactCommon/fabric/uimanager/primitives.h @@ -16,6 +16,9 @@ namespace facebook { namespace react { +using BackgroundExecutor = + std::function &&callback)>; + struct EventHandlerWrapper : public EventHandler { EventHandlerWrapper(jsi::Function eventHandler) : callback(std::move(eventHandler)) {}