From 949dedca9ff2185b5cb39ce54c89f91b8df0a7ef Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Wed, 29 Jul 2020 15:38:31 -0700 Subject: [PATCH] Implement BackgroundExecutor for layouts Summary: As a followup to D22743723 (https://github.com/facebook/react-native/commit/d53fc8a3cd44c7ec7e6eade985daf3d4feb2d736) on the iOS side, I implement a BackgroundExecutor that can be used from C++ to schedule layouts on their own thread, off the JS and UI thread. This is a potential perf improvement that we will experiment with. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22826795 fbshipit-source-id: 899bd67fe1b86f52910951e9536b59a1414a304c --- .../react/bridge/BackgroundExecutor.java | 46 +++++++++++++++++++ .../com/facebook/react/fabric/jni/Binding.cpp | 6 +++ .../com/facebook/react/fabric/jni/Binding.h | 2 + .../react/fabric/jni/JBackgroundExecutor.cpp | 35 ++++++++++++++ .../react/fabric/jni/JBackgroundExecutor.h | 34 ++++++++++++++ 5 files changed, 123 insertions(+) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/bridge/BackgroundExecutor.java create mode 100644 ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.cpp create mode 100644 ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.h diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/BackgroundExecutor.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/BackgroundExecutor.java new file mode 100644 index 00000000000..8b5d7c94254 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/BackgroundExecutor.java @@ -0,0 +1,46 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.bridge; + +import com.facebook.proguard.annotations.DoNotStrip; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +@DoNotStrip +public class BackgroundExecutor { + private static final String TAG = "FabricBackgroundExecutor"; + private final ExecutorService mExecutorService; + + @DoNotStrip + private BackgroundExecutor() { + mExecutorService = Executors.newFixedThreadPool(1); + } + + @DoNotStrip + private void queueRunnable(Runnable runnable) { + // Very rarely, an NPE is hit here - probably has to do with deallocation + // race conditions and the JNI. + // It's not clear yet which of these is most prevalent, or if either is a concern. + // If we don't find these logs in production then we can probably safely remove the logging, + // but it's also cheap to leave it here. + + if (runnable == null) { + ReactSoftException.logSoftException(TAG, new ReactNoCrashSoftException("runnable is null")); + return; + } + + final ExecutorService executorService = mExecutorService; + if (executorService == null) { + ReactSoftException.logSoftException( + TAG, new ReactNoCrashSoftException("executorService is null")); + return; + } + + executorService.execute(runnable); + } +} 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 e44f1c232cb..10e22094c15 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 @@ -278,6 +278,12 @@ 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(); + } + if (enableLayoutAnimations) { animationDriver_ = std::make_shared(this); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h index b43f2b0532b..5c2f33fad27 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h @@ -19,6 +19,7 @@ #include #include "ComponentFactoryDelegate.h" #include "EventBeatManager.h" +#include "JBackgroundExecutor.h" namespace facebook { namespace react { @@ -115,6 +116,7 @@ class Binding : public jni::HybridClass, virtual void onAllAnimationsComplete() override; LayoutAnimationDriver *getAnimationDriver(); std::shared_ptr animationDriver_; + std::unique_ptr backgroundExecutor_; std::shared_ptr scheduler_; std::mutex schedulerMutex_; diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.cpp new file mode 100644 index 00000000000..39cb4a4e593 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.cpp @@ -0,0 +1,35 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include +#include + +#include "JBackgroundExecutor.h" + +namespace facebook { +namespace react { + +using namespace facebook::jni; + +using facebook::react::JNativeRunnable; +using facebook::react::Runnable; + +BackgroundExecutor JBackgroundExecutor::get() { + auto self = JBackgroundExecutor::create(); + + return [self](std::function &&runnable) { + static auto method = + findClassStatic(JBackgroundExecutor::JBackgroundExecutorJavaDescriptor) + ->getMethod("queueRunnable"); + + auto jrunnable = JNativeRunnable::newObjectCxxArgs(std::move(runnable)); + method(self, static_ref_cast(jrunnable).get()); + }; +} + +} // namespace react +} // namespace facebook diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.h new file mode 100644 index 00000000000..002c0249d63 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.h @@ -0,0 +1,34 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include + +namespace facebook { +namespace react { + +using namespace facebook::jni; + +class JBackgroundExecutor : public JavaClass { + public: + static auto constexpr kJavaDescriptor = + "Lcom/facebook/react/bridge/BackgroundExecutor;"; + + constexpr static auto JBackgroundExecutorJavaDescriptor = + "com/facebook/react/bridge/BackgroundExecutor"; + + static global_ref create() { + return make_global(newInstance()); + } + + BackgroundExecutor get(); +}; + +} // namespace react +} // namespace facebook