[Tracing w/SDK] Replace TraceStateObserver with TrackEventSessionObserver

With v8_use_perfetto = true, tracing sessions are controlled by
Perfetto, not TraceLog. This can lead to inconsistencies, e.g.
TraceLog signalling trace start while V8's TrackEvent datasource
hasn't been initialized yet.

This CL removes the TraceStateObserver interface and replaces its
uses with perfetto::TrackEventSessionObserver which correctly tracks
Perfetto tracing sessions start and end.

See also crrev.com/c/4066184 for the corresponding Chrome change.

Bug: chromium:1006766
Change-Id: I94b2189c8b28aec8b17ec8fc1246e27c904e4ee9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4062038
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Mikhail Khokhlov <khokhlov@google.com>
Cr-Commit-Position: refs/heads/main@{#84590}

include/libplatform/v8-tracing.h

@@ -282,12 +282,12 @@ class V8_PLATFORM_EXPORT TracingController
                                 const char* name, uint64_t handle) override;
 
   static const char* GetCategoryGroupName(const uint8_t* category_enabled_flag);
-#endif  // !defined(V8_USE_PERFETTO)
 
   void AddTraceStateObserver(
       v8::TracingController::TraceStateObserver* observer) override;
   void RemoveTraceStateObserver(
       v8::TracingController::TraceStateObserver* observer) override;
+#endif  // !defined(V8_USE_PERFETTO)
 
   void StartTracing(TraceConfig* trace_config);
   void StopTracing();
@@ -307,7 +307,6 @@ class V8_PLATFORM_EXPORT TracingController
   std::unique_ptr<base::Mutex> mutex_;
   std::unique_ptr<TraceConfig> trace_config_;
   std::atomic_bool recording_{false};
-  std::unordered_set<v8::TracingController::TraceStateObserver*> observers_;
 
 #if defined(V8_USE_PERFETTO)
   std::ostream* output_stream_ = nullptr;
@@ -316,6 +315,7 @@ class V8_PLATFORM_EXPORT TracingController
   TraceEventListener* listener_for_testing_ = nullptr;
   std::unique_ptr<perfetto::TracingSession> tracing_session_;
 #else   // !defined(V8_USE_PERFETTO)
+  std::unordered_set<v8::TracingController::TraceStateObserver*> observers_;
   std::unique_ptr<TraceBuffer> trace_buffer_;
 #endif  // !defined(V8_USE_PERFETTO)
 

include/v8-platform.h

@@ -285,6 +285,8 @@ class ConvertableToTraceFormat {
  * V8 Tracing controller.
  *
  * Can be implemented by an embedder to record trace events from V8.
+ *
+ * Will become obsolete in Perfetto SDK build (v8_use_perfetto = true).
  */
 class TracingController {
  public:
@@ -348,10 +350,16 @@ class TracingController {
     virtual void OnTraceDisabled() = 0;
   };
 
-  /** Adds tracing state change observer. */
+  /**
+   * Adds tracing state change observer.
+   * Does nothing in Perfetto SDK build (v8_use_perfetto = true).
+   */
   virtual void AddTraceStateObserver(TraceStateObserver*) {}
 
-  /** Removes tracing state change observer. */
+  /**
+   * Removes tracing state change observer.
+   * Does nothing in Perfetto SDK build (v8_use_perfetto = true).
+   */
   virtual void RemoveTraceStateObserver(TraceStateObserver*) {}
 };
 

src/libplatform/tracing/tracing-controller.cc

@@ -206,18 +206,19 @@ void TracingController::StartTracing(TraceConfig* trace_config) {
 #endif  // V8_USE_PERFETTO
 
   trace_config_.reset(trace_config);
+  recording_.store(true, std::memory_order_release);
+
+#ifndef V8_USE_PERFETTO
   std::unordered_set<v8::TracingController::TraceStateObserver*> observers_copy;
   {
     base::MutexGuard lock(mutex_.get());
-    recording_.store(true, std::memory_order_release);
-#ifndef V8_USE_PERFETTO
     UpdateCategoryGroupEnabledFlags();
-#endif
     observers_copy = observers_;
   }
   for (auto o : observers_copy) {
     o->OnTraceEnabled();
   }
+#endif
 }
 
 void TracingController::StopTracing() {
@@ -227,7 +228,6 @@ void TracingController::StopTracing() {
   }
 #ifndef V8_USE_PERFETTO
   UpdateCategoryGroupEnabledFlags();
-#endif
   std::unordered_set<v8::TracingController::TraceStateObserver*> observers_copy;
   {
     base::MutexGuard lock(mutex_.get());
@@ -236,6 +236,7 @@ void TracingController::StopTracing() {
   for (auto o : observers_copy) {
     o->OnTraceDisabled();
   }
+#endif
 
 #ifdef V8_USE_PERFETTO
   tracing_session_->StopBlocking();
@@ -341,7 +342,6 @@ const uint8_t* TracingController::GetCategoryGroupEnabled(
   }
   return category_group_enabled;
 }
-#endif  // !defined(V8_USE_PERFETTO)
 
 void TracingController::AddTraceStateObserver(
     v8::TracingController::TraceStateObserver* observer) {
@@ -360,6 +360,7 @@ void TracingController::RemoveTraceStateObserver(
   DCHECK(observers_.find(observer) != observers_.end());
   observers_.erase(observer);
 }
+#endif  // !defined(V8_USE_PERFETTO)
 
 }  // namespace tracing
 }  // namespace platform

src/profiler/tracing-cpu-profiler.cc

@@ -14,16 +14,31 @@ namespace internal {
 
 TracingCpuProfilerImpl::TracingCpuProfilerImpl(Isolate* isolate)
     : isolate_(isolate), profiling_enabled_(false) {
+#if defined(V8_USE_PERFETTO)
+  TrackEvent::AddSessionObserver(this);
+  // Fire the observer if tracing is already in progress.
+  if (TrackEvent::IsEnabled()) OnStart({});
+#else
   V8::GetCurrentPlatform()->GetTracingController()->AddTraceStateObserver(this);
+#endif
 }
 
 TracingCpuProfilerImpl::~TracingCpuProfilerImpl() {
   StopProfiling();
+#if defined(V8_USE_PERFETTO)
+  TrackEvent::RemoveSessionObserver(this);
+#else
   V8::GetCurrentPlatform()->GetTracingController()->RemoveTraceStateObserver(
       this);
+#endif
 }
 
+#if defined(V8_USE_PERFETTO)
+void TracingCpuProfilerImpl::OnStart(
+    const perfetto::DataSourceBase::StartArgs&) {
+#else
 void TracingCpuProfilerImpl::OnTraceEnabled() {
+#endif
   bool enabled;
   TRACE_EVENT_CATEGORY_GROUP_ENABLED(
       TRACE_DISABLED_BY_DEFAULT("v8.cpu_profiler"), &enabled);
@@ -36,7 +51,11 @@ void TracingCpuProfilerImpl::OnTraceEnabled() {
       this);
 }
 
+#if defined(V8_USE_PERFETTO)
+void TracingCpuProfilerImpl::OnStop(const perfetto::DataSourceBase::StopArgs&) {
+#else
 void TracingCpuProfilerImpl::OnTraceDisabled() {
+#endif
   base::MutexGuard lock(&mutex_);
   if (!profiling_enabled_) return;
   profiling_enabled_ = false;

src/profiler/tracing-cpu-profiler.h

@@ -11,6 +11,7 @@
 #include "src/base/atomic-utils.h"
 #include "src/base/macros.h"
 #include "src/base/platform/mutex.h"
+#include "src/tracing/trace-event.h"
 
 namespace v8 {
 namespace internal {
@@ -19,16 +20,26 @@ class CpuProfiler;
 class Isolate;
 
 class TracingCpuProfilerImpl final
+#if defined(V8_USE_PERFETTO)
+    : public perfetto::TrackEventSessionObserver {
+#else
     : private v8::TracingController::TraceStateObserver {
+#endif
  public:
   explicit TracingCpuProfilerImpl(Isolate*);
   ~TracingCpuProfilerImpl() override;
   TracingCpuProfilerImpl(const TracingCpuProfilerImpl&) = delete;
   TracingCpuProfilerImpl& operator=(const TracingCpuProfilerImpl&) = delete;
 
+#if defined(V8_USE_PERFETTO)
+  // perfetto::TrackEventSessionObserver
+  void OnStart(const perfetto::DataSourceBase::StartArgs&) override;
+  void OnStop(const perfetto::DataSourceBase::StopArgs&) override;
+#else
   // v8::TracingController::TraceStateObserver
   void OnTraceEnabled() final;
   void OnTraceDisabled() final;
+#endif
 
  private:
   void StartProfiling();

src/tracing/tracing-category-observer.cc

@@ -16,17 +16,32 @@ TracingCategoryObserver* TracingCategoryObserver::instance_ = nullptr;
 
 void TracingCategoryObserver::SetUp() {
   TracingCategoryObserver::instance_ = new TracingCategoryObserver();
+#if defined(V8_USE_PERFETTO)
+  TrackEvent::AddSessionObserver(instance_);
+  // Fire the observer if tracing is already in progress.
+  if (TrackEvent::IsEnabled()) instance_->OnStart({});
+#else
   i::V8::GetCurrentPlatform()->GetTracingController()->AddTraceStateObserver(
       TracingCategoryObserver::instance_);
+#endif
 }
 
 void TracingCategoryObserver::TearDown() {
+#if defined(V8_USE_PERFETTO)
+  TrackEvent::RemoveSessionObserver(TracingCategoryObserver::instance_);
+#else
   i::V8::GetCurrentPlatform()->GetTracingController()->RemoveTraceStateObserver(
       TracingCategoryObserver::instance_);
+#endif
   delete TracingCategoryObserver::instance_;
 }
 
+#if defined(V8_USE_PERFETTO)
+void TracingCategoryObserver::OnStart(
+    const perfetto::DataSourceBase::StartArgs&) {
+#else
 void TracingCategoryObserver::OnTraceEnabled() {
+#endif
   bool enabled = false;
   TRACE_EVENT_CATEGORY_GROUP_ENABLED(
       TRACE_DISABLED_BY_DEFAULT("v8.runtime_stats"), &enabled);
@@ -66,7 +81,12 @@ void TracingCategoryObserver::OnTraceEnabled() {
   }
 }
 
+#if defined(V8_USE_PERFETTO)
+void TracingCategoryObserver::OnStop(
+    const perfetto::DataSourceBase::StopArgs&) {
+#else
 void TracingCategoryObserver::OnTraceDisabled() {
+#endif
   i::TracingFlags::runtime_stats.fetch_and(
       ~(ENABLED_BY_TRACING | ENABLED_BY_SAMPLING), std::memory_order_relaxed);
 

src/tracing/tracing-category-observer.h

@@ -6,11 +6,17 @@
 #define V8_TRACING_TRACING_CATEGORY_OBSERVER_H_
 
 #include "include/v8-platform.h"
+#include "src/tracing/trace-event.h"
 
 namespace v8 {
 namespace tracing {
 
-class TracingCategoryObserver : public TracingController::TraceStateObserver {
+class TracingCategoryObserver
+#if defined(V8_USE_PERFETTO)
+    : public perfetto::TrackEventSessionObserver {
+#else
+    : public TracingController::TraceStateObserver {
+#endif
  public:
   enum Mode {
     ENABLED_BY_NATIVE = 1 << 0,
@@ -21,9 +27,15 @@ class TracingCategoryObserver : public TracingController::TraceStateObserver {
   static void SetUp();
   static void TearDown();
 
+#if defined(V8_USE_PERFETTO)
+  // perfetto::TrackEventSessionObserver
+  void OnStart(const perfetto::DataSourceBase::StartArgs&) override;
+  void OnStop(const perfetto::DataSourceBase::StopArgs&) override;
+#else
   // v8::TracingController::TraceStateObserver
   void OnTraceEnabled() final;
   void OnTraceDisabled() final;
+#endif
 
  private:
   static TracingCategoryObserver* instance_;

test/unittests/libplatform/tracing-unittest.cc

@@ -389,6 +389,10 @@ TEST_F(PlatformTracingTest, TestTracingControllerMultipleArgsAndCopy) {
 }
 #endif  // !defined(V8_USE_PERFETTO)
 
+// In Perfetto build there are no TracingObservers. Instead the code relies on
+// TrackEventSessionObserver to track tracing sessions, which is tested
+// upstream.
+#if !defined(V8_USE_PERFETTO)
 namespace {
 
 class TraceStateObserverImpl : public TracingController::TraceStateObserver {
@@ -412,16 +416,11 @@ TEST_F(PlatformTracingTest, TracingObservers) {
   v8::platform::tracing::TracingController* tracing_controller = tracing.get();
   static_cast<v8::platform::DefaultPlatform*>(default_platform.get())
       ->SetTracingController(std::move(tracing));
-#ifdef V8_USE_PERFETTO
-  std::ostringstream sstream;
-  tracing_controller->InitializeForPerfetto(&sstream);
-#else
   MockTraceWriter* writer = new MockTraceWriter();
   v8::platform::tracing::TraceBuffer* ring_buffer =
       v8::platform::tracing::TraceBuffer::CreateTraceBufferRingBuffer(1,
                                                                       writer);
   tracing_controller->Initialize(ring_buffer);
-#endif
   v8::platform::tracing::TraceConfig* trace_config =
       new v8::platform::tracing::TraceConfig();
   trace_config->AddIncludedCategory("v8");
@@ -469,6 +468,7 @@ TEST_F(PlatformTracingTest, TracingObservers) {
 
   i::V8::SetPlatformForTesting(old_platform);
 }
+#endif  // !defined(V8_USE_PERFETTO)
 
 // With Perfetto the tracing controller doesn't observe events.
 #if !defined(V8_USE_PERFETTO)