Unify bridge and bridgeless ReactMarker implementations (#57040)

Summary:

The split between `logTaggedMarkerImpl` and `logTaggedMarkerBridgelessImpl` no longer earns its keep — bridgeless callers always pass the same instance key, and the bridge mode is on its way out. Collapse the two paths into a single hook so platform code only has to wire up one slot.

`LogTaggedMarkerBridgeless` becomes a type alias for `LogTaggedMarker`. The free `logMarkerBridgeless` / `logTaggedMarkerBridgeless` functions and the `logTaggedMarkerBridgelessImpl` slot stay around as `[[deprecated]]` shims. Call-sites in `ReactInstance` switch to the unified `logMarker` / `logTaggedMarker`. On the Android side, `logPerfMarkerBridgeless` and the `logPerfMarkerWithInstanceKey` indirection go away — there's only one instance key now.

Changelog:
[General][Deprecated] - Deprecate `logMarkerBridgeless`, `logTaggedMarkerBridgeless`, and `logTaggedMarkerBridgelessImpl` in favor of the non-bridgeless variants

Reviewed By: mdvacca

Differential Revision: D88068665

Author: javache

packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.cpp

@@ -13,13 +13,14 @@
 
 namespace facebook::react {
 
+namespace {
+const int kInstanceKey = 1;
+}
+
 void JReactMarker::setLogPerfMarkerIfNeeded() {
   static std::once_flag flag{};
   std::call_once(flag, []() {
-    std::unique_lock lock(ReactMarker::logTaggedMarkerImplMutex);
     ReactMarker::logTaggedMarkerImpl = JReactMarker::logPerfMarker;
-    ReactMarker::logTaggedMarkerBridgelessImpl =
-        JReactMarker::logPerfMarkerBridgeless;
   });
 }
 
@@ -51,21 +52,6 @@ void JReactMarker::logMarker(
 void JReactMarker::logPerfMarker(
     const ReactMarker::ReactMarkerId markerId,
     const char* tag) {
-  const int bridgeInstanceKey = 0;
-  logPerfMarkerWithInstanceKey(markerId, tag, bridgeInstanceKey);
-}
-
-void JReactMarker::logPerfMarkerBridgeless(
-    const ReactMarker::ReactMarkerId markerId,
-    const char* tag) {
-  const int bridgelessInstanceKey = 1;
-  logPerfMarkerWithInstanceKey(markerId, tag, bridgelessInstanceKey);
-}
-
-void JReactMarker::logPerfMarkerWithInstanceKey(
-    const ReactMarker::ReactMarkerId markerId,
-    const char* tag,
-    const int instanceKey) {
   switch (markerId) {
     case ReactMarker::APP_STARTUP_START:
       JReactMarker::logMarker("APP_STARTUP_START");
@@ -80,10 +66,10 @@ void JReactMarker::logPerfMarkerWithInstanceKey(
       JReactMarker::logMarker("INIT_REACT_RUNTIME_END");
       break;
     case ReactMarker::RUN_JS_BUNDLE_START:
-      JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag, instanceKey);
+      JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag, kInstanceKey);
       break;
     case ReactMarker::RUN_JS_BUNDLE_STOP:
-      JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag, instanceKey);
+      JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag, kInstanceKey);
       break;
     case ReactMarker::CREATE_REACT_CONTEXT_STOP:
       JReactMarker::logMarker("CREATE_REACT_CONTEXT_END");
@@ -95,16 +81,16 @@ void JReactMarker::logPerfMarkerWithInstanceKey(
       JReactMarker::logMarker("loadApplicationScript_endStringConvert");
       break;
     case ReactMarker::NATIVE_MODULE_SETUP_START:
-      JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag, instanceKey);
+      JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag, kInstanceKey);
       break;
     case ReactMarker::NATIVE_MODULE_SETUP_STOP:
-      JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag, instanceKey);
+      JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag, kInstanceKey);
       break;
     case ReactMarker::REGISTER_JS_SEGMENT_START:
-      JReactMarker::logMarker("REGISTER_JS_SEGMENT_START", tag, instanceKey);
+      JReactMarker::logMarker("REGISTER_JS_SEGMENT_START", tag, kInstanceKey);
       break;
     case ReactMarker::REGISTER_JS_SEGMENT_STOP:
-      JReactMarker::logMarker("REGISTER_JS_SEGMENT_STOP", tag, instanceKey);
+      JReactMarker::logMarker("REGISTER_JS_SEGMENT_STOP", tag, kInstanceKey);
       break;
     case ReactMarker::NATIVE_REQUIRE_START:
     case ReactMarker::NATIVE_REQUIRE_STOP:

packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.h

@@ -25,8 +25,6 @@ class JReactMarker : public facebook::jni::JavaClass<JReactMarker> {
   static void logMarker(const std::string &marker, const std::string &tag);
   static void logMarker(const std::string &marker, const std::string &tag, int instanceKey);
   static void logPerfMarker(ReactMarker::ReactMarkerId markerId, const char *tag);
-  static void logPerfMarkerBridgeless(ReactMarker::ReactMarkerId markerId, const char *tag);
-  static void logPerfMarkerWithInstanceKey(ReactMarker::ReactMarkerId markerId, const char *tag, int instanceKey);
   static void nativeLogMarker(jni::alias_ref<jclass> /* unused */, const std::string &markerNameStr, jlong markerTime);
 };
 

packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp

@@ -9,40 +9,23 @@
 
 namespace facebook::react::ReactMarker {
 
-#if __clang__
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wglobal-constructors"
-#endif
-
-LogTaggedMarker logTaggedMarkerBridgelessImpl = nullptr;
-LogTaggedMarker logTaggedMarkerImpl = nullptr;
-std::shared_mutex logTaggedMarkerImplMutex;
-
-#if __clang__
-#pragma clang diagnostic pop
-#endif
+AtomicLogTaggedMarker logTaggedMarkerImpl;
+AtomicLogTaggedMarker logTaggedMarkerBridgelessImpl;
 
 void logMarker(const ReactMarkerId markerId) {
   logTaggedMarker(markerId, nullptr);
 }
 
 void logTaggedMarker(const ReactMarkerId markerId, const char* tag) {
-  LogTaggedMarker marker = nullptr;
-  {
-    std::shared_lock lock(logTaggedMarkerImplMutex);
-    marker = logTaggedMarkerImpl;
-  }
-  if (marker != nullptr) {
-    marker(markerId, tag);
-  }
+  logTaggedMarkerImpl(markerId, tag);
 }
 
 void logMarkerBridgeless(const ReactMarkerId markerId) {
-  logTaggedMarkerBridgeless(markerId, nullptr);
+  logTaggedMarker(markerId, nullptr);
 }
 
 void logTaggedMarkerBridgeless(const ReactMarkerId markerId, const char* tag) {
-  logTaggedMarkerBridgelessImpl(markerId, tag);
+  logTaggedMarker(markerId, tag);
 }
 
 void logMarkerDone(const ReactMarkerId markerId, double markerTime) {

packages/react-native/ReactCommon/cxxreact/ReactMarker.h

@@ -8,11 +8,9 @@
 #pragma once
 
 #include <cmath>
-#include <shared_mutex>
-
-#ifdef __APPLE__
 #include <functional>
-#endif
+#include <mutex>
+#include <shared_mutex>
 
 namespace facebook::react::ReactMarker {
 
@@ -36,28 +34,51 @@ enum ReactMarkerId {
   REACT_INSTANCE_INIT_STOP
 };
 
-#ifdef __APPLE__
-using LogTaggedMarker = std::function<void(const ReactMarkerId, const char *tag)>; // Bridge only
-using LogTaggedMarkerBridgeless = std::function<void(const ReactMarkerId, const char *tag)>;
-#else
-typedef void (*LogTaggedMarker)(const ReactMarkerId, const char *tag); // Bridge only
-typedef void (*LogTaggedMarkerBridgeless)(const ReactMarkerId, const char *tag);
-#endif
+using LogTaggedMarker = std::function<void(ReactMarkerId, const char *tag)>;
+using LogTaggedMarkerBridgeless = LogTaggedMarker;
 
 #ifndef RN_EXPORT
 #define RN_EXPORT __attribute__((visibility("default")))
 #endif
 
-extern RN_EXPORT std::shared_mutex logTaggedMarkerImplMutex;
-/// - important: To ensure this gets read and written to in a thread safe
-/// manner, make use of `logTaggedMarkerImplMutex`.
-extern RN_EXPORT LogTaggedMarker logTaggedMarkerImpl;
-extern RN_EXPORT LogTaggedMarker logTaggedMarkerBridgelessImpl;
+/// Thread-safe holder for a LogTaggedMarker callback. Reads and writes are
+/// internally synchronized, so callers do not need external locking.
+struct RN_EXPORT AtomicLogTaggedMarker {
+  AtomicLogTaggedMarker &operator=(LogTaggedMarker marker)
+  {
+    std::unique_lock lock(mutex_);
+    impl_ = std::move(marker);
+    return *this;
+  }
+
+  explicit operator bool() const noexcept
+  {
+    std::shared_lock lock(mutex_);
+    return static_cast<bool>(impl_);
+  }
+
+  void operator()(ReactMarkerId markerId, const char *tag) const
+  {
+    std::shared_lock lock(mutex_);
+    if (impl_) {
+      impl_(markerId, tag);
+    }
+  }
+
+ private:
+  LogTaggedMarker impl_;
+  mutable std::shared_mutex mutex_;
+};
+
+extern RN_EXPORT AtomicLogTaggedMarker logTaggedMarkerImpl;
+[[deprecated("Use logTaggedMarkerImpl instead")]]
+extern RN_EXPORT AtomicLogTaggedMarker logTaggedMarkerBridgelessImpl;
 
-extern RN_EXPORT void logMarker(ReactMarkerId markerId); // Bridge only
-extern RN_EXPORT void logTaggedMarker(ReactMarkerId markerId,
-                                      const char *tag); // Bridge only
+extern RN_EXPORT void logMarker(ReactMarkerId markerId);
+extern RN_EXPORT void logTaggedMarker(ReactMarkerId markerId, const char *tag);
+[[deprecated("Use logMarker instead")]]
 extern RN_EXPORT void logMarkerBridgeless(ReactMarkerId markerId);
+[[deprecated("Use logTaggedMarker instead")]]
 extern RN_EXPORT void logTaggedMarkerBridgeless(ReactMarkerId markerId, const char *tag);
 
 struct ReactMarkerEvent {

packages/react-native/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp

@@ -144,25 +144,14 @@ void JSIExecutor::initializeRuntime() {
   if (runtimeInstaller_) {
     runtimeInstaller_(*runtime_);
   }
-  bool hasLogger = false;
-  {
-    std::shared_lock lock(ReactMarker::logTaggedMarkerImplMutex);
-    hasLogger = ReactMarker::logTaggedMarkerImpl != nullptr;
-  }
-  if (hasLogger) {
-    ReactMarker::logMarker(ReactMarker::CREATE_REACT_CONTEXT_STOP);
-  }
+  ReactMarker::logMarker(ReactMarker::CREATE_REACT_CONTEXT_STOP);
 }
 
 void JSIExecutor::loadBundle(
     std::unique_ptr<const JSBigString> script,
     std::string sourceURL) {
   TraceSection s("JSIExecutor::loadBundle");
-  bool hasLogger = false;
-  {
-    std::shared_lock lock(ReactMarker::logTaggedMarkerImplMutex);
-    hasLogger = ReactMarker::logTaggedMarkerImpl != nullptr;
-  }
+  bool hasLogger(ReactMarker::logTaggedMarkerImpl);
   std::string scriptName = simpleBasename(sourceURL);
   if (hasLogger) {
     ReactMarker::logTaggedMarker(

packages/react-native/ReactCommon/jsiexecutor/jsireact/JSINativeModules.cpp

@@ -70,15 +70,8 @@ void JSINativeModules::reset() {
 std::optional<Object> JSINativeModules::createModule(
     Runtime& rt,
     const std::string& name) {
-  bool hasLogger = false;
-  {
-    std::shared_lock lock(ReactMarker::logTaggedMarkerImplMutex);
-    hasLogger = ReactMarker::logTaggedMarkerImpl != nullptr;
-  }
-  if (hasLogger) {
-    ReactMarker::logTaggedMarker(
-        ReactMarker::NATIVE_MODULE_SETUP_START, name.c_str());
-  }
+  ReactMarker::logTaggedMarker(
+      ReactMarker::NATIVE_MODULE_SETUP_START, name.c_str());
 
   auto result = m_moduleRegistry->getConfig(name);
   if (!result.has_value()) {
@@ -101,10 +94,8 @@ std::optional<Object> JSINativeModules::createModule(
   std::optional<Object> module(
       moduleInfo.asObject(rt).getPropertyAsObject(rt, "module"));
 
-  if (hasLogger) {
-    ReactMarker::logTaggedMarker(
-        ReactMarker::NATIVE_MODULE_SETUP_STOP, name.c_str());
-  }
+  ReactMarker::logTaggedMarker(
+      ReactMarker::NATIVE_MODULE_SETUP_STOP, name.c_str());
 
   return module;
 }