Add perfetto tracing to host tooling by jmacnak · Pull Request #2502 · google/android-cuttlefish

jmacnak · 2026-05-01T22:46:30Z

Example cvd create trace:
https://ui.perfetto.dev/#!/?s=315a987bda469405966eb46797201e11b12d247c

Example cvd fetch trace:
https://ui.perfetto.dev/#!/?s=8e6e182ffab8e92f0f0a690b205c73313ec71f17

jmacnak · 2026-05-18T14:55:17Z

This needs a couple upstream changes to Perfetto but generally ready for a first look.

Databean

Not finished reviewing, but I'll leave the comments I have so far.

Databean · 2026-05-19T01:10:31Z

+          worker_condition_variable_.wait(lock, [this] { return !trace_events_.empty() || worker_shutting_down_; });
+
+          if (worker_shutting_down_ && trace_events_.empty()) {
+            break;
+          }


Confirming my understanding: once worker_shutting_down_ is true, this will never wait on worker_condition_variable_ again, but will still make sure all events are handled before going into shutdown?

Databean · 2026-05-19T01:44:54Z

+
+  const bool enabled_ = false;
+
+  std::mutex mutex_;


Using std::recursive_mutex would avoid the extra Locked methods at the cost of some runtime performance.

Bug: b/378560215 This would be useful for google/android-cuttlefish#2502 where we could eliminate most of https://github.com/google/android-cuttlefish/pull/2502/changes#diff-d28a094890e585d0e9bec027b845e31c3c15ebd9c2262dd814dd6e7537ab2af8 when using the c library via `dlopen()` and we do not want to directly depend on the library itself.

Example cvd create trace: https://ui.perfetto.dev/#!/?s=315a987bda469405966eb46797201e11b12d247c Example cvd fetch trace: https://ui.perfetto.dev/#!/?s=8e6e182ffab8e92f0f0a690b205c73313ec71f17 Perfetto uses a shared memory buffer between producer processes and the traced consumer daemon. `fork()` is problematic for this as the parent and child processes would end up clobbering each other when the shared memory buffer is inherited by the child. To prevent this, we introduce a perfetto wrapper that uses `pthread_atfork()` to register handlers that shutdown and restart tracing before and after forking. Perfetto also depends on global static data and explicitly calls out that it can not be restarted unless this data is cleared [1]. With this, the Perfetto wrapper utilizes `dlopen()` and `dlclose()` during init and shutdown. Perfetto also uses thread local data which requires extra care around `dlclose()` to prevent some thread local destructors from running after `dlclose()` has already unloaded the Perfetto library. To work around this, the Perfetto wrapper isolates all Perfetto calls into a single worker thread. [1] https://github.com/google/perfetto/blob/ab21398d658ce3bccc192b226d2731add6c02890/include/perfetto/tracing/tracing.h#L252 Bug: b/378560215

Databean

Sorry, still not finished: I haven't fully reviewed TracingState, but have looked at everything else.

Databean · 2026-05-19T23:04:09Z

 }

 Result<bool> DiskBuilder::BuildCompositeDiskIfNecessary() {
+  CF_TRACEF("BuildCompositeDisk: %s", composite_disk_path_.c_str());


Does this and the other CF_TRACEF still need to use .c_str() if absl::StrFormat is producing the string?

Databean · 2026-05-19T23:04:49Z

+    std::string downloaded;
+
+    {
+      CF_TRACEF("Download: %s", artifact_name_.c_str());


nit: Does this and the other CF_TRACEF still need .c_str()?

Databean · 2026-05-19T23:05:43Z

    setenv(kCuttlefishInstanceEnvVarName, instance.id().c_str(),
           /* overwrite */ 1);

+    CF_TRACEF("Running run_cvd: %s", instance.instance_name().c_str());


nit: is the .c_str() still necessary?

Databean · 2026-05-19T23:05:59Z

            "Dependency issue detected, not performing any setup.");
  // TODO(b/189153501): This can potentially be parallelized.
  for (auto& feature : ordered_features) {
+    CF_TRACEF("FeatureSetup %s", feature->Name().c_str());


nit: is the .c_str() still necessary?

Databean · 2026-05-19T23:19:01Z

+  // Emit trace end events for all of the active traces. If we are forking,
+  // the trace begin events will be re-emitted after the fork.
+  const uint64_t shutdown_timestamp = GetTimestamp();
+  for (auto& [_, thread_state] : thread_states_) {


nit: _ is not a placeholder, this declares a variable named _.

Databean · 2026-05-20T01:37:03Z

+  struct LibraryCloser {
+    void operator()(void* handle) const;
+  };
+  std::unique_ptr<void, LibraryCloser> handle;


nit: handle seems like private state to PerfettoFunctions, I don't see it accessed outside of PerfettoFunctions::Load.

Databean · 2026-05-20T01:41:05Z

+  absl::MutexLock lock(mutex_);
+  trace_events_.push_back(event);


If the worker thread fails to start, this will accumulate trace events without ever draining them. Maybe this can reuse the shutting_down_ variable: if WorkerThreadLoop exits early it can set shutting_down_, and EnqueueTraceEvent can drop events if shutting_down_ is true.

Databean · 2026-05-20T02:00:53Z

+  if (!LoadPerfetto()) {
+    LOG(WARNING) << "Failed to initialize tracing: failed to load Perfetto.";
+    return;
+  }


As far as I can tell, this code upholds the property that perfetto_ is not used if LoadPerfetto() fails. However, this is only only implicit: perfetto_ can be in uninitialized or initialized states, and the initialization state is never checked.

What do you think of accepting the PerfettoFunctions instance as a constructor argument? If you want to get really fancy, it would also be possible to write unit tests with a mock PerfettoFunctions instance by filling in the function pointers with mocks / stubs.

Databean · 2026-05-20T02:07:37Z

+  PFN_PerfettoTePublishCategories PerfettoTePublishCategories = nullptr;
+  uint64_t* perfetto_te_process_track_uuid_ptr = nullptr;
+
+  static Result<PerfettoFunctions> Load();


Related to the other comment about initialization in PerfettoWorkerThread, part of the intention of static factory functions (https://abseil.io/tips/42) is that the static factory function is the only way of producing an instance. Can the default constructor be made private?

Databean · 2026-05-20T02:30:58Z

+    if (res != 0) {
+      LOG(ERROR) << "Failed to register pthread fork callbacks.";
+    }
+    std::atexit(TracingAtExit);


Abseil has its own concept of "run on exit" handlers:

https://github.com/abseil/abseil-cpp/blob/8fb4507d10a24a1204e98f77ad19c27c291e13e9/absl/log/log.h#L28-L31

Oddly this seems to be internal-only: it references process_state.h which is not public. Despite this, if I understand correclty abseil still uses abort (ignoring atexit handlers) internally to exit the process in LOG(FATAL) and CHECK cases:

https://github.com/abseil/abseil-cpp/blob/8fb4507d10a24a1204e98f77ad19c27c291e13e9/absl/log/internal/conditions.h#L241

However, this contradicts the abseil documentation which claims that LOG(QFATAL) and QCHECK are the only version that don't run exit handlers. As far as I can tell from the source though, LOG(FATAL) calls abort and LOG(QFATAL) calls _exit, neither of which is much help.

All that to say, not sure there's anything we can do about it.

jmacnak force-pushed the perfetto branch 7 times, most recently from e958a29 to 8330f27 Compare May 6, 2026 17:15

jmacnak added the kokoro:force-run Trigger a presubmit build unconditionally. label May 6, 2026

GoogleCuttlefishTesterBot removed the kokoro:force-run Trigger a presubmit build unconditionally. label May 6, 2026

jmacnak force-pushed the perfetto branch 4 times, most recently from 19542bf to 6e9eaf0 Compare May 7, 2026 18:24

jmacnak force-pushed the perfetto branch 5 times, most recently from 1d90f66 to 1727b13 Compare May 15, 2026 20:30

jmacnak mentioned this pull request May 18, 2026

shared_lib: Expose shutdown google/perfetto#5936

Open

jmacnak requested a review from Databean May 18, 2026 14:54

jmacnak mentioned this pull request May 18, 2026

shared_lib: Expose header only library in Bazel build google/perfetto#5955

Merged

Databean reviewed May 19, 2026

View reviewed changes

jmacnak force-pushed the perfetto branch 4 times, most recently from d806762 to 7be473c Compare May 19, 2026 20:15

jmacnak force-pushed the perfetto branch from 7be473c to 7e57fd5 Compare May 19, 2026 20:19

Databean reviewed May 20, 2026

View reviewed changes

		absl::MutexLock lock(mutex_);
		trace_events_.push_back(event);

Conversation

jmacnak commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmacnak commented May 18, 2026

Uh oh!

Databean left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Databean left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jmacnak commented May 1, 2026 •

edited

Loading