From 3ed1af8a3cb40aa729e2bec79d491b0402014785 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 26 Jun 2025 04:50:40 -0700 Subject: [PATCH] Fix incorrect rebase, apply lost changes (#52288) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/52288 # Changelog: [Internal] Reviewed By: motiz88, rubennorte Differential Revision: D77368159 fbshipit-source-id: 3beefb01d8e25596269c1212ce90864f39fd9f0c --- .../tracing/PerformanceTracer.cpp | 55 +++++++++---------- .../tracing/PerformanceTracer.h | 28 ++++++++-- 2 files changed, 49 insertions(+), 34 deletions(-) diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp index 3112d0e7f70..7dac97c4c78 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.cpp @@ -28,18 +28,18 @@ PerformanceTracer::PerformanceTracer() bool PerformanceTracer::startTracing() { { - std::lock_guard lock(tracingMutex_); - if (isTracing()) { + std::lock_guard lock(mutex_); + if (tracingAtomic_) { return false; } - tracing_ = true; + tracingAtomic_ = true; } reportProcess(processId_, "React Native"); { - std::lock_guard lock(tracingMutex_); - if (!isTracing()) { + std::lock_guard lock(mutex_); + if (!tracingAtomic_) { return false; } buffer_.emplace_back(TraceEvent{ @@ -57,11 +57,11 @@ bool PerformanceTracer::startTracing() { } bool PerformanceTracer::stopTracing() { - std::lock_guard lock(tracingMutex_); - if (!isTracing()) { + std::lock_guard lock(mutex_); + if (!tracingAtomic_) { return false; } - tracing_ = false; + 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 @@ -78,7 +78,6 @@ bool PerformanceTracer::stopTracing() { .tid = oscompat::getCurrentThreadId(), }); - // Potential increments of this counter are covered by tracing_ atomic flag. performanceMeasureCount_ = 0; return true; } @@ -89,7 +88,7 @@ void PerformanceTracer::collectEvents( uint16_t chunkSize) { std::vector localBuffer; { - std::lock_guard lock(tracingMutex_); + std::lock_guard lock(mutex_); buffer_.swap(localBuffer); } @@ -115,12 +114,12 @@ void PerformanceTracer::collectEvents( void PerformanceTracer::reportMark( const std::string_view& name, HighResTimeStamp start) { - if (!isTracing()) { + if (!tracingAtomic_) { return; } - std::lock_guard lock(tracingMutex_); - if (!isTracing()) { + std::lock_guard lock(mutex_); + if (!tracingAtomic_) { return; } @@ -139,7 +138,7 @@ void PerformanceTracer::reportMeasure( HighResTimeStamp start, HighResDuration duration, const std::optional& trackMetadata) { - if (!isTracing()) { + if (!tracingAtomic_) { return; } @@ -154,8 +153,8 @@ void PerformanceTracer::reportMeasure( auto currentThreadId = oscompat::getCurrentThreadId(); - std::lock_guard lock(tracingMutex_); - if (!isTracing()) { + std::lock_guard lock(mutex_); + if (!tracingAtomic_) { return; } auto eventId = ++performanceMeasureCount_; @@ -182,12 +181,12 @@ void PerformanceTracer::reportMeasure( } void PerformanceTracer::reportProcess(uint64_t id, const std::string& name) { - if (!isTracing()) { + if (!tracingAtomic_) { return; } - std::lock_guard lock(tracingMutex_); - if (!isTracing()) { + std::lock_guard lock(mutex_); + if (!tracingAtomic_) { return; } @@ -207,12 +206,12 @@ void PerformanceTracer::reportJavaScriptThread() { } void PerformanceTracer::reportThread(uint64_t id, const std::string& name) { - if (!isTracing()) { + if (!tracingAtomic_) { return; } - std::lock_guard lock(tracingMutex_); - if (!isTracing()) { + std::lock_guard lock(mutex_); + if (!tracingAtomic_) { return; } @@ -244,12 +243,12 @@ void PerformanceTracer::reportThread(uint64_t id, const std::string& name) { void PerformanceTracer::reportEventLoopTask( HighResTimeStamp start, HighResTimeStamp end) { - if (!isTracing()) { + if (!tracingAtomic_) { return; } - std::lock_guard lock(tracingMutex_); - if (!isTracing()) { + std::lock_guard lock(mutex_); + if (!tracingAtomic_) { return; } @@ -267,12 +266,12 @@ void PerformanceTracer::reportEventLoopTask( void PerformanceTracer::reportEventLoopMicrotasks( HighResTimeStamp start, HighResTimeStamp end) { - if (!isTracing()) { + if (!tracingAtomic_) { return; } - std::lock_guard lock(tracingMutex_); - if (!isTracing()) { + std::lock_guard lock(mutex_); + if (!tracingAtomic_) { return; } diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h index 5cacb3971af..1a3c0a3be08 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tracing/PerformanceTracer.h @@ -49,7 +49,7 @@ class PerformanceTracer { * enabled. */ inline bool isTracing() const { - return tracing_; + return tracingAtomic_; } /** @@ -140,14 +140,30 @@ class PerformanceTracer { */ folly::dynamic serializeTraceEvent(TraceEvent&& event) const; - uint64_t processId_; + const uint64_t processId_; - std::atomic tracing_{false}; - std::atomic performanceMeasureCount_{0}; + /** + * The flag is atomic in order to enable any thread to read it (via + * isTracing()) without holding the mutex. + * Within this class, both reads and writes MUST be protected by the mutex to + * avoid false positives and data races. + */ + std::atomic tracingAtomic_{false}; + /** + * The counter for recorded User Timing "measure" events. + * Used for generating unique IDs for each measure event inside a specific + * Trace. + * Does not need to be atomic, because it is always accessed within the mutex + * lock. + */ + uint32_t performanceMeasureCount_{0}; std::vector buffer_; - // Protects buffer_ operations and tracing_ modifications. - std::mutex tracingMutex_; + /** + * Protects data members of this class for concurrent access, including + * the tracingAtomic_, in order to eliminate potential "logic" races. + */ + std::mutex mutex_; }; } // namespace facebook::react::jsinspector_modern::tracing