From 0d9e054ec4c61fc61d67fe56627b226bb6cf6413 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Mon, 9 May 2022 02:07:26 -0700 Subject: [PATCH] Cleanup JBackgroundExecutor and support thread naming (#33780) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/33780 Currently this just has a default ThreadPool name, which can be confusing in systraces and crash reports. Changelog: [Internal] Reviewed By: ryancat Differential Revision: D36200090 fbshipit-source-id: 22918993e7c822ed721ccaf79cdcd9d2a972193d --- .../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 0c56d2227f..083288bed7 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 @@ package com.facebook.react.bridge; 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 71bbef3fb6..491288cd68 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 9b0a1fd49c..38a392e929 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 c742a4555b..88117cbb84 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 1cc2615287..f1bad8d100 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