From 62210531792fd3f07ebc0aeefa19d37ebc7da707 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Fri, 12 May 2017 17:57:12 -0700 Subject: [PATCH] Improve systrace markers Reviewed By: mhorowitz Differential Revision: D4860135 fbshipit-source-id: ce963010883e6b9cc8e34f7ff01b4018cd195eba --- React/CxxBridge/RCTCxxBridge.mm | 12 ++++---- React/CxxModule/RCTNativeModule.h | 2 +- React/CxxModule/RCTNativeModule.mm | 22 +++++++++------ React/Modules/RCTUIManager.m | 5 ++-- React/Profiler/RCTProfile.h | 4 +-- React/Profiler/RCTProfile.m | 26 ++++++++--------- .../main/jni/xreact/jni/JavaModuleWrapper.cpp | 22 ++++++++++++--- .../main/jni/xreact/jni/JavaModuleWrapper.h | 4 +-- ReactCommon/cxxreact/CxxNativeModule.cpp | 13 +++++++-- ReactCommon/cxxreact/CxxNativeModule.h | 2 +- ReactCommon/cxxreact/JSCExecutor.cpp | 28 ++++++++----------- ReactCommon/cxxreact/ModuleRegistry.cpp | 9 +----- ReactCommon/cxxreact/NativeModule.h | 4 +-- ReactCommon/cxxreact/NativeToJsBridge.cpp | 17 ++++------- ReactCommon/jschelpers/JSCHelpers.cpp | 3 -- ReactCommon/jschelpers/Value.h | 28 +++++++++++++++---- 16 files changed, 111 insertions(+), 90 deletions(-) diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index ad3f209bc..8dd9179bf 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -54,7 +54,7 @@ RCTAssert(self.executorClass || self->_jsThread == [NSThread currentThread], \ @"This method must be called on JS thread") -NSString *const RCTJSThreadName = @"com.facebook.React.JavaScript"; +static NSString *const RCTJSThreadName = @"com.facebook.react.JavaScript"; using namespace facebook::react; @@ -76,7 +76,7 @@ static bool isRAMBundle(NSData *script) { static void registerPerformanceLoggerHooks(RCTPerformanceLogger *performanceLogger) { __weak RCTPerformanceLogger *weakPerformanceLogger = performanceLogger; - ReactMarker::logTaggedMarker = [weakPerformanceLogger](const ReactMarker::ReactMarkerId markerId, const char* tag) { + ReactMarker::logTaggedMarker = [weakPerformanceLogger](const ReactMarker::ReactMarkerId markerId, const char *tag) { switch (markerId) { case ReactMarker::RUN_JS_BUNDLE_START: [weakPerformanceLogger markStartForTag:RCTPLScriptExecution]; @@ -255,6 +255,8 @@ struct RCTInstanceCallback : public InstanceCallback { - (void)start { + RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, @"-[RCTCxxBridge start]", nil); + [[NSNotificationCenter defaultCenter] postNotificationName:RCTJavaScriptWillStartLoadingNotification object:_parentBridge userInfo:@{@"bridge": self}]; @@ -349,6 +351,7 @@ struct RCTInstanceCallback : public InstanceCallback { } }); } + RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @""); } - (void)loadSource:(RCTSourceLoadBlock)_onSourceLoad onProgress:(RCTSourceLoadProgressBlock)onProgress @@ -963,7 +966,7 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR [_performanceLogger markStopForTag:RCTPLBridgeStartup]; - RCT_PROFILE_BEGIN_EVENT(0, @"Processing pendingCalls", @{ @"count": @(_pendingCalls.count) }); + RCT_PROFILE_BEGIN_EVENT(0, @"Processing pendingCalls", @{ @"count": [@(_pendingCalls.count) stringValue] }); // Phase B: _flushPendingCalls happens. Each block in _pendingCalls is // executed, adding work to the queue, and _pendingCount is decremented. // loading is set to NO. @@ -1051,8 +1054,7 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR { RCTAssert(onComplete != nil, @"onComplete block passed in should be non-nil"); - RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, - @"-[RCTCxxBridge enqueueApplicationScript]", nil); + RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, @"-[RCTCxxBridge enqueueApplicationScript]", nil); [self _tryAndHandleError:^{ if (isRAMBundle(script)) { diff --git a/React/CxxModule/RCTNativeModule.h b/React/CxxModule/RCTNativeModule.h index e4d1b0892..2245f8c94 100644 --- a/React/CxxModule/RCTNativeModule.h +++ b/React/CxxModule/RCTNativeModule.h @@ -20,7 +20,7 @@ class RCTNativeModule : public NativeModule { std::string getName() override; std::vector getMethods() override; folly::dynamic getConstants() override; - void invoke(unsigned int methodId, folly::dynamic &¶ms) override; + void invoke(unsigned int methodId, folly::dynamic &¶ms, int callId) override; MethodCallResult callSerializableNativeHook(unsigned int reactMethodId, folly::dynamic &¶ms) override; private: diff --git a/React/CxxModule/RCTNativeModule.mm b/React/CxxModule/RCTNativeModule.mm index 3402bc098..7e51af062 100644 --- a/React/CxxModule/RCTNativeModule.mm +++ b/React/CxxModule/RCTNativeModule.mm @@ -17,6 +17,10 @@ #import #import +#ifdef WITH_FBSYSTRACE +#include +#endif + namespace facebook { namespace react { @@ -50,20 +54,20 @@ folly::dynamic RCTNativeModule::getConstants() { return ret; } -void RCTNativeModule::invoke(unsigned int methodId, folly::dynamic &¶ms) { +void RCTNativeModule::invoke(unsigned int methodId, folly::dynamic &¶ms, int callId) { // 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 = [this, methodId, params=std::move(params)] { - if (!m_bridge.valid) { - return; + dispatch_block_t block = [this, methodId, params=std::move(params), callId] { + #ifdef WITH_FBSYSTRACE + if (callId != -1) { + fbsystrace_end_async_flow(TRACE_TAG_REACT_APPS, "native", callId); } - + #endif invokeInner(methodId, std::move(params)); }; dispatch_queue_t queue = m_moduleData.methodQueue; - if (queue == RCTJSThread) { block(); } else if (queue) { @@ -76,6 +80,10 @@ MethodCallResult RCTNativeModule::callSerializableNativeHook(unsigned int reactM } MethodCallResult RCTNativeModule::invokeInner(unsigned int methodId, const folly::dynamic &¶ms) { + if (!m_bridge.valid) { + return folly::none; + } + id method = m_moduleData.methods[methodId]; if (RCT_DEBUG && !method) { RCTLogError(@"Unknown methodID: %ud for module: %@", @@ -83,7 +91,6 @@ MethodCallResult RCTNativeModule::invokeInner(unsigned int methodId, const folly } NSArray *objcParams = convertFollyDynamicToId(params); - @try { id result = [method invokeWithBridge:m_bridge module:m_moduleData.instance arguments:objcParams]; return convertIdToFollyDynamic(result); @@ -99,7 +106,6 @@ MethodCallResult RCTNativeModule::invokeInner(unsigned int methodId, const folly exception, method.JSMethodName, m_moduleData.name, objcParams]; RCTFatal(RCTErrorWithMessage(message)); } - } } diff --git a/React/Modules/RCTUIManager.m b/React/Modules/RCTUIManager.m index 2147b4eaf..642399575 100644 --- a/React/Modules/RCTUIManager.m +++ b/React/Modules/RCTUIManager.m @@ -32,8 +32,8 @@ #import "RCTRootViewInternal.h" #import "RCTScrollableProtocol.h" #import "RCTShadowView.h" -#import "RCTUtils.h" #import "RCTUIManagerObserverCoordinator.h" +#import "RCTUtils.h" #import "RCTView.h" #import "RCTViewManager.h" #import "UIView+React.h" @@ -1192,7 +1192,7 @@ RCT_EXPORT_METHOD(dispatchViewManagerCommand:(nonnull NSNumber *)reactTag dispatch_async(dispatch_get_main_queue(), ^{ RCTProfileEndFlowEvent(); RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, @"-[UIManager flushUIBlocks]", (@{ - @"count": @(previousPendingUIBlocks.count), + @"count": [@(previousPendingUIBlocks.count) stringValue], })); @try { for (RCTViewManagerUIBlock block in previousPendingUIBlocks) { @@ -1202,6 +1202,7 @@ RCT_EXPORT_METHOD(dispatchViewManagerCommand:(nonnull NSNumber *)reactTag @catch (NSException *exception) { RCTLogError(@"Exception thrown while executing UI block: %@", exception); } + RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @""); }); } } diff --git a/React/Profiler/RCTProfile.h b/React/Profiler/RCTProfile.h index 73cb3e869..b0ab6cd81 100644 --- a/React/Profiler/RCTProfile.h +++ b/React/Profiler/RCTProfile.h @@ -68,7 +68,7 @@ RCT_EXTERN void _RCTProfileBeginEvent(NSThread *calleeThread, NSTimeInterval time, uint64_t tag, NSString *name, - NSDictionary *args); + NSDictionary *args); #define RCT_PROFILE_BEGIN_EVENT(tag, name, args) \ do { \ if (RCTProfileIsProfiling()) { \ @@ -104,7 +104,7 @@ RCT_EXTERN void _RCTProfileEndEvent(NSThread *calleeThread, */ RCT_EXTERN NSUInteger RCTProfileBeginAsyncEvent(uint64_t tag, NSString *name, - NSDictionary *args); + NSDictionary *args); /** * The ID returned by BeginEvent should then be passed into EndEvent, with the diff --git a/React/Profiler/RCTProfile.m b/React/Profiler/RCTProfile.m index eb5a0b9a5..ec1046c03 100644 --- a/React/Profiler/RCTProfile.m +++ b/React/Profiler/RCTProfile.m @@ -71,7 +71,7 @@ if (!RCTProfileIsProfiling()) { \ static RCTProfileCallbacks *callbacks; static char *systrace_buffer; -static systrace_arg_t *RCTProfileSystraceArgsFromNSDictionary(NSDictionary *args) +static systrace_arg_t *systraceArgsFromDictionary(NSDictionary *args) { if (args.count == 0) { return NULL; @@ -79,14 +79,11 @@ static systrace_arg_t *RCTProfileSystraceArgsFromNSDictionary(NSDictionary *args systrace_arg_t *systrace_args = malloc(sizeof(systrace_arg_t) * args.count); __block size_t i = 0; - [args enumerateKeysAndObjectsUsingBlock:^(id key, id value, __unused BOOL *stop) { - const char *keyc = [key description].UTF8String; - systrace_args[i].key = keyc; - systrace_args[i].key_len = (int)strlen(keyc); - - const char *valuec = RCTJSONStringify(value, NULL).UTF8String; - systrace_args[i].value = valuec; - systrace_args[i].value_len = (int)strlen(valuec); + [args enumerateKeysAndObjectsUsingBlock:^(NSString *key, NSString *value, __unused BOOL *stop) { + systrace_args[i].key = [key UTF8String]; + systrace_args[i].key_len = [key length]; + systrace_args[i].value = [value UTF8String]; + systrace_args[i].value_len = [value length]; i++; }]; return systrace_args; @@ -123,7 +120,7 @@ static NSDictionary *RCTProfileGetMemoryUsage(void) TASK_BASIC_INFO, (task_info_t)&info, &size); - if( kerr == KERN_SUCCESS ) { + if ( kerr == KERN_SUCCESS ) { return @{ @"suspend_count": @(info.suspend_count), @"virtual_size": RCTProfileMemory(info.virtual_size), @@ -545,12 +542,12 @@ void _RCTProfileBeginEvent( NSTimeInterval time, uint64_t tag, NSString *name, - NSDictionary *args + NSDictionary *args ) { CHECK(); if (callbacks != NULL) { - callbacks->begin_section(tag, name.UTF8String, args.count, RCTProfileSystraceArgsFromNSDictionary(args)); + callbacks->begin_section(tag, name.UTF8String, args.count, systraceArgsFromDictionary(args)); return; } @@ -603,7 +600,7 @@ void _RCTProfileEndEvent( NSUInteger RCTProfileBeginAsyncEvent( uint64_t tag, NSString *name, - NSDictionary *args + NSDictionary *args ) { CHECK(0); @@ -613,7 +610,8 @@ NSUInteger RCTProfileBeginAsyncEvent( NSUInteger currentEventID = ++eventID; if (callbacks != NULL) { - callbacks->begin_async_section(tag, name.UTF8String, (int)(currentEventID % INT_MAX), args.count, RCTProfileSystraceArgsFromNSDictionary(args)); + callbacks->begin_async_section(tag, name.UTF8String, (int)(currentEventID % INT_MAX), + args.count, systraceArgsFromDictionary(args)); } else { dispatch_async(RCTProfileGetQueue(), ^{ RCTProfileOngoingEvents[@(currentEventID)] = @[ diff --git a/ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.cpp b/ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.cpp index 4ca86bef1..ba3030061 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.cpp +++ b/ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.cpp @@ -10,6 +10,10 @@ #include #include +#ifdef WITH_FBSYSTRACE +#include +#endif + #include "CatalystInstanceImpl.h" #include "ReadableNativeArray.h" @@ -80,9 +84,14 @@ folly::dynamic JavaNativeModule::getConstants() { } } -void JavaNativeModule::invoke(unsigned int reactMethodId, folly::dynamic&& params) { - messageQueueThread_->runOnQueue([this, reactMethodId, params=std::move(params)] { +void JavaNativeModule::invoke(unsigned int reactMethodId, folly::dynamic&& params, int callId) { + messageQueueThread_->runOnQueue([this, reactMethodId, params=std::move(params), callId] { static auto invokeMethod = wrapper_->getClass()->getMethod("invoke"); + #ifdef WITH_FBSYSTRACE + if (callId != -1) { + fbsystrace_end_async_flow(TRACE_TAG_REACT_APPS, "native", callId); + } + #endif invokeMethod( wrapper_, static_cast(reactMethodId), @@ -148,13 +157,18 @@ folly::dynamic NewJavaNativeModule::getConstants() { } } -void NewJavaNativeModule::invoke(unsigned int reactMethodId, folly::dynamic&& params) { +void NewJavaNativeModule::invoke(unsigned int reactMethodId, folly::dynamic&& params, int callId) { if (reactMethodId >= methods_.size()) { throw std::invalid_argument( folly::to("methodId ", reactMethodId, " out of range [0..", methods_.size(), "]")); } CHECK(!methods_[reactMethodId].isSyncHook()) << "Trying to invoke a synchronous hook asynchronously"; - messageQueueThread_->runOnQueue([this, reactMethodId, params=std::move(params)] () mutable { + messageQueueThread_->runOnQueue([this, reactMethodId, params=std::move(params), callId] () mutable { + #ifdef WITH_FBSYSTRACE + if (callId != -1) { + fbsystrace_end_async_flow(TRACE_TAG_REACT_APPS, "native", callId); + } + #endif invokeInner(reactMethodId, std::move(params)); }); } diff --git a/ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.h b/ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.h index 0612ccde4..6bc4f6b75 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.h +++ b/ReactAndroid/src/main/jni/xreact/jni/JavaModuleWrapper.h @@ -59,7 +59,7 @@ class JavaNativeModule : public NativeModule { std::string getName() override; folly::dynamic getConstants() override; std::vector getMethods() override; - void invoke(unsigned int reactMethodId, folly::dynamic&& params) override; + void invoke(unsigned int reactMethodId, folly::dynamic&& params, int callId) override; MethodCallResult callSerializableNativeHook(unsigned int reactMethodId, folly::dynamic&& params) override; private: @@ -80,7 +80,7 @@ class NewJavaNativeModule : public NativeModule { std::string getName() override; std::vector getMethods() override; folly::dynamic getConstants() override; - void invoke(unsigned int reactMethodId, folly::dynamic&& params) override; + void invoke(unsigned int reactMethodId, folly::dynamic&& params, int callId) override; MethodCallResult callSerializableNativeHook(unsigned int reactMethodId, folly::dynamic&& params) override; private: diff --git a/ReactCommon/cxxreact/CxxNativeModule.cpp b/ReactCommon/cxxreact/CxxNativeModule.cpp index b744c6330..314d3837a 100644 --- a/ReactCommon/cxxreact/CxxNativeModule.cpp +++ b/ReactCommon/cxxreact/CxxNativeModule.cpp @@ -7,7 +7,8 @@ #include -#include +#include "JsArgumentHelpers.h" +#include "SystraceSection.h" using facebook::xplat::module::CxxModule; @@ -75,7 +76,7 @@ folly::dynamic CxxNativeModule::getConstants() { return constants; } -void CxxNativeModule::invoke(unsigned int reactMethodId, folly::dynamic&& params) { +void CxxNativeModule::invoke(unsigned int reactMethodId, folly::dynamic&& params, int callId) { if (reactMethodId >= methods_.size()) { throw std::invalid_argument(folly::to("methodId ", reactMethodId, " out of range [0..", methods_.size(), "]")); @@ -128,7 +129,13 @@ void CxxNativeModule::invoke(unsigned int reactMethodId, folly::dynamic&& params // stack. I'm told that will be possible in the future. TODO // mhorowitz #7128529: convert C++ exceptions to Java - messageQueueThread_->runOnQueue([method, params=std::move(params), first, second] () { + messageQueueThread_->runOnQueue([method, params=std::move(params), first, second, callId] () { + #ifdef WITH_FBSYSTRACE + if (callId != -1) { + fbsystrace_end_async_flow(TRACE_TAG_REACT_APPS, "native", callId); + } + #endif + SystraceSection s(method.name.c_str()); try { method.func(std::move(params), first, second); } catch (const facebook::xplat::JsArgumentException& ex) { diff --git a/ReactCommon/cxxreact/CxxNativeModule.h b/ReactCommon/cxxreact/CxxNativeModule.h index 9b1810d58..da42b29c6 100644 --- a/ReactCommon/cxxreact/CxxNativeModule.h +++ b/ReactCommon/cxxreact/CxxNativeModule.h @@ -27,7 +27,7 @@ public: std::string getName() override; std::vector getMethods() override; folly::dynamic getConstants() override; - void invoke(unsigned int reactMethodId, folly::dynamic&& params) override; + void invoke(unsigned int reactMethodId, folly::dynamic&& params, int callId) override; MethodCallResult callSerializableNativeHook(unsigned int hookId, folly::dynamic&& args) override; private: diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 42dc75e02..2aaf9d8e8 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/ReactCommon/cxxreact/JSCExecutor.cpp @@ -182,7 +182,7 @@ static bool canUseInspector(JSContextRef context) { #endif void JSCExecutor::initOnJSVMThread() throw(JSException) { - SystraceSection s("JSCExecutor.initOnJSVMThread"); + SystraceSection s("JSCExecutor::initOnJSVMThread"); #if defined(__APPLE__) const bool useCustomJSC = m_jscConfig.getDefault("UseCustomJSC", false).getBool(); @@ -353,20 +353,15 @@ void JSCExecutor::loadApplicationScript(std::unique_ptr scrip } else #endif { - #ifdef WITH_FBSYSTRACE - fbsystrace_begin_section( - TRACE_TAG_REACT_CXX_BRIDGE, - "JSCExecutor::loadApplicationScript-createExpectingAscii"); - #endif - - ReactMarker::logMarker(ReactMarker::JS_BUNDLE_STRING_CONVERT_START); - String jsScript = jsStringFromBigString(m_context, *script); - ReactMarker::logMarker(ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP); - - #ifdef WITH_FBSYSTRACE - fbsystrace_end_section(TRACE_TAG_REACT_CXX_BRIDGE); - #endif + String jsScript; + { + SystraceSection s_("JSCExecutor::loadApplicationScript-createExpectingAscii"); + ReactMarker::logMarker(ReactMarker::JS_BUNDLE_STRING_CONVERT_START); + jsScript = jsStringFromBigString(m_context, *script); + ReactMarker::logMarker(ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP); + } + SystraceSection s_("JSCExecutor::loadApplicationScript-evaluateScript"); evaluateScript(m_context, jsScript, jsSourceURL); } @@ -457,9 +452,9 @@ void JSCExecutor::flush() { void JSCExecutor::callFunction(const std::string& moduleId, const std::string& methodId, const folly::dynamic& arguments) { SystraceSection s("JSCExecutor::callFunction"); + // This weird pattern is because Value is not default constructible. // The lambda is inlined, so there's no overhead. - auto result = [&] { try { if (!m_callFunctionReturnResultAndFlushedQueueJS) { @@ -524,8 +519,7 @@ Value JSCExecutor::callFunctionSyncWithValue( void JSCExecutor::setGlobalVariable(std::string propName, std::unique_ptr jsonValue) { try { - SystraceSection s("JSCExecutor.setGlobalVariable", - "propName", propName); + SystraceSection s("JSCExecutor::setGlobalVariable", "propName", propName); auto valueToInject = Value::fromJSON(m_context, jsStringFromBigString(m_context, *jsonValue)); Object::getGlobalObject(m_context).setProperty(propName.c_str(), valueToInject); diff --git a/ReactCommon/cxxreact/ModuleRegistry.cpp b/ReactCommon/cxxreact/ModuleRegistry.cpp index 72c9e5ecb..98042f401 100644 --- a/ReactCommon/cxxreact/ModuleRegistry.cpp +++ b/ReactCommon/cxxreact/ModuleRegistry.cpp @@ -118,14 +118,7 @@ void ModuleRegistry::callNativeMethod(unsigned int moduleId, unsigned int method throw std::runtime_error( folly::to("moduleId ", moduleId, " out of range [0..", modules_.size(), ")")); } - -#ifdef WITH_FBSYSTRACE - if (callId != -1) { - fbsystrace_end_async_flow(TRACE_TAG_REACT_APPS, "native", callId); - } -#endif - - modules_[moduleId]->invoke(methodId, std::move(params)); + modules_[moduleId]->invoke(methodId, std::move(params), callId); } MethodCallResult ModuleRegistry::callSerializableNativeHook(unsigned int moduleId, unsigned int methodId, folly::dynamic&& params) { diff --git a/ReactCommon/cxxreact/NativeModule.h b/ReactCommon/cxxreact/NativeModule.h index e0181d39b..3ec7f3880 100644 --- a/ReactCommon/cxxreact/NativeModule.h +++ b/ReactCommon/cxxreact/NativeModule.h @@ -5,8 +5,8 @@ #include #include -#include #include +#include namespace facebook { namespace react { @@ -29,7 +29,7 @@ class NativeModule { virtual folly::dynamic getConstants() = 0; // TODO mhorowitz: do we need initialize()/onCatalystInstanceDestroy() in C++ // or only Java? - virtual void invoke(unsigned int reactMethodId, folly::dynamic&& params) = 0; + virtual void invoke(unsigned int reactMethodId, folly::dynamic&& params, int callId) = 0; virtual MethodCallResult callSerializableNativeHook(unsigned int reactMethodId, folly::dynamic&& args) = 0; }; diff --git a/ReactCommon/cxxreact/NativeToJsBridge.cpp b/ReactCommon/cxxreact/NativeToJsBridge.cpp index dcbab72c2..33f73161f 100644 --- a/ReactCommon/cxxreact/NativeToJsBridge.cpp +++ b/ReactCommon/cxxreact/NativeToJsBridge.cpp @@ -124,27 +124,21 @@ void NativeToJsBridge::callFunction( int systraceCookie = -1; #ifdef WITH_FBSYSTRACE systraceCookie = m_systraceCookie++; - std::string tracingName = fbsystrace_is_tracing(TRACE_TAG_REACT_CXX_BRIDGE) ? - folly::to("JSCall__", module, '_', method) : std::string(); - SystraceSection s(tracingName.c_str()); FbSystraceAsyncFlow::begin( TRACE_TAG_REACT_CXX_BRIDGE, - tracingName.c_str(), + "JSCall", systraceCookie); - #else - std::string tracingName; #endif - runOnExecutorQueue([module = std::move(module), method = std::move(method), arguments = std::move(arguments), tracingName = std::move(tracingName), systraceCookie] + runOnExecutorQueue([module = std::move(module), method = std::move(method), arguments = std::move(arguments), systraceCookie] (JSExecutor* executor) { #ifdef WITH_FBSYSTRACE FbSystraceAsyncFlow::end( TRACE_TAG_REACT_CXX_BRIDGE, - tracingName.c_str(), + "JSCall", systraceCookie); - SystraceSection s(tracingName.c_str()); + SystraceSection s("NativeToJsBridge::callFunction", "module", module, "method", method); #endif - // This is safe because we are running on the executor's thread: it won't // destruct until after it's been unregistered (which we check above) and // that will happen on this thread @@ -169,9 +163,8 @@ void NativeToJsBridge::invokeCallback(double callbackId, folly::dynamic&& argume TRACE_TAG_REACT_CXX_BRIDGE, "", systraceCookie); - SystraceSection s("NativeToJsBridge.invokeCallback"); + SystraceSection s("NativeToJsBridge::invokeCallback"); #endif - executor->invokeCallback(callbackId, arguments); }); } diff --git a/ReactCommon/jschelpers/JSCHelpers.cpp b/ReactCommon/jschelpers/JSCHelpers.cpp index 975e12376..ade9eaf9e 100644 --- a/ReactCommon/jschelpers/JSCHelpers.cpp +++ b/ReactCommon/jschelpers/JSCHelpers.cpp @@ -123,9 +123,6 @@ void removeGlobal(JSGlobalContextRef ctx, const char* name) { } JSValueRef evaluateScript(JSContextRef context, JSStringRef script, JSStringRef source) { - #ifdef WITH_FBSYSTRACE - fbsystrace::FbSystraceSection s(TRACE_TAG_REACT_CXX_BRIDGE, "evaluateScript"); - #endif JSValueRef exn, result; result = JSC_JSEvaluateScript(context, script, NULL, source, 0, &exn); if (result == nullptr) { diff --git a/ReactCommon/jschelpers/Value.h b/ReactCommon/jschelpers/Value.h index e0f1d0e08..ad9e81e04 100644 --- a/ReactCommon/jschelpers/Value.h +++ b/ReactCommon/jschelpers/Value.h @@ -41,9 +41,10 @@ private: class String : public noncopyable { public: - explicit String(JSContextRef context, const char* utf8) : - m_context(context), m_string(JSC_JSStringCreateWithUTF8CString(context, utf8)) - {} + explicit String(): m_context(nullptr), m_string(nullptr) {} // dummy empty constructor + + explicit String(JSContextRef context, const char* utf8) + : m_context(context), m_string(JSC_JSStringCreateWithUTF8CString(context, utf8)) {} String(String&& other) : m_context(other.m_context), m_string(other.m_string) @@ -65,18 +66,30 @@ public: } } + String& operator=(String&& other) { + if (m_string) { + JSC_JSStringRelease(m_context, m_string); + } + + m_context = other.m_context; + m_string = other.m_string; + other.m_string = nullptr; + + return *this; + } + operator JSStringRef() const { return m_string; } // Length in characters size_t length() const { - return JSC_JSStringGetLength(m_context, m_string); + return m_string ? JSC_JSStringGetLength(m_context, m_string) : 0; } // Length in bytes of a null-terminated utf8 encoded value size_t utf8Size() const { - return JSC_JSStringGetMaximumUTF8CStringSize(m_context, m_string); + return m_string ? JSC_JSStringGetMaximumUTF8CStringSize(m_context, m_string) : 0; } /* @@ -94,6 +107,9 @@ public: * https://mathiasbynens.be/notes/javascript-unicode */ std::string str() const { + if (!m_string) { + return ""; + } const JSChar* utf16 = JSC_JSStringGetCharactersPtr(m_context, m_string); size_t stringLength = JSC_JSStringGetLength(m_context, m_string); return unicode::utf16toUTF8(utf16, stringLength); @@ -101,7 +117,7 @@ public: // Assumes that utf8 is null terminated bool equals(const char* utf8) { - return JSC_JSStringIsEqualToUTF8CString(m_context, m_string, utf8); + return m_string ? JSC_JSStringIsEqualToUTF8CString(m_context, m_string, utf8) : false; } // This assumes ascii is nul-terminated.