[tracing] Fix TracingCpuProfiler test for Perfetto

Update the TracingCpuProfiler test to work properly with perfetto.

Roll perfetto to get fixes for bugs encountered with starting/stopping
tracing rapidly, which happens in the test for the tracing profiler.

Add a check that the DataSource::Register call was successful to flush
out any errors there (although they are fixed by the perfetto roll).

Emit a fake trace event when stopping tracing in order to avoid
losing the final trace event that the user provided. Remove the ad-hoc
fake final trace events that the cctests for perfetto added.

Add a test StartAndStopRepeated which flushes out the issues fixed by
the perfetto roll.

TBR=yangguo@chromium.org

Cq-Include-Trybots: luci.v8.try:v8_linux64_perfetto_dbg_ng
Bug: v8:8339
Change-Id: I042d7385486bf42c86f1631406974693868a477f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1731006
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63083}

DEPS

@@ -233,7 +233,7 @@ deps = {
   'v8/test/wasm-js/data':
     Var('chromium_url') + '/external/github.com/WebAssembly/spec.git' + '@' + 'a221f2574d7106e92cf8abaf05d5bb1131b19d76',
   'v8/third_party/perfetto':
-    Var('android_url') + '/platform/external/perfetto.git' + '@' + '0e8281399fd854de13461f2c1c9f2fb0b8e9c3ae',
+    Var('android_url') + '/platform/external/perfetto.git' + '@' + '3150f3961532dccfe18a6c87c9494f754d3b6a51',
   'v8/third_party/protobuf':
     Var('chromium_url') + '/external/github.com/google/protobuf'+ '@' + 'b68a347f56137b4b1a746e8c7438495a6ac1bd91',
 }

include/libplatform/v8-tracing.h

@@ -244,6 +244,8 @@ class V8_PLATFORM_EXPORT TracingController
 
   TracingController();
   ~TracingController() override;
+
+  // Takes ownership of |trace_buffer|.
   void Initialize(TraceBuffer* trace_buffer);
 #ifdef V8_USE_PERFETTO
   // Must be called before StartTracing() if V8_USE_PERFETTO is true. Provides

src/libplatform/tracing/trace-buffer.h

@@ -17,6 +17,7 @@ namespace tracing {
 
 class TraceBufferRingBuffer : public TraceBuffer {
  public:
+  // Takes ownership of |trace_writer|.
   TraceBufferRingBuffer(size_t max_chunks, TraceWriter* trace_writer);
   ~TraceBufferRingBuffer() override = default;
 

src/libplatform/tracing/tracing-controller.cc

@@ -294,7 +294,8 @@ void TracingController::StartTracing(TraceConfig* trace_config) {
 
   perfetto::DataSourceDescriptor dsd;
   dsd.set_name("v8.trace_events");
-  V8DataSource::Register(dsd);
+  bool registered = V8DataSource::Register(dsd);
+  CHECK(registered);
 
   tracing_session_ =
       perfetto::Tracing::NewTrace(perfetto::BackendType::kUnspecifiedBackend);
@@ -334,6 +335,16 @@ void TracingController::StopTracing() {
   }
 
 #ifdef V8_USE_PERFETTO
+  // Emit a fake trace event from the main thread. The final trace event is
+  // sometimes skipped because perfetto can't guarantee that the caller is
+  // totally finished writing to it without synchronization. To avoid the
+  // situation where we lose the last trace event, add a fake one here that will
+  // be sacrificed.
+  // TODO(petermarshall): Use the Client API to flush here rather than this
+  // workaround when that becomes available.
+  V8DataSource::Trace([&](V8DataSource::TraceContext ctx) {
+    auto packet = ctx.NewTracePacket();
+  });
   base::Semaphore stopped_{0};
   tracing_session_->SetOnStopCallback([&stopped_]() { stopped_.Signal(); });
   tracing_session_->Stop();

test/cctest/DEPS

@@ -2,5 +2,5 @@ include_rules = [
   "+src",
   "+tools",
   "+torque-generated",
-  "+perfetto/tracing.h"
+  "+perfetto",
 ]

test/cctest/libplatform/test-tracing.cc

@@ -625,8 +625,6 @@ TEST(Perfetto) {
     TRACE_EVENT1("v8", "test2", "arg1", uint64_arg);
     TRACE_EVENT2("v8", "test3", "arg1", uint64_arg, "arg2", str_arg);
   }
-  TRACE_EVENT_INSTANT0("v8", "final event not captured",
-                       TRACE_EVENT_SCOPE_THREAD);
 
   harness.StopTracing();
 
@@ -688,8 +686,6 @@ TEST(Categories) {
     TRACE_EVENT0("cat", "v8.Test2");
     TRACE_EVENT0("v8", "v8.Test3");
   }
-  TRACE_EVENT_INSTANT0("v8", "final event not captured",
-                       TRACE_EVENT_SCOPE_THREAD);
 
   harness.StopTracing();
 
@@ -765,8 +761,6 @@ TEST(MultipleArgsAndCopy) {
                          std::move(trace_event_arg), "a2",
                          new ConvertableToTraceFormatMock(123));
   }
-  TRACE_EVENT_INSTANT0("v8", "final event not captured",
-                       TRACE_EVENT_SCOPE_THREAD);
 
   harness.StopTracing();
 
@@ -894,8 +888,6 @@ TEST(JsonIntegrationTest) {
     TRACE_EVENT1("v8", "v8.Test.3", "3", inf_num);
     TRACE_EVENT1("v8", "v8.Test.4", "4", neg_inf_num);
   }
-  TRACE_EVENT_INSTANT0("v8", "final event not captured",
-                       TRACE_EVENT_SCOPE_THREAD);
 
   harness.StopTracing();
   std::string json = harness.perfetto_json_stream();
@@ -957,6 +949,40 @@ TEST(TracingPerfetto) {
   CHECK_GT(perfetto_json_stream_.str().length(), 0);
 }
 
+TEST(StartAndStopRepeated) {
+  for (int i = 0; i < 3; i++) {
+    ::perfetto::TraceConfig perfetto_trace_config;
+    perfetto_trace_config.add_buffers()->set_size_kb(4096);
+    auto* ds_config =
+        perfetto_trace_config.add_data_sources()->mutable_config();
+    ds_config->set_name("v8.trace_events");
+
+    perfetto::DataSourceDescriptor dsd;
+    dsd.set_name("v8.trace_events");
+    TestDataSource::Register(dsd);
+
+    auto tracing_session_ =
+        perfetto::Tracing::NewTrace(perfetto::BackendType::kInProcessBackend);
+    tracing_session_->Setup(perfetto_trace_config);
+    tracing_session_->StartBlocking();
+
+    for (int i = 0; i < 15; i++) {
+      TestDataSource::Trace([&](TestDataSource::TraceContext ctx) {
+        auto packet = ctx.NewTracePacket();
+        auto* trace_event_bundle = packet->set_chrome_events();
+        auto* trace_event = trace_event_bundle->add_trace_events();
+
+        trace_event->set_phase('c');
+        trace_event->set_thread_id(v8::base::OS::GetCurrentThreadId());
+        trace_event->set_timestamp(123);
+        trace_event->set_process_id(v8::base::OS::GetCurrentProcessId());
+        trace_event->set_thread_timestamp(123);
+      });
+    }
+    tracing_session_->StopBlocking();
+  }
+}
+
 #endif  // V8_USE_PERFETTO
 
 }  // namespace tracing

test/cctest/test-cpu-profiler.cc

@@ -48,8 +48,14 @@
 #include "test/cctest/profiler-extension.h"
 
 #include "include/libplatform/v8-tracing.h"
+#include "src/libplatform/tracing/trace-event-listener.h"
 #include "src/tracing/trace-event.h"
 
+#ifdef V8_USE_PERFETTO
+#include "perfetto/trace/chrome/chrome_trace_event.pb.h"
+#include "perfetto/trace/trace.pb.h"
+#endif
+
 namespace v8 {
 namespace internal {
 namespace test_cpu_profiler {
@@ -2635,6 +2641,41 @@ using v8::platform::tracing::TraceObject;
 
 namespace {
 
+#ifdef V8_USE_PERFETTO
+
+class CpuProfilerListener : public platform::tracing::TraceEventListener {
+ public:
+  void ProcessPacket(const ::perfetto::protos::TracePacket& packet) {
+    for (const ::perfetto::protos::ChromeTraceEvent& trace_event :
+         packet.chrome_events().trace_events()) {
+      if (trace_event.name() != std::string("Profile") &&
+          trace_event.name() != std::string("ProfileChunk"))
+        return;
+      CHECK(!profile_id_ || trace_event.id() == profile_id_);
+      CHECK_EQ(1, trace_event.args_size());
+      CHECK(trace_event.args()[0].has_json_value());
+      profile_id_ = trace_event.id();
+      result_json_ += result_json_.empty() ? "[" : ",\n";
+      result_json_ += trace_event.args()[0].json_value();
+    }
+  }
+
+  const std::string& result_json() {
+    result_json_ += "]";
+    return result_json_;
+  }
+  void Reset() {
+    result_json_.clear();
+    profile_id_ = 0;
+  }
+
+ private:
+  std::string result_json_;
+  uint64_t profile_id_ = 0;
+};
+
+#else
+
 class CpuProfileEventChecker : public v8::platform::tracing::TraceWriter {
  public:
   void AppendTraceEvent(TraceObject* trace_event) override {
@@ -2663,6 +2704,8 @@ class CpuProfileEventChecker : public v8::platform::tracing::TraceWriter {
   uint64_t profile_id_ = 0;
 };
 
+#endif  // !V8_USE_PERFETTO
+
 }  // namespace
 
 TEST(TracingCpuProfiler) {
@@ -2670,17 +2713,20 @@ TEST(TracingCpuProfiler) {
   v8::Local<v8::Context> env = CcTest::NewContext({PROFILER_EXTENSION_ID});
   v8::Context::Scope context_scope(env);
 
-  CpuProfileEventChecker* event_checker = new CpuProfileEventChecker();
-  TraceBuffer* ring_buffer =
-      TraceBuffer::CreateTraceBufferRingBuffer(1, event_checker);
   auto* tracing_controller =
       static_cast<v8::platform::tracing::TracingController*>(
           i::V8::GetCurrentPlatform()->GetTracingController());
-  tracing_controller->Initialize(ring_buffer);
 
 #ifdef V8_USE_PERFETTO
   std::ostringstream perfetto_output;
   tracing_controller->InitializeForPerfetto(&perfetto_output);
+  CpuProfilerListener listener;
+  tracing_controller->SetTraceEventListenerForTesting(&listener);
+#else
+  CpuProfileEventChecker* event_checker = new CpuProfileEventChecker();
+  TraceBuffer* ring_buffer =
+      TraceBuffer::CreateTraceBufferRingBuffer(1, event_checker);
+  tracing_controller->Initialize(ring_buffer);
 #endif
 
   bool result = false;
@@ -2705,8 +2751,13 @@ TEST(TracingCpuProfiler) {
     CompileRun(test_code.c_str());
     tracing_controller->StopTracing();
 
+#ifdef V8_USE_PERFETTO
+    std::string profile_json = listener.result_json();
+    listener.Reset();
+#else
     std::string profile_json = event_checker->result_json();
     event_checker->Reset();
+#endif
     CHECK_LT(0u, profile_json.length());
     printf("Profile JSON: %s\n", profile_json.c_str());