From a897314f8236f80f2372341f5e217d6451ba29ba Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Thu, 12 May 2022 05:49:01 -0700 Subject: [PATCH] Cleanup TurboModule constructor and headers Summary: Avoid copying of std::string and pass std::shared_ptr by reference where possible. Changelog: [Internal] Reviewed By: appden Differential Revision: D36311151 fbshipit-source-id: b0cc791e5eb8353c172d256c69b166d2bdd0db27 --- .../core/ReactCommon/TurboModule.cpp | 6 +-- .../core/ReactCommon/TurboModule.h | 2 +- .../android/ReactCommon/JavaTurboModule.cpp | 51 ++++++++++--------- .../android/ReactCommon/JavaTurboModule.h | 24 ++------- 4 files changed, 32 insertions(+), 51 deletions(-) diff --git a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.cpp b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.cpp index 094e36b25c5855..3e27164946a84c 100644 --- a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.cpp +++ b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.cpp @@ -7,15 +7,13 @@ #include "TurboModule.h" -using namespace facebook; - namespace facebook { namespace react { TurboModule::TurboModule( - const std::string &name, + std::string name, std::shared_ptr jsInvoker) - : name_(name), jsInvoker_(jsInvoker) {} + : name_(std::move(name)), jsInvoker_(std::move(jsInvoker)) {} } // namespace react } // namespace facebook diff --git a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.h b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.h index 7d9189f10e7e10..4ca5b5c77f6fe7 100644 --- a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.h +++ b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.h @@ -37,7 +37,7 @@ enum TurboModuleMethodValueKind { */ class JSI_EXPORT TurboModule : public facebook::jsi::HostObject { public: - TurboModule(const std::string &name, std::shared_ptr jsInvoker); + TurboModule(std::string name, std::shared_ptr jsInvoker); virtual facebook::jsi::Value get( facebook::jsi::Runtime &runtime, diff --git a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index b427dcbfba024d..dbea5632fac2ff 100644 --- a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -22,11 +23,11 @@ #include "JavaTurboModule.h" -namespace TMPL = facebook::react::TurboModulePerfLogger; - namespace facebook { namespace react { +namespace TMPL = TurboModulePerfLogger; + JavaTurboModule::JavaTurboModule(const InitParams ¶ms) : TurboModule(params.moduleName, params.jsInvoker), instance_(jni::make_global(params.instance)), @@ -54,11 +55,18 @@ JavaTurboModule::~JavaTurboModule() { } namespace { + +struct JNIArgs { + JNIArgs(size_t count) : args_(count) {} + std::vector args_; + std::vector globalRefs_; +}; + jni::local_ref createJavaCallbackFromJSIFunction( - JSCallbackRetainer retainJSCallback, + const JSCallbackRetainer &retainJSCallback, jsi::Function &&function, jsi::Runtime &rt, - std::shared_ptr jsInvoker) { + const std::shared_ptr &jsInvoker) { auto weakWrapper = retainJSCallback != nullptr ? retainJSCallback(std::move(function), rt, jsInvoker) : react::CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); @@ -119,13 +127,6 @@ jni::local_ref createJavaCallbackFromJSIFunction( return JCxxCallbackImpl::newObjectCxxArgs(fn); } -template -std::string to_string(T v) { - std::ostringstream stream; - stream << v; - return stream.str(); -} - // This is used for generating short exception strings. std::string stringifyJSIValue(const jsi::Value &v, jsi::Runtime *rt = nullptr) { if (v.isUndefined()) { @@ -141,7 +142,7 @@ std::string stringifyJSIValue(const jsi::Value &v, jsi::Runtime *rt = nullptr) { } if (v.isNumber()) { - return "a number (" + to_string(v.getNumber()) + ")"; + return "a number (" + std::to_string(v.getNumber()) + ")"; } if (v.isString()) { @@ -162,7 +163,7 @@ class JavaTurboModuleArgumentConversionException : public std::runtime_error { const jsi::Value *arg, jsi::Runtime *rt) : std::runtime_error( - "Expected argument " + to_string(index) + " of method \"" + + "Expected argument " + std::to_string(index) + " of method \"" + methodName + "\" to be a " + expectedType + ", but got " + stringifyJSIValue(*arg, rt)) {} }; @@ -175,7 +176,7 @@ class JavaTurboModuleInvalidArgumentTypeException : public std::runtime_error { const std::string &methodName) : std::runtime_error( "Called method \"" + methodName + "\" with unsupported type " + - actualType + " at argument " + to_string(argIndex)) {} + actualType + " at argument " + std::to_string(argIndex)) {} }; class JavaTurboModuleInvalidArgumentCountException : public std::runtime_error { @@ -186,9 +187,9 @@ class JavaTurboModuleInvalidArgumentCountException : public std::runtime_error { int expectedArgCount) : std::runtime_error( "TurboModule method \"" + methodName + "\" called with " + - to_string(actualArgCount) + + std::to_string(actualArgCount) + " arguments (expected argument count: " + - to_string(expectedArgCount) + ").") {} + std::to_string(expectedArgCount) + ").") {} }; /** @@ -240,22 +241,20 @@ int32_t getUniqueId() { return counter++; } -} // namespace - -// fnjni already does this conversion, but since we are using plain JNI, this +// fbjni already does this conversion, but since we are using plain JNI, this // needs to be done again // TODO (axe) Reuse existing implementation as needed - the exist in // MethodInvoker.cpp -JNIArgs JavaTurboModule::convertJSIArgsToJNIArgs( +JNIArgs convertJSIArgsToJNIArgs( JNIEnv *env, jsi::Runtime &rt, - std::string methodName, - std::vector methodArgTypes, + const std::string &methodName, + const std::vector &methodArgTypes, const jsi::Value *args, size_t count, - std::shared_ptr jsInvoker, + const std::shared_ptr &jsInvoker, TurboModuleMethodValueKind valueKind, - JSCallbackRetainer retainJSCallback) { + const JSCallbackRetainer &retainJSCallback) { unsigned int expectedArgumentCount = valueKind == PromiseKind ? methodArgTypes.size() - 1 : methodArgTypes.size(); @@ -285,7 +284,7 @@ JNIArgs JavaTurboModule::convertJSIArgsToJNIArgs( jclass doubleClass = nullptr; for (unsigned int argIndex = 0; argIndex < count; argIndex += 1) { - std::string type = methodArgTypes.at(argIndex); + const std::string &type = methodArgTypes.at(argIndex); const jsi::Value *arg = &args[argIndex]; jvalue *jarg = &jargs[argIndex]; @@ -430,6 +429,8 @@ jsi::Value convertFromJMapToValue(JNIEnv *env, jsi::Runtime &rt, jobject arg) { return jsi::valueFromDynamic(rt, result->cthis()->consume()); } +} // namespace + jsi::Value JavaTurboModule::invokeJavaMethod( jsi::Runtime &runtime, TurboModuleMethodValueKind valueKind, diff --git a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h index 47a682b24a6106..fda5cc8a07d24b 100644 --- a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h +++ b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.h @@ -11,22 +11,15 @@ #include #include -#include +#include #include -#include -#include #include +#include #include namespace facebook { namespace react { -struct JNIArgs { - JNIArgs(size_t count) : args_(count) {} - std::vector args_; - std::vector globalRefs_; -}; - struct JTurboModule : jni::JavaClass { static auto constexpr kJavaDescriptor = "Lcom/facebook/react/turbomodule/core/interfaces/TurboModule;"; @@ -35,7 +28,7 @@ struct JTurboModule : jni::JavaClass { using JSCallbackRetainer = std::function( jsi::Function &&callback, jsi::Runtime &runtime, - std::shared_ptr jsInvoker)>; + const std::shared_ptr &jsInvoker)>; class JSI_EXPORT JavaTurboModule : public TurboModule { public: @@ -63,17 +56,6 @@ class JSI_EXPORT JavaTurboModule : public TurboModule { jni::global_ref instance_; std::shared_ptr nativeInvoker_; JSCallbackRetainer retainJSCallback_; - - JNIArgs convertJSIArgsToJNIArgs( - JNIEnv *env, - jsi::Runtime &rt, - std::string methodName, - std::vector methodArgTypes, - const jsi::Value *args, - size_t count, - std::shared_ptr jsInvoker, - TurboModuleMethodValueKind valueKind, - JSCallbackRetainer retainJSCallbacks); }; } // namespace react