Change how app startup time is collected from platform side (#38325)

Summary: Pull Request resolved: #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
facebook · Jul 19, 2023 · cb173e6 · cb173e6
1 parent c126702
commit cb173e6
Show file tree

Hide file tree

Showing 11 changed files with 74 additions and 39 deletions.
diff --git 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
@@ -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
@@ -49,6 +49,8 @@
       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
@@ -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
@@ -65,9 +65,6 @@ void logFabricMarker(
   private static final List<FabricMarkerListener> 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 static void logMarker(ReactMarkerConstants name, @Nullable String tag) {
     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 @@ private static void notifyNativeMarker(ReactMarkerConstants name) {
 
   @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/...act-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarkerConstants.java b/...act-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarkerConstants.java
@@ -9,6 +9,8 @@
 
 /** 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
@@ -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<double()>("getAppStartTime");
-  return meth(cls);
-}
-
 void JReactMarker::nativeLogMarker(
     jni::alias_ref<jclass> /* 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
@@ -38,7 +38,6 @@ class JReactMarker : public facebook::jni::JavaClass<JReactMarker> {
       const ReactMarker::ReactMarkerId markerId,
       const char *tag,
       const int instanceKey);
-  static double getAppStartTime();
   static void nativeLogMarker(
       jni::alias_ref<jclass> /* unused */,
       std::string markerNameStr,

diff --git 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
@@ -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<void(const ReactMarkerId, const char *tag)>; // Bridge only
 using LogTaggedMarkerBridgeless =
     std::function<void(const ReactMarkerId, const char *tag)>;
-using GetAppStartTime = std::function<double()>;
 #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/.../react-native/ReactCommon/react/bridgeless/platform/ios/Core/RCTPerformanceLoggerUtils.mm b/.../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;