From d839e4abacc9e22955746bbd56028cdc90fc8f0c Mon Sep 17 00:00:00 2001 From: Xin Chen Date: Mon, 17 Jul 2023 14:32:00 -0700 Subject: [PATCH] Call C++ ReactMarker from android side to log platform specific timing (#38321) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/38321 This diff adds the native method in `ReactMarker.java` file so that any logs in the android side will call into C++. Given that currently the C++ uses the native implementation from hosting platforms, this allows any call (either from C++ or android) will end up calling the C++ method after logging is done. The motivation of this change is to allow us to collect timing information from hosting platforms, and report back to JS performance startup API. We will have items that are logged before the JNI part is loaded, and those will be cached and sent over in the next diff. Changelog: [Android][Internal] - Implemented native method for Android LogMarker API to report timing values to C++. Reviewed By: mdvacca Differential Revision: D43806115 fbshipit-source-id: 11497bdc0af8d1b0299a797c636bb7af7a6ddda4 --- .../facebook/react/bridge/ReactMarker.java | 7 +++ .../src/main/jni/react/jni/JReactMarker.cpp | 44 ++++++++++++++++++- .../src/main/jni/react/jni/JReactMarker.h | 5 +++ .../src/main/jni/react/jni/OnLoad.cpp | 2 + .../ReactCommon/cxxreact/ReactMarker.cpp | 15 ++++--- .../ReactCommon/cxxreact/ReactMarker.h | 9 +++- 6 files changed, 74 insertions(+), 8 deletions(-) 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 83ecd9d39368e1..73aa6922b9fdb4 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 @@ -140,8 +140,15 @@ public static void logMarker(ReactMarkerConstants name, @Nullable String tag, in for (MarkerListener listener : sListeners) { listener.logMarker(name, tag, instanceKey); } + + if (ReactBridge.isInitialized()) { + nativeLogMarker(name.name(), SystemClock.uptimeMillis()); + } } + @DoNotStrip + private static native void nativeLogMarker(String markerName, long markerTime); + @DoNotStrip public static void setAppStartTime(long appStartTime) { sAppStartTime = appStartTime; 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 c3be1ad2b55a3f..5c49f6effe54c8 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 @@ -63,7 +63,7 @@ void JReactMarker::logPerfMarkerBridgeless( } void JReactMarker::logPerfMarkerWithInstanceKey( - const facebook::react::ReactMarker::ReactMarkerId markerId, + const ReactMarker::ReactMarkerId markerId, const char *tag, const int instanceKey) { switch (markerId) { @@ -109,4 +109,46 @@ double JReactMarker::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") { + ReactMarker::logMarkerDone( + ReactMarker::RUN_JS_BUNDLE_START, (double)markerTime); + } else if (markerNameStr == "RUN_JS_BUNDLE_END") { + ReactMarker::logMarkerDone( + ReactMarker::RUN_JS_BUNDLE_STOP, (double)markerTime); + } else if (markerNameStr == "CREATE_REACT_CONTEXT_END") { + ReactMarker::logMarkerDone( + ReactMarker::CREATE_REACT_CONTEXT_STOP, (double)markerTime); + } else if (markerNameStr == "loadApplicationScript_startStringConvert") { + ReactMarker::logMarkerDone( + ReactMarker::JS_BUNDLE_STRING_CONVERT_START, (double)markerTime); + } else if (markerNameStr == "loadApplicationScript_endStringConvert") { + ReactMarker::logMarkerDone( + ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP, (double)markerTime); + } else if (markerNameStr == "NATIVE_MODULE_SETUP_START") { + ReactMarker::logMarkerDone( + ReactMarker::NATIVE_MODULE_SETUP_START, (double)markerTime); + } else if (markerNameStr == "NATIVE_MODULE_SETUP_END") { + ReactMarker::logMarkerDone( + ReactMarker::NATIVE_MODULE_SETUP_STOP, (double)markerTime); + } else if (markerNameStr == "REGISTER_JS_SEGMENT_START") { + ReactMarker::logMarkerDone( + ReactMarker::REGISTER_JS_SEGMENT_START, (double)markerTime); + } else if (markerNameStr == "REGISTER_JS_SEGMENT_STOP") { + ReactMarker::logMarkerDone( + ReactMarker::REGISTER_JS_SEGMENT_STOP, (double)markerTime); + } +} + +void JReactMarker::registerNatives() { + javaClassLocal()->registerNatives({ + makeNativeMethod("nativeLogMarker", JReactMarker::nativeLogMarker), + }); +} + } // namespace facebook::react 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 a857ae4f3f72e1..30a3a1f61e2267 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 @@ -18,6 +18,7 @@ class JReactMarker : public facebook::jni::JavaClass { public: static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/ReactMarker;"; + static void registerNatives(); static void setLogPerfMarkerIfNeeded(); private: @@ -38,6 +39,10 @@ class JReactMarker : public facebook::jni::JavaClass { const char *tag, const int instanceKey); static double getAppStartTime(); + static void nativeLogMarker( + jni::alias_ref /* unused */, + std::string markerNameStr, + jlong markerTime); }; } // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp index 58bd589c43d7ab..e3d6c3ef03aa79 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp @@ -16,6 +16,7 @@ #include "CatalystInstanceImpl.h" #include "CxxModuleWrapper.h" #include "JCallback.h" +#include "JReactMarker.h" #include "JavaScriptExecutorHolder.h" #include "ProxyExecutor.h" #include "WritableNativeArray.h" @@ -85,6 +86,7 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM *vm, void *reserved) { NativeMap::registerNatives(); ReadableNativeMap::registerNatives(); WritableNativeMap::registerNatives(); + JReactMarker::registerNatives(); #ifdef WITH_INSPECTOR JInspector::registerNatives(); diff --git a/packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp b/packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp index a84b8d71b3aae0..20e7d3bf5457a3 100644 --- a/packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp +++ b/packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp @@ -29,7 +29,6 @@ void logMarker(const ReactMarkerId markerId) { } void logTaggedMarker(const ReactMarkerId markerId, const char *tag) { - StartupLogger::getInstance().logStartupEvent(markerId); logTaggedMarkerImpl(markerId, tag); } @@ -38,27 +37,31 @@ void logMarkerBridgeless(const ReactMarkerId markerId) { } void logTaggedMarkerBridgeless(const ReactMarkerId markerId, const char *tag) { - StartupLogger::getInstance().logStartupEvent(markerId); logTaggedMarkerBridgelessImpl(markerId, tag); } +void logMarkerDone(const ReactMarkerId markerId, double markerTime) { + StartupLogger::getInstance().logStartupEvent(markerId, markerTime); +} + StartupLogger &StartupLogger::getInstance() { static StartupLogger instance; return instance; } -void StartupLogger::logStartupEvent(const ReactMarkerId markerId) { - auto now = JSExecutor::performanceNow(); +void StartupLogger::logStartupEvent( + const ReactMarkerId markerId, + double markerTime) { switch (markerId) { case ReactMarkerId::RUN_JS_BUNDLE_START: if (runJSBundleStartTime == 0) { - runJSBundleStartTime = now; + runJSBundleStartTime = markerTime; } return; case ReactMarkerId::RUN_JS_BUNDLE_STOP: if (runJSBundleEndTime == 0) { - runJSBundleEndTime = now; + runJSBundleEndTime = markerTime; } return; diff --git a/packages/react-native/ReactCommon/cxxreact/ReactMarker.h b/packages/react-native/ReactCommon/cxxreact/ReactMarker.h index 76b8e7a165e8dc..75802184b7465c 100644 --- a/packages/react-native/ReactCommon/cxxreact/ReactMarker.h +++ b/packages/react-native/ReactCommon/cxxreact/ReactMarker.h @@ -71,7 +71,7 @@ class StartupLogger { public: static StartupLogger &getInstance(); - void logStartupEvent(const ReactMarker::ReactMarkerId markerId); + void logStartupEvent(const ReactMarkerId markerName, double markerTime); double getAppStartTime(); double getRunJSBundleStartTime(); double getRunJSBundleEndTime(); @@ -85,5 +85,12 @@ class StartupLogger { double runJSBundleEndTime; }; +// When the marker got logged from the platform, it will notify here. This is +// used to collect react markers that are logged in the platform instead of in +// C++. +extern RN_EXPORT void logMarkerDone( + const ReactMarkerId markerId, + double markerTime); + } // namespace ReactMarker } // namespace facebook::react