Simplify PerformanceTracer API and move processing to TracingAgent (#52934)

Summary:
Pull Request resolved: #52934

Changelog: [internal]

Right now, the `PerformanceTracer` API has a method to stop the trace and a separate one to collect serialized events. This refactors the API to return the collected events when calling `stopTracing` instead.

The caller (in this case `TracingAgent`) is responsible for serializing and sending the events in chunks through CDP.

Reviewed By: hoxyq

Differential Revision: D79271690

fbshipit-source-id: bdb48c80be4fd07d96e381e6bb4d099cae91f8de

Author: rubennorte

packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp

@@ -9,6 +9,7 @@
 
 #include <jsinspector-modern/tracing/PerformanceTracer.h>
 #include <jsinspector-modern/tracing/RuntimeSamplingProfileTraceEventSerializer.h>
+#include <jsinspector-modern/tracing/TraceEventSerializer.h>
 
 namespace facebook::react::jsinspector_modern {
 
@@ -29,6 +30,26 @@ const uint16_t TRACE_EVENT_CHUNK_SIZE = 1000;
  */
 const uint16_t PROFILE_TRACE_EVENT_CHUNK_SIZE = 1;
 
+void serializeTraceEventsInChunks(
+    std::vector<tracing::TraceEvent>&& traceEvents,
+    uint16_t chunkSize,
+    const std::function<void(folly::dynamic&& eventsChunk)>& resultCallback) {
+  auto serializedTraceEvents = folly::dynamic::array();
+  for (auto&& traceEvent : traceEvents) {
+    // Emit trace events
+    serializedTraceEvents.push_back(
+        tracing::TraceEventSerializer::serialize(std::move(traceEvent)));
+
+    if (serializedTraceEvents.size() == chunkSize) {
+      resultCallback(std::move(serializedTraceEvents));
+      serializedTraceEvents = folly::dynamic::array();
+    }
+  }
+  if (!serializedTraceEvents.empty()) {
+    resultCallback(std::move(serializedTraceEvents));
+  }
+}
+
 } // namespace
 
 TracingAgent::TracingAgent(
@@ -89,8 +110,8 @@ bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) {
 
     tracing::PerformanceTracer& performanceTracer =
         tracing::PerformanceTracer::getInstance();
-    bool correctlyStopped = performanceTracer.stopTracing();
-    if (!correctlyStopped) {
+    auto collectedEvents = performanceTracer.stopTracing();
+    if (!collectedEvents) {
       frontendChannel_(cdp::jsonError(
           req.id,
           cdp::ErrorCode::InternalError,
@@ -107,8 +128,11 @@ bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) {
           "Tracing.dataCollected",
           folly::dynamic::object("value", std::move(eventsChunk))));
     };
-    performanceTracer.collectEvents(
-        dataCollectedCallback, TRACE_EVENT_CHUNK_SIZE);
+
+    serializeTraceEventsInChunks(
+        std::move(*collectedEvents),
+        TRACE_EVENT_CHUNK_SIZE,
+        dataCollectedCallback);
 
     auto tracingProfile = instanceAgent_->collectTracingProfile();
     tracing::IdGenerator profileIdGenerator;

packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp

@@ -28,38 +28,39 @@ PerformanceTracer::PerformanceTracer()
 
 bool PerformanceTracer::startTracing() {
   std::lock_guard lock(mutex_);
+
   if (tracingAtomic_) {
     return false;
   }
-  tracingAtomic_ = true;
 
-  buffer_.emplace_back(TraceEvent{
-      .name = "TracingStartedInPage",
-      .cat = "disabled-by-default-devtools.timeline",
-      .ph = 'I',
-      .ts = HighResTimeStamp::now(),
-      .pid = processId_,
-      .tid = oscompat::getCurrentThreadId(),
-      .args = folly::dynamic::object("data", folly::dynamic::object()),
-  });
+  tracingAtomic_ = true;
 
   return true;
 }
 
-bool PerformanceTracer::stopTracing() {
-  std::lock_guard lock(mutex_);
-  if (!tracingAtomic_) {
-    return false;
+std::optional<std::vector<TraceEvent>> PerformanceTracer::stopTracing() {
+  std::vector<TraceEvent> events;
+
+  {
+    std::lock_guard lock(mutex_);
+
+    if (!tracingAtomic_) {
+      return std::nullopt;
+    }
+
+    tracingAtomic_ = false;
+    performanceMeasureCount_ = 0;
+
+    buffer_.swap(events);
   }
-  tracingAtomic_ = false;
 
   // This is synthetic Trace Event, which should not be represented on a
   // timeline. CDT is not using Profile or ProfileChunk events for determining
   // trace timeline window, this is why trace that only contains JavaScript
-  // samples will be displayed as empty. We use this event to avoid that.
+  // samples will be displayed as empty. We use these events to avoid that.
   // This could happen for non-bridgeless apps, where Performance interface is
   // not supported and no spec-compliant Event Loop implementation.
-  buffer_.emplace_back(TraceEvent{
+  events.emplace_back(TraceEvent{
       .name = "ReactNative-TracingStopped",
       .cat = "disabled-by-default-devtools.timeline",
       .ph = 'I',
@@ -68,45 +69,6 @@ bool PerformanceTracer::stopTracing() {
       .tid = oscompat::getCurrentThreadId(),
   });
 
-  performanceMeasureCount_ = 0;
-  return true;
-}
-
-void PerformanceTracer::collectEvents(
-    const std::function<void(folly::dynamic&& eventsChunk)>& resultCallback,
-    uint16_t chunkSize) {
-  std::vector<TraceEvent> localBuffer;
-  {
-    std::lock_guard lock(mutex_);
-    buffer_.swap(localBuffer);
-  }
-
-  if (localBuffer.empty()) {
-    return;
-  }
-
-  auto serializedTraceEvents = folly::dynamic::array();
-  for (auto&& event : localBuffer) {
-    // Emit trace events
-    serializedTraceEvents.push_back(
-        TraceEventSerializer::serialize(std::move(event)));
-
-    if (serializedTraceEvents.size() == chunkSize) {
-      resultCallback(std::move(serializedTraceEvents));
-      serializedTraceEvents = folly::dynamic::array();
-    }
-  }
-  if (!serializedTraceEvents.empty()) {
-    resultCallback(std::move(serializedTraceEvents));
-  }
-}
-
-std::vector<TraceEvent> PerformanceTracer::collectTraceEvents() {
-  std::vector<TraceEvent> events;
-  {
-    std::lock_guard lock(mutex_);
-    buffer_.swap(events);
-  }
   return events;
 }
 

packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h

@@ -15,7 +15,6 @@
 
 #include <folly/dynamic.h>
 #include <atomic>
-#include <functional>
 #include <mutex>
 #include <optional>
 #include <vector>
@@ -34,14 +33,15 @@ class PerformanceTracer {
   static PerformanceTracer& getInstance();
 
   /**
-   * Mark trace session as started. Returns `false` if already tracing.
+   * Starts a tracing session. Returns `false` if already tracing.
    */
   bool startTracing();
 
   /**
-   * Mark trace session as stopped. Returns `false` if wasn't tracing.
+   * If there is a current tracing session, it stops tracing and returns all
+   * collected events. Otherwise, it returns empty.
    */
-  bool stopTracing();
+  std::optional<std::vector<TraceEvent>> stopTracing();
 
   /**
    * Returns whether the tracer is currently tracing. This can be useful to
@@ -52,19 +52,6 @@ class PerformanceTracer {
     return tracingAtomic_;
   }
 
-  /**
-   * Flush out buffered CDP Trace Events using the given callback.
-   */
-  void collectEvents(
-      const std::function<void(folly::dynamic&& eventsChunk)>& resultCallback,
-      uint16_t chunkSize);
-
-  /**
-   * Transfers an ownership of all buffered TraceEvents, the local buffer state
-   * is invalidated after this call.
-   */
-  std::vector<TraceEvent> collectTraceEvents();
-
   /**
    * Record a `Performance.mark()` event - a labelled timestamp. If not
    * currently tracing, this is a no-op.