From cb173e6a6fe75dcd5f289d34703a7b42144546a6 Mon Sep 17 00:00:00 2001 From: Xin Chen Date: Tue, 18 Jul 2023 19:20:44 -0700 Subject: [PATCH] Change how app startup time is collected from platform side (#38325) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/38325 This diff changed how we log app startup time by leveraging ReactMarker `logMarker` API, instead of the custom `setAppStartTime` API. Changelog: [Android][Internal] - Refactor how app should notify C++ about the app startup time. Reviewed By: mdvacca Differential Revision: D43863975 fbshipit-source-id: f80bcdb55fae82abce08eb2eff689985f90f1213 --- .../WebPerformance/NativePerformance.cpp | 4 +-- packages/react-native/React/Base/RCTPLTag.h | 1 + .../React/Base/RCTPerformanceLoggerLabels.m | 2 ++ .../React/CxxBridge/RCTCxxBridge.mm | 6 ++++ .../facebook/react/bridge/ReactMarker.java | 34 ++++++++++--------- .../react/bridge/ReactMarkerConstants.java | 2 ++ .../src/main/jni/react/jni/JReactMarker.cpp | 21 +++++++----- .../src/main/jni/react/jni/JReactMarker.h | 1 - .../ReactCommon/cxxreact/ReactMarker.cpp | 25 ++++++++++---- .../ReactCommon/cxxreact/ReactMarker.h | 11 +++--- .../ios/Core/RCTPerformanceLoggerUtils.mm | 6 ++++ 11 files changed, 74 insertions(+), 39 deletions(-) diff --git a/packages/react-native/Libraries/WebPerformance/NativePerformance.cpp b/packages/react-native/Libraries/WebPerformance/NativePerformance.cpp index e9ccdf332b6..f2ec308fda0 100644 --- a/packages/react-native/Libraries/WebPerformance/NativePerformance.cpp +++ b/packages/react-native/Libraries/WebPerformance/NativePerformance.cpp @@ -60,12 +60,12 @@ ReactNativeStartupTiming NativePerformance::getReactNativeStartupTiming( ReactMarker::StartupLogger &startupLogger = ReactMarker::StartupLogger::getInstance(); - result.startTime = startupLogger.getAppStartTime(); + result.startTime = startupLogger.getAppStartupStartTime(); result.executeJavaScriptBundleEntryPointStart = startupLogger.getRunJSBundleStartTime(); result.executeJavaScriptBundleEntryPointEnd = startupLogger.getRunJSBundleEndTime(); - result.endTime = startupLogger.getRunJSBundleEndTime(); + result.endTime = startupLogger.getAppStartupEndTime(); return result; } diff --git a/packages/react-native/React/Base/RCTPLTag.h b/packages/react-native/React/Base/RCTPLTag.h index 5aa15cb5889..cb9f8f0ded0 100644 --- a/packages/react-native/React/Base/RCTPLTag.h +++ b/packages/react-native/React/Base/RCTPLTag.h @@ -25,5 +25,6 @@ typedef NS_ENUM(NSInteger, RCTPLTag) { RCTPLTTI, RCTPLBundleSize, RCTPLReactInstanceInit, + RCTPLAppStartup, RCTPLSize // This is used to count the size }; diff --git a/packages/react-native/React/Base/RCTPerformanceLoggerLabels.m b/packages/react-native/React/Base/RCTPerformanceLoggerLabels.m index 5312d1443a8..c508dbfc39a 100644 --- a/packages/react-native/React/Base/RCTPerformanceLoggerLabels.m +++ b/packages/react-native/React/Base/RCTPerformanceLoggerLabels.m @@ -49,6 +49,8 @@ NSString *RCTPLLabelForTag(RCTPLTag tag) return @"BundleSize"; case RCTPLReactInstanceInit: return @"ReactInstanceInit"; + case RCTPLAppStartup: + return @"AppStartup"; case RCTPLSize: // Only used to count enum size RCTAssert(NO, @"RCTPLSize should not be used to track performance timestamps."); return nil; diff --git a/packages/react-native/React/CxxBridge/RCTCxxBridge.mm b/packages/react-native/React/CxxBridge/RCTCxxBridge.mm index 23f8be7b384..55793e9d02e 100644 --- a/packages/react-native/React/CxxBridge/RCTCxxBridge.mm +++ b/packages/react-native/React/CxxBridge/RCTCxxBridge.mm @@ -134,6 +134,12 @@ static void mapReactMarkerToPerformanceLogger( const char *tag) { switch (markerId) { + case ReactMarker::APP_STARTUP_START: + [performanceLogger markStartForTag:RCTPLAppStartup]; + break; + case ReactMarker::APP_STARTUP_STOP: + [performanceLogger markStopForTag:RCTPLAppStartup]; + break; case ReactMarker::RUN_JS_BUNDLE_START: [performanceLogger markStartForTag:RCTPLScriptExecution]; break; diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarker.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarker.java index fa82230f2a9..ebfd755f8f7 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarker.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarker.java @@ -65,9 +65,6 @@ public class ReactMarker { private static final List sFabricMarkerListeners = new CopyOnWriteArrayList<>(); - // The android app start time that to be set by the corresponding app - private static long sAppStartTime; - @DoNotStrip public static void addListener(MarkerListener listener) { if (!sListeners.contains(listener)) { @@ -157,24 +154,39 @@ public class ReactMarker { logMarker(name, tag, 0); } + @DoNotStrip + public static void logMarker(ReactMarkerConstants name, long time) { + logMarker(name, null, 0, time); + } + @DoNotStrip @AnyThread public static void logMarker(ReactMarkerConstants name, @Nullable String tag, int instanceKey) { + logMarker(name, tag, instanceKey, null); + } + + @DoNotStrip + @AnyThread + public static void logMarker( + ReactMarkerConstants name, @Nullable String tag, int instanceKey, @Nullable Long time) { logFabricMarker(name, tag, instanceKey); for (MarkerListener listener : sListeners) { listener.logMarker(name, tag, instanceKey); } - notifyNativeMarker(name); + notifyNativeMarker(name, time); } @DoNotStrip - private static void notifyNativeMarker(ReactMarkerConstants name) { + private static void notifyNativeMarker(ReactMarkerConstants name, @Nullable Long time) { if (!name.hasMatchingNameMarker()) { return; } - long now = SystemClock.uptimeMillis(); + @Nullable Long now = time; + if (now == null) { + now = SystemClock.uptimeMillis(); + } if (ReactBridge.isInitialized()) { // First send the current marker @@ -193,14 +205,4 @@ public class ReactMarker { @DoNotStrip private static native void nativeLogMarker(String markerName, long markerTime); - - @DoNotStrip - public static void setAppStartTime(long appStartTime) { - sAppStartTime = appStartTime; - } - - @DoNotStrip - public static double getAppStartTime() { - return (double) sAppStartTime; - } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarkerConstants.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarkerConstants.java index af8fcb804d0..ce4f659698e 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarkerConstants.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarkerConstants.java @@ -9,6 +9,8 @@ package com.facebook.react.bridge; /** Constants used by ReactMarker. */ public enum ReactMarkerConstants { + APP_STARTUP_START(true), + APP_STARTUP_END(true), CREATE_REACT_CONTEXT_START, CREATE_REACT_CONTEXT_END(true), PROCESS_PACKAGES_START, diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.cpp index 5c49f6effe5..59cf545a074 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.cpp @@ -19,7 +19,6 @@ void JReactMarker::setLogPerfMarkerIfNeeded() { ReactMarker::logTaggedMarkerImpl = JReactMarker::logPerfMarker; ReactMarker::logTaggedMarkerBridgelessImpl = JReactMarker::logPerfMarkerBridgeless; - ReactMarker::getAppStartTimeImpl = JReactMarker::getAppStartTime; }); } @@ -67,6 +66,12 @@ void JReactMarker::logPerfMarkerWithInstanceKey( const char *tag, const int instanceKey) { switch (markerId) { + case ReactMarker::APP_STARTUP_START: + JReactMarker::logMarker("APP_STARTUP_START"); + break; + case ReactMarker::APP_STARTUP_STOP: + JReactMarker::logMarker("APP_STARTUP_END"); + break; case ReactMarker::RUN_JS_BUNDLE_START: JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag, instanceKey); break; @@ -103,19 +108,19 @@ void JReactMarker::logPerfMarkerWithInstanceKey( } } -double JReactMarker::getAppStartTime() { - static auto cls = javaClassStatic(); - static auto meth = cls->getStaticMethod("getAppStartTime"); - return meth(cls); -} - void JReactMarker::nativeLogMarker( jni::alias_ref /* unused */, std::string markerNameStr, jlong markerTime) { // TODO: refactor this to a bidirectional map along with // logPerfMarkerWithInstanceKey - if (markerNameStr == "RUN_JS_BUNDLE_START") { + if (markerNameStr == "APP_STARTUP_START") { + ReactMarker::logMarkerDone( + ReactMarker::APP_STARTUP_START, (double)markerTime); + } else if (markerNameStr == "APP_STARTUP_END") { + ReactMarker::logMarkerDone( + ReactMarker::APP_STARTUP_STOP, (double)markerTime); + } else if (markerNameStr == "RUN_JS_BUNDLE_START") { ReactMarker::logMarkerDone( ReactMarker::RUN_JS_BUNDLE_START, (double)markerTime); } else if (markerNameStr == "RUN_JS_BUNDLE_END") { diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.h b/packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.h index 30a3a1f61e2..1a48426c4fc 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.h @@ -38,7 +38,6 @@ class JReactMarker : public facebook::jni::JavaClass { const ReactMarker::ReactMarkerId markerId, const char *tag, const int instanceKey); - static double getAppStartTime(); static void nativeLogMarker( jni::alias_ref /* unused */, std::string markerNameStr, diff --git a/packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp b/packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp index 20e7d3bf545..2b6478b33c6 100644 --- a/packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp +++ b/packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp @@ -18,7 +18,6 @@ namespace ReactMarker { LogTaggedMarker logTaggedMarkerImpl = nullptr; LogTaggedMarker logTaggedMarkerBridgelessImpl = nullptr; -GetAppStartTime getAppStartTimeImpl = nullptr; #if __clang__ #pragma clang diagnostic pop @@ -53,6 +52,18 @@ void StartupLogger::logStartupEvent( const ReactMarkerId markerId, double markerTime) { switch (markerId) { + case ReactMarkerId::APP_STARTUP_START: + if (appStartupStartTime == 0) { + appStartupStartTime = markerTime; + } + return; + + case ReactMarkerId::APP_STARTUP_STOP: + if (appStartupEndTime == 0) { + appStartupEndTime = markerTime; + } + return; + case ReactMarkerId::RUN_JS_BUNDLE_START: if (runJSBundleStartTime == 0) { runJSBundleStartTime = markerTime; @@ -70,12 +81,8 @@ void StartupLogger::logStartupEvent( } } -double StartupLogger::getAppStartTime() { - if (getAppStartTimeImpl == nullptr) { - return 0; - } - - return getAppStartTimeImpl(); +double StartupLogger::getAppStartupStartTime() { + return appStartupStartTime; } double StartupLogger::getRunJSBundleStartTime() { @@ -86,5 +93,9 @@ double StartupLogger::getRunJSBundleEndTime() { return runJSBundleEndTime; } +double StartupLogger::getAppStartupEndTime() { + return appStartupEndTime; +} + } // namespace ReactMarker } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/cxxreact/ReactMarker.h b/packages/react-native/ReactCommon/cxxreact/ReactMarker.h index 75802184b74..178cdbc13a1 100644 --- a/packages/react-native/ReactCommon/cxxreact/ReactMarker.h +++ b/packages/react-native/ReactCommon/cxxreact/ReactMarker.h @@ -15,6 +15,8 @@ namespace facebook::react { namespace ReactMarker { enum ReactMarkerId { + APP_STARTUP_START, + APP_STARTUP_STOP, NATIVE_REQUIRE_START, NATIVE_REQUIRE_STOP, RUN_JS_BUNDLE_START, @@ -35,12 +37,10 @@ using LogTaggedMarker = std::function; // Bridge only using LogTaggedMarkerBridgeless = std::function; -using GetAppStartTime = std::function; #else typedef void ( *LogTaggedMarker)(const ReactMarkerId, const char *tag); // Bridge only typedef void (*LogTaggedMarkerBridgeless)(const ReactMarkerId, const char *tag); -typedef double (*GetAppStartTime)(); #endif #ifndef RN_EXPORT @@ -49,7 +49,6 @@ typedef double (*GetAppStartTime)(); extern RN_EXPORT LogTaggedMarker logTaggedMarkerImpl; // Bridge only extern RN_EXPORT LogTaggedMarker logTaggedMarkerBridgelessImpl; -extern RN_EXPORT GetAppStartTime getAppStartTimeImpl; extern RN_EXPORT void logMarker(const ReactMarkerId markerId); // Bridge only extern RN_EXPORT void logTaggedMarker( @@ -59,7 +58,6 @@ extern RN_EXPORT void logMarkerBridgeless(const ReactMarkerId markerId); extern RN_EXPORT void logTaggedMarkerBridgeless( const ReactMarkerId markerId, const char *tag); -extern RN_EXPORT double getAppStartTime(); struct ReactMarkerEvent { const ReactMarkerId markerId; @@ -72,15 +70,18 @@ class StartupLogger { static StartupLogger &getInstance(); void logStartupEvent(const ReactMarkerId markerName, double markerTime); - double getAppStartTime(); + double getAppStartupStartTime(); double getRunJSBundleStartTime(); double getRunJSBundleEndTime(); + double getAppStartupEndTime(); private: StartupLogger() = default; StartupLogger(const StartupLogger &) = delete; StartupLogger &operator=(const StartupLogger &) = delete; + double appStartupStartTime; + double appStartupEndTime; double runJSBundleStartTime; double runJSBundleEndTime; }; diff --git a/packages/react-native/ReactCommon/react/bridgeless/platform/ios/Core/RCTPerformanceLoggerUtils.mm b/packages/react-native/ReactCommon/react/bridgeless/platform/ios/Core/RCTPerformanceLoggerUtils.mm index a8c20b83f46..7a069100eea 100644 --- a/packages/react-native/ReactCommon/react/bridgeless/platform/ios/Core/RCTPerformanceLoggerUtils.mm +++ b/packages/react-native/ReactCommon/react/bridgeless/platform/ios/Core/RCTPerformanceLoggerUtils.mm @@ -17,6 +17,12 @@ static void mapReactMarkerToPerformanceLogger( RCTPerformanceLogger *performanceLogger) { switch (markerId) { + case ReactMarker::APP_STARTUP_START: + [performanceLogger markStartForTag:RCTPLAppStartup]; + break; + case ReactMarker::APP_STARTUP_STOP: + [performanceLogger markStopForTag:RCTPLAppStartup]; + break; case ReactMarker::RUN_JS_BUNDLE_START: [performanceLogger markStartForTag:RCTPLScriptExecution]; break;