diff --git a/React/CxxModule/RCTNativeModule.h b/React/CxxModule/RCTNativeModule.h index c73cf5f4ab7..a59f9ae4818 100644 --- a/React/CxxModule/RCTNativeModule.h +++ b/React/CxxModule/RCTNativeModule.h @@ -16,6 +16,7 @@ class RCTNativeModule : public NativeModule { RCTNativeModule(RCTBridge *bridge, RCTModuleData *moduleData); std::string getName() override; + std::string getSyncMethodName(unsigned int methodId) override; std::vector getMethods() override; folly::dynamic getConstants() override; void invoke(unsigned int methodId, folly::dynamic &¶ms, int callId) diff --git a/React/CxxModule/RCTNativeModule.mm b/React/CxxModule/RCTNativeModule.mm index 00737371518..e390f4d8023 100644 --- a/React/CxxModule/RCTNativeModule.mm +++ b/React/CxxModule/RCTNativeModule.mm @@ -15,16 +15,26 @@ #import #import #import +#import #ifdef WITH_FBSYSTRACE #include #endif +namespace { +enum SchedulingContext { Sync, Async }; +} + namespace facebook { namespace react { -static MethodCallResult -invokeInner(RCTBridge *bridge, RCTModuleData *moduleData, unsigned int methodId, const folly::dynamic ¶ms); +static MethodCallResult invokeInner( + RCTBridge *bridge, + RCTModuleData *moduleData, + unsigned int methodId, + const folly::dynamic ¶ms, + int callId, + SchedulingContext context); RCTNativeModule::RCTNativeModule(RCTBridge *bridge, RCTModuleData *moduleData) : m_bridge(bridge), m_moduleData(moduleData) @@ -36,6 +46,11 @@ std::string RCTNativeModule::getName() return [m_moduleData.name UTF8String]; } +std::string RCTNativeModule::getSyncMethodName(unsigned int methodId) +{ + return m_moduleData.methods[methodId].JSMethodName; +} + std::vector RCTNativeModule::getMethods() { std::vector descs; @@ -58,13 +73,26 @@ folly::dynamic RCTNativeModule::getConstants() void RCTNativeModule::invoke(unsigned int methodId, folly::dynamic &¶ms, int callId) { + const char *moduleName = [m_moduleData.name UTF8String]; + const char *methodName = m_moduleData.methods[methodId].JSMethodName; + + dispatch_queue_t queue = m_moduleData.methodQueue; + const bool isSyncModule = queue == RCTJSThread; + + if (isSyncModule) { + NativeModulePerfLogger::getInstance().syncMethodCallStart(moduleName, methodName); + NativeModulePerfLogger::getInstance().syncMethodCallArgConversionStart(moduleName, methodName); + } else { + NativeModulePerfLogger::getInstance().asyncMethodCallStart(moduleName, methodName); + } + // capture by weak pointer so that we can safely use these variables in a callback __weak RCTBridge *weakBridge = m_bridge; __weak RCTModuleData *weakModuleData = m_moduleData; // The BatchedBridge version of this buckets all the callbacks by thread, and // queues one block on each. This is much simpler; we'll see how it goes and // iterate. - dispatch_block_t block = [weakBridge, weakModuleData, methodId, params = std::move(params), callId] { + dispatch_block_t block = [weakBridge, weakModuleData, methodId, params = std::move(params), callId, isSyncModule] { #ifdef WITH_FBSYSTRACE if (callId != -1) { fbsystrace_end_async_flow(TRACE_TAG_REACT_APPS, "native", callId); @@ -72,13 +100,14 @@ void RCTNativeModule::invoke(unsigned int methodId, folly::dynamic &¶ms, int #else (void)(callId); #endif - invokeInner(weakBridge, weakModuleData, methodId, std::move(params)); + invokeInner(weakBridge, weakModuleData, methodId, std::move(params), callId, isSyncModule ? Sync : Async); }; - dispatch_queue_t queue = m_moduleData.methodQueue; - if (queue == RCTJSThread) { + if (isSyncModule) { block(); + NativeModulePerfLogger::getInstance().syncMethodCallReturnConversionEnd(moduleName, methodName); } else if (queue) { + NativeModulePerfLogger::getInstance().asyncMethodCallDispatch(moduleName, methodName); dispatch_async(queue, block); } @@ -90,17 +119,36 @@ void RCTNativeModule::invoke(unsigned int methodId, folly::dynamic &¶ms, int m_moduleData.name); } #endif + + if (isSyncModule) { + NativeModulePerfLogger::getInstance().syncMethodCallEnd(moduleName, methodName); + } else { + NativeModulePerfLogger::getInstance().asyncMethodCallEnd(moduleName, methodName); + } } MethodCallResult RCTNativeModule::callSerializableNativeHook(unsigned int reactMethodId, folly::dynamic &¶ms) { - return invokeInner(m_bridge, m_moduleData, reactMethodId, params); + return invokeInner(m_bridge, m_moduleData, reactMethodId, params, 0, Sync); } -static MethodCallResult -invokeInner(RCTBridge *bridge, RCTModuleData *moduleData, unsigned int methodId, const folly::dynamic ¶ms) +static MethodCallResult invokeInner( + RCTBridge *bridge, + RCTModuleData *moduleData, + unsigned int methodId, + const folly::dynamic ¶ms, + int callId, + SchedulingContext context) { if (!bridge || !bridge.valid || !moduleData) { + if (context == Sync) { + /** + * NOTE: moduleName and methodName are "". This shouldn't be an issue because there can only be one ongoing sync + * call at a time, and when we call syncMethodCallFail, that one call should terminate. This is also an + * exceptional scenario, so it shouldn't occur often. + */ + NativeModulePerfLogger::getInstance().syncMethodCallFail("N/A", "N/A"); + } return folly::none; } @@ -109,11 +157,46 @@ invokeInner(RCTBridge *bridge, RCTModuleData *moduleData, unsigned int methodId, RCTLogError(@"Unknown methodID: %ud for module: %@", methodId, moduleData.name); } + const char *moduleName = [moduleData.name UTF8String]; + const char *methodName = moduleData.methods[methodId].JSMethodName; + + if (context == Async) { + NativeModulePerfLogger::getInstance().asyncMethodCallExecutionStart(moduleName, methodName, (int32_t)callId); + NativeModulePerfLogger::getInstance().asyncMethodCallExecutionArgConversionStart( + moduleName, methodName, (int32_t)callId); + } + NSArray *objcParams = convertFollyDynamicToId(params); + + if (context == Sync) { + NativeModulePerfLogger::getInstance().syncMethodCallArgConversionEnd(moduleName, methodName); + } + @try { + if (context == Sync) { + NativeModulePerfLogger::getInstance().syncMethodCallExecutionStart(moduleName, methodName); + } else { + NativeModulePerfLogger::getInstance().asyncMethodCallExecutionArgConversionEnd( + moduleName, methodName, (int32_t)callId); + } + id result = [method invokeWithBridge:bridge module:moduleData.instance arguments:objcParams]; + + if (context == Sync) { + NativeModulePerfLogger::getInstance().syncMethodCallExecutionEnd(moduleName, methodName); + NativeModulePerfLogger::getInstance().syncMethodCallReturnConversionStart(moduleName, methodName); + } else { + NativeModulePerfLogger::getInstance().asyncMethodCallExecutionEnd(moduleName, methodName, (int32_t)callId); + } + return convertIdToFollyDynamic(result); } @catch (NSException *exception) { + if (context == Sync) { + NativeModulePerfLogger::getInstance().syncMethodCallFail(moduleName, methodName); + } else { + NativeModulePerfLogger::getInstance().asyncMethodCallExecutionFail(moduleName, methodName, (int32_t)callId); + } + // Pass on JS exceptions if ([exception.name hasPrefix:RCTFatalExceptionName]) { @throw exception; diff --git a/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.cpp b/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.cpp index 0ad1f03b37b..e8281b8f1cb 100644 --- a/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.cpp +++ b/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.cpp @@ -50,6 +50,26 @@ std::string JavaNativeModule::getName() { return getNameMethod(wrapper_)->toStdString(); } +std::string JavaNativeModule::getSyncMethodName(unsigned int reactMethodId) { + if (reactMethodId >= syncMethods_.size()) { + throw std::invalid_argument(folly::to( + "methodId ", + reactMethodId, + " out of range [0..", + syncMethods_.size(), + "]")); + } + + auto &methodInvoker = syncMethods_[reactMethodId]; + + if (!methodInvoker.hasValue()) { + throw std::invalid_argument(folly::to( + "methodId ", reactMethodId, " is not a recognized sync method")); + } + + return methodInvoker->getMethodName(); +} + std::vector JavaNativeModule::getMethods() { std::vector ret; syncMethods_.clear(); @@ -69,6 +89,7 @@ std::vector JavaNativeModule::getMethods() { syncMethods_.begin() + methodIndex, MethodInvoker( desc->getMethod(), + methodName, desc->getSignature(), getName() + "." + methodName, true)); @@ -148,6 +169,7 @@ NewJavaNativeModule::NewJavaNativeModule( auto name = desc->getName(); methods_.emplace_back( desc->getMethod(), + desc->getName(), desc->getSignature(), moduleName + "." + name, type == "syncHook"); diff --git a/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.h b/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.h index 5c2d7cb9677..f4b46ce66b4 100644 --- a/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.h +++ b/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.h @@ -69,6 +69,7 @@ class JavaNativeModule : public NativeModule { messageQueueThread_(std::move(messageQueueThread)) {} std::string getName() override; + std::string getSyncMethodName(unsigned int reactMethodId) override; folly::dynamic getConstants() override; std::vector getMethods() override; void invoke(unsigned int reactMethodId, folly::dynamic &¶ms, int callId) diff --git a/ReactAndroid/src/main/jni/react/jni/MethodInvoker.cpp b/ReactAndroid/src/main/jni/react/jni/MethodInvoker.cpp index 571495b7723..496a99cdca3 100644 --- a/ReactAndroid/src/main/jni/react/jni/MethodInvoker.cpp +++ b/ReactAndroid/src/main/jni/react/jni/MethodInvoker.cpp @@ -190,10 +190,12 @@ std::size_t countJsArgs(const std::string &signature) { MethodInvoker::MethodInvoker( alias_ref method, + std::string methodName, std::string signature, std::string traceName, bool isSync) : method_(method->getMethodID()), + methodName_(methodName), signature_(signature), jsArgCount_(countJsArgs(signature) - 2), traceName_(std::move(traceName)), @@ -203,6 +205,10 @@ MethodInvoker::MethodInvoker( << "Non-sync hooks cannot have a non-void return type"; } +std::string MethodInvoker::getMethodName() const { + return methodName_; +} + MethodCallResult MethodInvoker::invoke( std::weak_ptr &instance, alias_ref module, diff --git a/ReactAndroid/src/main/jni/react/jni/MethodInvoker.h b/ReactAndroid/src/main/jni/react/jni/MethodInvoker.h index ee3eb087ddd..67f5fc06d63 100644 --- a/ReactAndroid/src/main/jni/react/jni/MethodInvoker.h +++ b/ReactAndroid/src/main/jni/react/jni/MethodInvoker.h @@ -37,6 +37,7 @@ class MethodInvoker { public: MethodInvoker( jni::alias_ref method, + std::string methodName, std::string signature, std::string traceName, bool isSync); @@ -46,12 +47,15 @@ class MethodInvoker { jni::alias_ref module, const folly::dynamic ¶ms); + std::string getMethodName() const; + bool isSyncHook() const { return isSync_; } private: jmethodID method_; + std::string methodName_; std::string signature_; std::size_t jsArgCount_; std::string traceName_; diff --git a/ReactCommon/cxxreact/CxxNativeModule.cpp b/ReactCommon/cxxreact/CxxNativeModule.cpp index 94ab82bfe0b..bfbb608553b 100644 --- a/ReactCommon/cxxreact/CxxNativeModule.cpp +++ b/ReactCommon/cxxreact/CxxNativeModule.cpp @@ -58,6 +58,18 @@ std::string CxxNativeModule::getName() { return name_; } +std::string CxxNativeModule::getSyncMethodName(unsigned int reactMethodId) { + if (reactMethodId >= methods_.size()) { + throw std::invalid_argument(folly::to( + "methodId ", + reactMethodId, + " out of range [0..", + methods_.size(), + "]")); + } + return methods_[reactMethodId].name; +} + std::vector CxxNativeModule::getMethods() { lazyInit(); diff --git a/ReactCommon/cxxreact/CxxNativeModule.h b/ReactCommon/cxxreact/CxxNativeModule.h index 2c2230409d1..9d730ff7942 100644 --- a/ReactCommon/cxxreact/CxxNativeModule.h +++ b/ReactCommon/cxxreact/CxxNativeModule.h @@ -37,6 +37,7 @@ class RN_EXPORT CxxNativeModule : public NativeModule { messageQueueThread_(messageQueueThread) {} std::string getName() override; + std::string getSyncMethodName(unsigned int methodId) override; std::vector getMethods() override; folly::dynamic getConstants() override; void invoke(unsigned int reactMethodId, folly::dynamic &¶ms, int callId) diff --git a/ReactCommon/cxxreact/ModuleRegistry.cpp b/ReactCommon/cxxreact/ModuleRegistry.cpp index ccffe90c0fc..ac39f975517 100644 --- a/ReactCommon/cxxreact/ModuleRegistry.cpp +++ b/ReactCommon/cxxreact/ModuleRegistry.cpp @@ -195,6 +195,26 @@ folly::Optional ModuleRegistry::getConfig( } } +std::string ModuleRegistry::getModuleName(unsigned int moduleId) { + if (moduleId >= modules_.size()) { + throw std::runtime_error(folly::to( + "moduleId ", moduleId, " out of range [0..", modules_.size(), ")")); + } + + return modules_[moduleId]->getName(); +} + +std::string ModuleRegistry::getModuleSyncMethodName( + unsigned int moduleId, + unsigned int methodId) { + if (moduleId >= modules_.size()) { + throw std::runtime_error(folly::to( + "moduleId ", moduleId, " out of range [0..", modules_.size(), ")")); + } + + return modules_[moduleId]->getSyncMethodName(methodId); +} + void ModuleRegistry::callNativeMethod( unsigned int moduleId, unsigned int methodId, diff --git a/ReactCommon/cxxreact/ModuleRegistry.h b/ReactCommon/cxxreact/ModuleRegistry.h index b24f5390e30..fba4496a621 100644 --- a/ReactCommon/cxxreact/ModuleRegistry.h +++ b/ReactCommon/cxxreact/ModuleRegistry.h @@ -59,6 +59,11 @@ class RN_EXPORT ModuleRegistry { unsigned int methodId, folly::dynamic &&args); + std::string getModuleName(unsigned int moduleId); + std::string getModuleSyncMethodName( + unsigned int moduleId, + unsigned int methodName); + private: // This is always populated std::vector> modules_; diff --git a/ReactCommon/cxxreact/NativeModule.h b/ReactCommon/cxxreact/NativeModule.h index 6e7287cf469..2441953b4a0 100644 --- a/ReactCommon/cxxreact/NativeModule.h +++ b/ReactCommon/cxxreact/NativeModule.h @@ -31,6 +31,7 @@ class NativeModule { public: virtual ~NativeModule() {} virtual std::string getName() = 0; + virtual std::string getSyncMethodName(unsigned int methodId) = 0; virtual std::vector getMethods() = 0; virtual folly::dynamic getConstants() = 0; virtual void diff --git a/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp b/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp index 6490e4d94c5..40655460d27 100644 --- a/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp +++ b/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp @@ -72,6 +72,7 @@ JSIExecutor::JSIExecutor( delegate_(delegate), nativeModules_(std::make_shared( delegate ? delegate->getModuleRegistry() : nullptr)), + moduleRegistry_(delegate ? delegate->getModuleRegistry() : nullptr), scopedTimeoutInvoker_(scopedTimeoutInvoker), runtimeInstaller_(runtimeInstaller) { runtime_->global().setProperty( @@ -443,16 +444,54 @@ Value JSIExecutor::nativeCallSyncHook(const Value *args, size_t count) { folly::to("method parameters should be array")); } + unsigned int moduleId = static_cast(args[0].getNumber()); + unsigned int methodId = static_cast(args[1].getNumber()); + std::string moduleName; + std::string methodName; + + if (moduleRegistry_) { + moduleName = moduleRegistry_->getModuleName(moduleId); + methodName = moduleRegistry_->getModuleSyncMethodName(moduleId, methodId); + + NativeModulePerfLogger::getInstance().syncMethodCallStart( + moduleName.c_str(), methodName.c_str()); + + NativeModulePerfLogger::getInstance().syncMethodCallArgConversionStart( + moduleName.c_str(), methodName.c_str()); + } + MethodCallResult result = delegate_->callSerializableNativeHook( - *this, - static_cast(args[0].getNumber()), // moduleId - static_cast(args[1].getNumber()), // methodId - dynamicFromValue(*runtime_, args[2])); // args + *this, moduleId, methodId, dynamicFromValue(*runtime_, args[2])); + + /** + * Note: + * In RCTNativeModule, folly::none is returned from callSerializableNativeHook + * when executing a NativeModule method fails. Therefore, it's safe to not + * terminate the syncMethodCall when folly::none is returned. + * + * TODO: In JavaNativeModule, folly::none is returned when the synchronous + * NativeModule method has the void return type. Change this to return + * folly::dynamic(nullptr) instead, so that folly::none is reserved for + * exceptional scenarios. + * + * TODO: Investigate CxxModule infra to see if folly::none is used for + * returns in exceptional scenarios. + **/ if (!result.hasValue()) { return Value::undefined(); } - return valueFromDynamic(*runtime_, result.value()); + + Value returnValue = valueFromDynamic(*runtime_, result.value()); + + if (moduleRegistry_) { + NativeModulePerfLogger::getInstance().syncMethodCallReturnConversionEnd( + moduleName.c_str(), methodName.c_str()); + NativeModulePerfLogger::getInstance().syncMethodCallEnd( + moduleName.c_str(), methodName.c_str()); + } + + return returnValue; } #if DEBUG diff --git a/ReactCommon/jsiexecutor/jsireact/JSIExecutor.h b/ReactCommon/jsiexecutor/jsireact/JSIExecutor.h index 2cd3776baf5..615172ed711 100644 --- a/ReactCommon/jsiexecutor/jsireact/JSIExecutor.h +++ b/ReactCommon/jsiexecutor/jsireact/JSIExecutor.h @@ -123,6 +123,7 @@ class JSIExecutor : public JSExecutor { std::shared_ptr runtime_; std::shared_ptr delegate_; std::shared_ptr nativeModules_; + std::shared_ptr moduleRegistry_; std::once_flag bindFlag_; std::unique_ptr bundleRegistry_; JSIScopedTimeoutInvoker scopedTimeoutInvoker_;