From c95570cc798a3735ee7522d490c97b52c8bc18d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Thu, 31 Jul 2025 10:12:30 -0700 Subject: [PATCH 1/2] Remove unnecessary collectEvents method from PerformanceTracer Differential Revision: D79271691 --- .../tracing/PerformanceTracer.cpp | 30 ------------------- .../tracing/PerformanceTracer.h | 6 ---- 2 files changed, 36 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp index e287b6f584d4..f23c77253821 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp @@ -101,36 +101,6 @@ void PerformanceTracer::collectEvents( } } -folly::dynamic PerformanceTracer::collectEvents(uint16_t chunkSize) { - std::vector localBuffer; - { - std::lock_guard lock(mutex_); - buffer_.swap(localBuffer); - } - - auto chunks = folly::dynamic::array(); - if (localBuffer.empty()) { - return chunks; - } - - auto chunk = folly::dynamic::array(); - chunk.reserve(chunkSize); - for (auto&& event : localBuffer) { - chunk.push_back(TraceEventSerializer::serialize(std::move(event))); - - if (chunk.size() == chunkSize) { - chunks.push_back(std::move(chunk)); - chunk = folly::dynamic::array(); - chunk.reserve(chunkSize); - } - } - - if (!chunk.empty()) { - chunks.push_back(std::move(chunk)); - } - return chunks; -} - std::vector PerformanceTracer::collectTraceEvents() { std::vector events; { diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h index 088f98a77586..530dce0d44f3 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h @@ -59,12 +59,6 @@ class PerformanceTracer { const std::function& resultCallback, uint16_t chunkSize); - /** - * Flush out buffered CDP Trace Events into a folly::dynamic collection of - * chunks, which can be sent over CDP later. - */ - folly::dynamic collectEvents(uint16_t chunkSize); - /** * Transfers an ownership of all buffered TraceEvents, the local buffer state * is invalidated after this call. From 3bb7195508e08caac44ebed4076d94121949af7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Thu, 31 Jul 2025 10:16:57 -0700 Subject: [PATCH 2/2] Simplify PerformanceTracer API and move processing to TracingAgent (#52934) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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 --- .../jsinspector-modern/TracingAgent.cpp | 32 +++++++- .../tracing/PerformanceTracer.cpp | 74 +++++-------------- .../tracing/PerformanceTracer.h | 21 +----- 3 files changed, 50 insertions(+), 77 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp index 24bcb0d25e11..bf9d31d24642 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp @@ -9,6 +9,7 @@ #include #include +#include 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&& traceEvents, + uint16_t chunkSize, + const std::function& 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; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp index f23c77253821..6ff316aec34f 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp +++ b/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> PerformanceTracer::stopTracing() { + std::vector 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& resultCallback, - uint16_t chunkSize) { - std::vector 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 PerformanceTracer::collectTraceEvents() { - std::vector events; - { - std::lock_guard lock(mutex_); - buffer_.swap(events); - } return events; } diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h index 530dce0d44f3..533a6c388c94 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h @@ -15,7 +15,6 @@ #include #include -#include #include #include #include @@ -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> 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& resultCallback, - uint16_t chunkSize); - - /** - * Transfers an ownership of all buffered TraceEvents, the local buffer state - * is invalidated after this call. - */ - std::vector collectTraceEvents(); - /** * Record a `Performance.mark()` event - a labelled timestamp. If not * currently tracing, this is a no-op.