From 3337add547c60b84816ef5dad82f4ead2e8742ef Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Fri, 13 May 2022 07:12:48 -0700 Subject: [PATCH] Pass string by ref in TurboModule lookup path Summary: Avoid unnecessary string copies Changelog: [internal] Reviewed By: nlutsenko Differential Revision: D36312750 fbshipit-source-id: caf0985f988eb497de3be3c0526809593b01a9e2 --- .../CompositeTurboModuleManagerDelegate.cpp | 6 +++--- .../CompositeTurboModuleManagerDelegate.h | 6 +++--- .../jni/ReactCommon/TurboModuleManagerDelegate.h | 6 +++--- .../android/ReactCommon/SampleTurboModuleSpec.cpp | 2 +- .../android/ReactCommon/SampleTurboModuleSpec.h | 2 +- .../src/generators/modules/GenerateModuleJniCpp.js | 2 +- .../src/generators/modules/GenerateModuleJniH.js | 2 +- .../GenerateModuleJniCpp-test.js.snap | 14 +++++++------- .../__snapshots__/GenerateModuleJniH-test.js.snap | 14 +++++++------- .../app/src/main/jni/RNTesterAppModuleProvider.cpp | 2 +- .../app/src/main/jni/RNTesterAppModuleProvider.h | 2 +- .../jni/RNTesterTurboModuleManagerDelegate.cpp | 8 ++++---- .../main/jni/RNTesterTurboModuleManagerDelegate.h | 8 ++++---- 13 files changed, 37 insertions(+), 37 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/CompositeTurboModuleManagerDelegate.cpp b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/CompositeTurboModuleManagerDelegate.cpp index a84c819e429435..dedcff8592ad62 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/CompositeTurboModuleManagerDelegate.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/CompositeTurboModuleManagerDelegate.cpp @@ -27,8 +27,8 @@ void CompositeTurboModuleManagerDelegate::registerNatives() { std::shared_ptr CompositeTurboModuleManagerDelegate::getTurboModule( - const std::string moduleName, - const std::shared_ptr jsInvoker) { + const std::string &moduleName, + const std::shared_ptr &jsInvoker) { for (auto delegate : mDelegates_) { if (auto turboModule = delegate->getTurboModule(moduleName, jsInvoker)) { return turboModule; @@ -39,7 +39,7 @@ CompositeTurboModuleManagerDelegate::getTurboModule( std::shared_ptr CompositeTurboModuleManagerDelegate::getTurboModule( - const std::string moduleName, + const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { for (auto delegate : mDelegates_) { if (auto turboModule = delegate->getTurboModule(moduleName, params)) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/CompositeTurboModuleManagerDelegate.h b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/CompositeTurboModuleManagerDelegate.h index 12b370d66c6c83..257d87b53b921d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/CompositeTurboModuleManagerDelegate.h +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/CompositeTurboModuleManagerDelegate.h @@ -29,10 +29,10 @@ class CompositeTurboModuleManagerDelegate static void registerNatives(); std::shared_ptr getTurboModule( - const std::string moduleName, - const std::shared_ptr jsInvoker) override; + const std::string &moduleName, + const std::shared_ptr &jsInvoker) override; std::shared_ptr getTurboModule( - const std::string moduleName, + const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) override; private: diff --git a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManagerDelegate.h b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManagerDelegate.h index 76485439ac3ccc..8e9dcb0a8e3947 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManagerDelegate.h +++ b/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/jni/ReactCommon/TurboModuleManagerDelegate.h @@ -23,11 +23,11 @@ class TurboModuleManagerDelegate "Lcom/facebook/react/turbomodule/core/TurboModuleManagerDelegate;"; virtual std::shared_ptr getTurboModule( - const std::string name, + const std::string &name, const JavaTurboModule::InitParams ¶ms) = 0; virtual std::shared_ptr getTurboModule( - const std::string name, - const std::shared_ptr jsInvoker) = 0; + const std::string &name, + const std::shared_ptr &jsInvoker) = 0; private: friend HybridBase; diff --git a/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp b/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp index 88070dad57ba18..c31e051333b0ba 100644 --- a/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp +++ b/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp @@ -197,7 +197,7 @@ NativeSampleTurboModuleSpecJSI::NativeSampleTurboModuleSpecJSI( } std::shared_ptr SampleTurboModuleSpec_ModuleProvider( - const std::string moduleName, + const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { if (moduleName == "SampleTurboModule") { return std::make_shared(params); diff --git a/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.h b/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.h index 695cb8764a5ef8..0754f9a9bba676 100644 --- a/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.h +++ b/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.h @@ -25,7 +25,7 @@ class JSI_EXPORT NativeSampleTurboModuleSpecJSI : public JavaTurboModule { }; std::shared_ptr SampleTurboModuleSpec_ModuleProvider( - const std::string moduleName, + const std::string &moduleName, const JavaTurboModule::InitParams ¶ms); } // namespace react diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleJniCpp.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleJniCpp.js index a000849647d5b1..447d6e8529ee13 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleJniCpp.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleJniCpp.js @@ -114,7 +114,7 @@ namespace react { ${modules} -std::shared_ptr ${libraryName}_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms) { +std::shared_ptr ${libraryName}_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { ${moduleLookups.map(ModuleLookupTemplate).join('\n')} return nullptr; } diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleJniH.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleJniH.js index 8825b05a589d28..30f3aa2e12493f 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleJniH.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleJniH.js @@ -54,7 +54,7 @@ namespace react { ${modules} -std::shared_ptr ${libraryName}_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms); +std::shared_ptr ${libraryName}_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms); } // namespace react } // namespace facebook diff --git a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJniCpp-test.js.snap b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJniCpp-test.js.snap index efffaf9fffe7c1..b1a293a45c8bc8 100644 --- a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJniCpp-test.js.snap +++ b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJniCpp-test.js.snap @@ -24,7 +24,7 @@ NativeSampleTurboModuleSpecJSI::NativeSampleTurboModuleSpecJSI(const JavaTurboMo } -std::shared_ptr SampleWithUppercaseName_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms) { +std::shared_ptr SampleWithUppercaseName_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { if (moduleName == \\"SampleTurboModule\\") { return std::make_shared(params); } @@ -93,7 +93,7 @@ NativeSampleTurboModuleSpecJSI::NativeSampleTurboModuleSpecJSI(const JavaTurboMo methodMap_[\\"getNullableArray\\"] = MethodMetadata {0, __hostFunction_NativeSampleTurboModuleSpecJSI_getNullableArray}; } -std::shared_ptr complex_objects_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms) { +std::shared_ptr complex_objects_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { if (moduleName == \\"SampleTurboModule\\") { return std::make_shared(params); } @@ -130,7 +130,7 @@ NativeSampleTurboModuleSpecJSI::NativeSampleTurboModuleSpecJSI(const JavaTurboMo } -std::shared_ptr empty_native_modules_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms) { +std::shared_ptr empty_native_modules_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { if (moduleName == \\"SampleTurboModule\\") { return std::make_shared(params); } @@ -171,7 +171,7 @@ AliasTurboModuleSpecJSI::AliasTurboModuleSpecJSI(const JavaTurboModule::InitPara methodMap_[\\"cropImage\\"] = MethodMetadata {1, __hostFunction_AliasTurboModuleSpecJSI_cropImage}; } -std::shared_ptr native_modules_with_type_aliases_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms) { +std::shared_ptr native_modules_with_type_aliases_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { if (moduleName == \\"AliasTurboModule\\") { return std::make_shared(params); } @@ -250,7 +250,7 @@ NativeExceptionsManagerSpecJSI::NativeExceptionsManagerSpecJSI(const JavaTurboMo methodMap_[\\"dismissRedbox\\"] = MethodMetadata {0, __hostFunction_NativeExceptionsManagerSpecJSI_dismissRedbox}; } -std::shared_ptr real_module_example_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms) { +std::shared_ptr real_module_example_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { if (moduleName == \\"CameraRollManager\\") { return std::make_shared(params); } @@ -342,7 +342,7 @@ NativeSampleTurboModuleSpecJSI::NativeSampleTurboModuleSpecJSI(const JavaTurboMo methodMap_[\\"getValueWithPromise\\"] = MethodMetadata {1, __hostFunction_NativeSampleTurboModuleSpecJSI_getValueWithPromise}; } -std::shared_ptr simple_native_modules_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms) { +std::shared_ptr simple_native_modules_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { if (moduleName == \\"SampleTurboModule\\") { return std::make_shared(params); } @@ -391,7 +391,7 @@ NativeSampleTurboModule2SpecJSI::NativeSampleTurboModule2SpecJSI(const JavaTurbo methodMap_[\\"voidFunc\\"] = MethodMetadata {0, __hostFunction_NativeSampleTurboModule2SpecJSI_voidFunc}; } -std::shared_ptr two_modules_different_files_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms) { +std::shared_ptr two_modules_different_files_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { if (moduleName == \\"SampleTurboModule\\") { return std::make_shared(params); } diff --git a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJniH-test.js.snap b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJniH-test.js.snap index 0ab8a73f50bfa6..2a041e62bb68ce 100644 --- a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJniH-test.js.snap +++ b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleJniH-test.js.snap @@ -30,7 +30,7 @@ public: }; -std::shared_ptr SampleWithUppercaseName_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms); +std::shared_ptr SampleWithUppercaseName_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms); } // namespace react } // namespace facebook @@ -139,7 +139,7 @@ public: }; -std::shared_ptr complex_objects_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms); +std::shared_ptr complex_objects_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms); } // namespace react } // namespace facebook @@ -248,7 +248,7 @@ public: }; -std::shared_ptr empty_native_modules_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms); +std::shared_ptr empty_native_modules_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms); } // namespace react } // namespace facebook @@ -357,7 +357,7 @@ public: }; -std::shared_ptr native_modules_with_type_aliases_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms); +std::shared_ptr native_modules_with_type_aliases_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms); } // namespace react } // namespace facebook @@ -474,7 +474,7 @@ public: }; -std::shared_ptr real_module_example_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms); +std::shared_ptr real_module_example_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms); } // namespace react } // namespace facebook @@ -583,7 +583,7 @@ public: }; -std::shared_ptr simple_native_modules_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms); +std::shared_ptr simple_native_modules_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms); } // namespace react } // namespace facebook @@ -700,7 +700,7 @@ public: }; -std::shared_ptr two_modules_different_files_ModuleProvider(const std::string moduleName, const JavaTurboModule::InitParams ¶ms); +std::shared_ptr two_modules_different_files_ModuleProvider(const std::string &moduleName, const JavaTurboModule::InitParams ¶ms); } // namespace react } // namespace facebook diff --git a/packages/rn-tester/android/app/src/main/jni/RNTesterAppModuleProvider.cpp b/packages/rn-tester/android/app/src/main/jni/RNTesterAppModuleProvider.cpp index 736546f70d1ca0..76104b7455ccd9 100644 --- a/packages/rn-tester/android/app/src/main/jni/RNTesterAppModuleProvider.cpp +++ b/packages/rn-tester/android/app/src/main/jni/RNTesterAppModuleProvider.cpp @@ -15,7 +15,7 @@ namespace facebook { namespace react { std::shared_ptr RNTesterAppModuleProvider( - const std::string moduleName, + const std::string &moduleName, const JavaTurboModule::InitParams ¶ms) { auto module = rntester_ModuleProvider(moduleName, params); if (module != nullptr) { diff --git a/packages/rn-tester/android/app/src/main/jni/RNTesterAppModuleProvider.h b/packages/rn-tester/android/app/src/main/jni/RNTesterAppModuleProvider.h index be885b463efb55..033031c495bdba 100644 --- a/packages/rn-tester/android/app/src/main/jni/RNTesterAppModuleProvider.h +++ b/packages/rn-tester/android/app/src/main/jni/RNTesterAppModuleProvider.h @@ -16,7 +16,7 @@ namespace facebook { namespace react { std::shared_ptr RNTesterAppModuleProvider( - const std::string moduleName, + const std::string &moduleName, const JavaTurboModule::InitParams ¶ms); } // namespace react diff --git a/packages/rn-tester/android/app/src/main/jni/RNTesterTurboModuleManagerDelegate.cpp b/packages/rn-tester/android/app/src/main/jni/RNTesterTurboModuleManagerDelegate.cpp index 3321a1adc751b0..6bf5c8e8afaebc 100644 --- a/packages/rn-tester/android/app/src/main/jni/RNTesterTurboModuleManagerDelegate.cpp +++ b/packages/rn-tester/android/app/src/main/jni/RNTesterTurboModuleManagerDelegate.cpp @@ -28,20 +28,20 @@ void RNTesterTurboModuleManagerDelegate::registerNatives() { } std::shared_ptr RNTesterTurboModuleManagerDelegate::getTurboModule( - const std::string name, - const std::shared_ptr jsInvoker) { + const std::string &name, + const std::shared_ptr &jsInvoker) { // Not implemented yet: provide pure-C++ NativeModules here. return nullptr; } std::shared_ptr RNTesterTurboModuleManagerDelegate::getTurboModule( - const std::string name, + const std::string &name, const JavaTurboModule::InitParams ¶ms) { return RNTesterAppModuleProvider(name, params); } bool RNTesterTurboModuleManagerDelegate::canCreateTurboModule( - std::string name) { + const std::string &name) { return getTurboModule(name, nullptr) != nullptr || getTurboModule(name, {.moduleName = name}) != nullptr; } diff --git a/packages/rn-tester/android/app/src/main/jni/RNTesterTurboModuleManagerDelegate.h b/packages/rn-tester/android/app/src/main/jni/RNTesterTurboModuleManagerDelegate.h index ae91d05089e2fb..d735d4dc521a39 100644 --- a/packages/rn-tester/android/app/src/main/jni/RNTesterTurboModuleManagerDelegate.h +++ b/packages/rn-tester/android/app/src/main/jni/RNTesterTurboModuleManagerDelegate.h @@ -27,17 +27,17 @@ class RNTesterTurboModuleManagerDelegate static void registerNatives(); std::shared_ptr getTurboModule( - const std::string name, - const std::shared_ptr jsInvoker) override; + const std::string &name, + const std::shared_ptr &jsInvoker) override; std::shared_ptr getTurboModule( - const std::string name, + const std::string &name, const JavaTurboModule::InitParams ¶ms) override; /** * Test-only method. Allows user to verify whether a TurboModule can be * created by instances of this class. */ - bool canCreateTurboModule(std::string name); + bool canCreateTurboModule(const std::string &name); private: friend HybridBase;