From 7dbcb4bf56ec856cd1ed8afd1d66eb39b002450d Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Fri, 6 May 2022 06:53:49 -0700 Subject: [PATCH 1/2] Name Fabric background thread Differential Revision: D36200090 fbshipit-source-id: afd8c44ad4079996342145cd90d01ebfd0741db5 --- .../react/bridge/BackgroundExecutor.java | 21 +++++++++++++++++-- .../com/facebook/react/fabric/jni/Binding.cpp | 9 ++++---- .../com/facebook/react/fabric/jni/Binding.h | 8 +++---- .../react/fabric/jni/JBackgroundExecutor.cpp | 13 ++++++------ .../react/fabric/jni/JBackgroundExecutor.h | 9 +------- 5 files changed, 35 insertions(+), 25 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/BackgroundExecutor.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/BackgroundExecutor.java index 0c56d2227f1383..083288bed74a17 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/BackgroundExecutor.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/BackgroundExecutor.java @@ -10,15 +10,32 @@ import com.facebook.proguard.annotations.DoNotStrip; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; @DoNotStrip public class BackgroundExecutor { private static final String TAG = "FabricBackgroundExecutor"; + + private static class NamedThreadFactory implements ThreadFactory { + private final String mName; + + public NamedThreadFactory(String name) { + mName = name; + } + + @Override + public Thread newThread(Runnable r) { + Thread thread = Executors.defaultThreadFactory().newThread(r); + thread.setName(mName); + return thread; + } + } + private final ExecutorService mExecutorService; @DoNotStrip - private BackgroundExecutor() { - mExecutorService = Executors.newFixedThreadPool(1); + private BackgroundExecutor(String name) { + mExecutorService = Executors.newFixedThreadPool(1, new NamedThreadFactory(name)); } @DoNotStrip 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 71bbef3fb6e665..491288cd68d568 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 @@ -6,8 +6,10 @@ */ #include "Binding.h" + #include "AsyncEventBeat.h" #include "EventEmitterWrapper.h" +#include "JBackgroundExecutor.h" #include "ReactNativeConfigHolder.h" #include "StateWrapperImpl.h" @@ -455,8 +457,8 @@ void Binding::installFabricUIManager( toolbox.synchronousEventBeatFactory = synchronousBeatFactory; toolbox.asynchronousEventBeatFactory = asynchronousBeatFactory; - backgroundExecutor_ = std::make_unique(); - toolbox.backgroundExecutor = backgroundExecutor_->get(); + backgroundExecutor_ = JBackgroundExecutor::create("fabric_bg"); + toolbox.backgroundExecutor = backgroundExecutor_; animationDriver_ = std::make_shared( runtimeExecutor, contextContainer, this); @@ -558,8 +560,7 @@ void Binding::preallocateView( }; if (dispatchPreallocationInBackground_) { - auto backgroundExecutor = backgroundExecutor_->get(); - backgroundExecutor(preallocationFunction); + backgroundExecutor_(preallocationFunction); } else { preallocationFunction(); } 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 9b0a1fd49cca02..38a392e929be32 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 @@ -9,6 +9,9 @@ #include "FabricMountingManager.h" +#include +#include + #include #include #include @@ -18,12 +21,9 @@ #include #include -#include -#include #include "ComponentFactory.h" #include "EventBeatManager.h" #include "EventEmitterWrapper.h" -#include "JBackgroundExecutor.h" #include "SurfaceHandlerBinding.h" namespace facebook { @@ -144,7 +144,7 @@ class Binding : public jni::HybridClass, std::shared_ptr animationDriver_; - std::unique_ptr backgroundExecutor_; + BackgroundExecutor backgroundExecutor_; butter::map surfaceHandlerRegistry_{}; butter::shared_mutex 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 index c742a4555bcded..88117cbb8452f0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.cpp @@ -18,16 +18,15 @@ using namespace facebook::jni; using facebook::react::JNativeRunnable; using facebook::react::Runnable; -BackgroundExecutor JBackgroundExecutor::get() { - auto self = JBackgroundExecutor::create(); - - return [self](std::function &&runnable) { +BackgroundExecutor JBackgroundExecutor::create(const std::string &name) { + auto instance = make_global(newInstance(name)); + return [instance = std::move(instance)](std::function &&runnable) { static auto method = - findClassStatic(JBackgroundExecutor::JBackgroundExecutorJavaDescriptor) - ->getMethod("queueRunnable"); + javaClassStatic()->getMethod( + "queueRunnable"); auto jrunnable = JNativeRunnable::newObjectCxxArgs(std::move(runnable)); - method(self, static_ref_cast(jrunnable).get()); + method(instance, jrunnable.get()); }; } 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 index 1cc2615287f032..f1bad8d1000ccb 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.h @@ -20,14 +20,7 @@ class JBackgroundExecutor : public JavaClass { 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(); + static BackgroundExecutor create(const std::string &name); }; } // namespace react From 681a8e90344ebf684053be029f5b71675925d233 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Fri, 6 May 2022 06:54:17 -0700 Subject: [PATCH 2/2] Replace NativeRunnable with fbjni implementation Summary: The implementations of these modules is pretty much identical, and we're already shipping the fbjni version of this anyway. Changelog: [Internal] Differential Revision: D36200330 fbshipit-source-id: 777e4e3e64288bcc128723939f36f997be7aecba --- .../react/bridge/queue/NativeRunnable.java | 25 --------- .../react/fabric/jni/JBackgroundExecutor.cpp | 12 ++--- ReactAndroid/src/main/jni/react/jni/BUCK | 1 - .../jni/react/jni/CatalystInstanceImpl.cpp | 3 -- .../jni/react/jni/JMessageQueueThread.cpp | 12 ++--- .../src/main/jni/react/jni/JNativeRunnable.h | 52 ------------------- 6 files changed, 9 insertions(+), 96 deletions(-) delete mode 100644 ReactAndroid/src/main/java/com/facebook/react/bridge/queue/NativeRunnable.java delete mode 100644 ReactAndroid/src/main/jni/react/jni/JNativeRunnable.h diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/NativeRunnable.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/NativeRunnable.java deleted file mode 100644 index 3573938d3d9579..00000000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/NativeRunnable.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and 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.queue; - -import com.facebook.jni.HybridData; -import com.facebook.proguard.annotations.DoNotStrip; - -/** A Runnable that has a native run implementation. */ -@DoNotStrip -public class NativeRunnable implements Runnable { - - private final HybridData mHybridData; - - @DoNotStrip - private NativeRunnable(HybridData hybridData) { - mHybridData = hybridData; - } - - public native void run(); -} 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 index 88117cbb8452f0..e6d4f641f5e1c4 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/JBackgroundExecutor.cpp @@ -5,26 +5,22 @@ * LICENSE file in the root directory of this source tree. */ -#include -#include - #include "JBackgroundExecutor.h" +#include +#include + namespace facebook { namespace react { using namespace facebook::jni; -using facebook::react::JNativeRunnable; -using facebook::react::Runnable; - BackgroundExecutor JBackgroundExecutor::create(const std::string &name) { auto instance = make_global(newInstance(name)); return [instance = std::move(instance)](std::function &&runnable) { static auto method = - javaClassStatic()->getMethod( + javaClassStatic()->getMethod( "queueRunnable"); - auto jrunnable = JNativeRunnable::newObjectCxxArgs(std::move(runnable)); method(instance, jrunnable.get()); }; diff --git a/ReactAndroid/src/main/jni/react/jni/BUCK b/ReactAndroid/src/main/jni/react/jni/BUCK index ab384eefa38f4b..787eb0f14bd6ca 100644 --- a/ReactAndroid/src/main/jni/react/jni/BUCK +++ b/ReactAndroid/src/main/jni/react/jni/BUCK @@ -8,7 +8,6 @@ EXPORTED_HEADERS = [ "JavaScriptExecutorHolder.h", "JCallback.h", "JMessageQueueThread.h", - "JNativeRunnable.h", "JReactMarker.h", "JSLoader.h", "JSLogging.h", diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp index 61911ce6e35120..6a936c66f66f23 100644 --- a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp +++ b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp @@ -35,7 +35,6 @@ #include #include "CxxModuleWrapper.h" -#include "JNativeRunnable.h" #include "JReactCxxErrorHandler.h" #include "JReactSoftExceptionLogger.h" #include "JavaScriptExecutorHolder.h" @@ -155,8 +154,6 @@ void CatalystInstanceImpl::registerNatives() { "warnOnLegacyNativeModuleSystemUse", CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse), }); - - JNativeRunnable::registerNatives(); } void log(ReactNativeLogLevel level, const char *message) { diff --git a/ReactAndroid/src/main/jni/react/jni/JMessageQueueThread.cpp b/ReactAndroid/src/main/jni/react/jni/JMessageQueueThread.cpp index 5f4caf8051c816..dcb4fcdb9aec7a 100644 --- a/ReactAndroid/src/main/jni/react/jni/JMessageQueueThread.cpp +++ b/ReactAndroid/src/main/jni/react/jni/JMessageQueueThread.cpp @@ -11,11 +11,10 @@ #include #include +#include #include #include -#include "JNativeRunnable.h" - namespace facebook { namespace react { @@ -69,11 +68,10 @@ void JMessageQueueThread::runOnQueue(std::function &&runnable) { jni::ThreadScope guard; static auto method = JavaMessageQueueThread::javaClassStatic() - ->getMethod("runOnQueue"); - method( - m_jobj, - JNativeRunnable::newObjectCxxArgs(wrapRunnable(std::move(runnable))) - .get()); + ->getMethod("runOnQueue"); + auto jrunnable = + JNativeRunnable::newObjectCxxArgs(wrapRunnable(std::move(runnable))); + method(m_jobj, jrunnable.get()); } void JMessageQueueThread::runOnQueueSync(std::function &&runnable) { diff --git a/ReactAndroid/src/main/jni/react/jni/JNativeRunnable.h b/ReactAndroid/src/main/jni/react/jni/JNativeRunnable.h deleted file mode 100644 index cd1a022e4c57e1..00000000000000 --- a/ReactAndroid/src/main/jni/react/jni/JNativeRunnable.h +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and 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 - -using namespace facebook::jni; - -namespace facebook { -namespace react { - -struct Runnable : public JavaClass { - public: - static constexpr auto kJavaDescriptor = "Ljava/lang/Runnable;"; -}; - -/** - * The c++ interface for the Java NativeRunnable class - */ -class JNativeRunnable : public HybridClass { - public: - static auto constexpr kJavaDescriptor = - "Lcom/facebook/react/bridge/queue/NativeRunnable;"; - - void run() { - m_runnable(); - } - - static void registerNatives() { - javaClassStatic()->registerNatives({ - makeNativeMethod("run", JNativeRunnable::run), - }); - } - - private: - friend HybridBase; - - JNativeRunnable(std::function runnable) - : m_runnable(std::move(runnable)) {} - - std::function m_runnable; -}; - -} // namespace react -} // namespace facebook