ps_tree: No scan on startup by default, make thread names optional

There can be over 1500 threads in total and dumping all of them at
startup causes:
1) Too much use of the trace buffer.
2) Because of 1, the buffer is more likely to wrap, and we are more
   likely to lose the whole TracePacket with the ps tree. However,
   once the initial dump is made, the data source is convinced that
   we recorded all those threads and will stop emitting them later
   on. As such we get ironically less data compared to the
   on-deman only case.

This CL:
- Makes the dump on startup optional.
- Makes dumping thread names optional (they are redundant given that
  they show up already in sched_switch events)

This CL diable the scan on startup

Bug: 78106582
Change-Id: I12e9c3e32f4af9e0dd5bb4d67be88706bb47b40e
Merged-In: I12e9c3e32f4af9e0dd5bb4d67be88706bb47b40e
(cherry picked from commit e8d7595f35ff1ad68581164d18d762afb3c03383)

8 files changed, 81 insertions, 11 deletions

diff --git a/include/perfetto/tracing/core/process_stats_config.h b/include/perfetto/tracing/core/process_stats_config.h
index 2601ca0de..c89ee86ff 100644
--- a/include/perfetto/tracing/core/process_stats_config.h
+++ b/include/perfetto/tracing/core/process_stats_config.h
@@ -69,8 +69,20 @@ class PERFETTO_EXPORT ProcessStatsConfig {
     return &quirks_.back();
   }
 
+  bool scan_all_processes_on_start() const {
+    return scan_all_processes_on_start_;
+  }
+  void set_scan_all_processes_on_start(bool value) {
+    scan_all_processes_on_start_ = value;
+  }
+
+  bool record_thread_names() const { return record_thread_names_; }
+  void set_record_thread_names(bool value) { record_thread_names_ = value; }
+
  private:
   std::vector<Quirks> quirks_;
+  bool scan_all_processes_on_start_ = {};
+  bool record_thread_names_ = {};
 
   // Allows to preserve unknown protobuf fields for compatibility
   // with future versions of .proto files.
diff --git a/protos/perfetto/config/perfetto_config.proto b/protos/perfetto/config/perfetto_config.proto
index 5cddc6216..48202b8a7 100644
--- a/protos/perfetto/config/perfetto_config.proto
+++ b/protos/perfetto/config/perfetto_config.proto
@@ -64,11 +64,23 @@ message InodeFileConfig {
 message ProcessStatsConfig {
   enum Quirks {
     QUIRKS_UNSPECIFIED = 0;
-    DISABLE_INITIAL_DUMP = 1;
+
+    // This has been deprecated and ignored as per 2018-05-01. Full scan at
+    // startup is now disabled by default and can be re-enabled using the
+    // |scan_all_processes_on_start| arg.
+    DISABLE_INITIAL_DUMP = 1 [deprecated = true];
+
     DISABLE_ON_DEMAND = 2;
   }
 
   repeated Quirks quirks = 1;
+
+  // If enabled all processes will be scanned and dumped when the trace starts.
+  optional bool scan_all_processes_on_start = 2;
+
+  // If enabled thread names are also recoded (this is redundant if sched_switch
+  // is enabled).
+  optional bool record_thread_names = 3;
 }
 
 // End of protos/perfetto/config/process_stats/process_stats_config.proto
diff --git a/protos/perfetto/config/process_stats/process_stats_config.proto b/protos/perfetto/config/process_stats/process_stats_config.proto
index 2f33988b2..de224f605 100644
--- a/protos/perfetto/config/process_stats/process_stats_config.proto
+++ b/protos/perfetto/config/process_stats/process_stats_config.proto
@@ -25,9 +25,21 @@ package perfetto.protos;
 message ProcessStatsConfig {
   enum Quirks {
     QUIRKS_UNSPECIFIED = 0;
-    DISABLE_INITIAL_DUMP = 1;
+
+    // This has been deprecated and ignored as per 2018-05-01. Full scan at
+    // startup is now disabled by default and can be re-enabled using the
+    // |scan_all_processes_on_start| arg.
+    DISABLE_INITIAL_DUMP = 1 [deprecated = true];
+
     DISABLE_ON_DEMAND = 2;
   }
 
   repeated Quirks quirks = 1;
+
+  // If enabled all processes will be scanned and dumped when the trace starts.
+  optional bool scan_all_processes_on_start = 2;
+
+  // If enabled thread names are also recoded (this is redundant if sched_switch
+  // is enabled).
+  optional bool record_thread_names = 3;
 }
diff --git a/src/traced/probes/probes_producer.cc b/src/traced/probes/probes_producer.cc
index be97d1260..ce06ab8cf 100644
--- a/src/traced/probes/probes_producer.cc
+++ b/src/traced/probes/probes_producer.cc
@@ -226,15 +226,14 @@ void ProbesProducer::CreateProcessStatsDataSourceInstance(
   auto source = std::unique_ptr<ProcessStatsDataSource>(
       new ProcessStatsDataSource(session_id, std::move(trace_writer), config));
   auto it_and_inserted = process_stats_sources_.emplace(id, std::move(source));
-  PERFETTO_DCHECK(it_and_inserted.second);
-  const auto& quirks =
-      it_and_inserted.first->second->config().process_stats_config().quirks();
-  if (std::find(quirks.begin(), quirks.end(),
-                ProcessStatsConfig::DISABLE_INITIAL_DUMP) != quirks.end()) {
-    PERFETTO_DLOG("Initial process tree dump is disabled.");
+  if (!it_and_inserted.second) {
+    PERFETTO_DCHECK(false);
     return;
   }
-  it_and_inserted.first->second->WriteAllProcesses();
+  ProcessStatsDataSource* ps_data_source = it_and_inserted.first->second.get();
+  if (config.process_stats_config().scan_all_processes_on_start()) {
+    ps_data_source->WriteAllProcesses();
+  }
 }
 
 void ProbesProducer::TearDownDataSourceInstance(DataSourceInstanceID id) {
diff --git a/src/traced/probes/process_stats_data_source.cc b/src/traced/probes/process_stats_data_source.cc
index ca15a9a29..882676a2f 100644
--- a/src/traced/probes/process_stats_data_source.cc
+++ b/src/traced/probes/process_stats_data_source.cc
@@ -70,6 +70,7 @@ ProcessStatsDataSource::ProcessStatsDataSource(
     : session_id_(id),
       writer_(std::move(writer)),
       config_(config),
+      record_thread_names_(config.process_stats_config().record_thread_names()),
       weak_factory_(this) {}
 
 ProcessStatsDataSource::~ProcessStatsDataSource() = default;
@@ -113,6 +114,9 @@ void ProcessStatsDataSource::OnPids(const std::vector<int32_t>& pids) {
 }
 
 void ProcessStatsDataSource::Flush() {
+  // We shouldn't get this in the middle of WriteAllProcesses() or OnPids().
+  PERFETTO_DCHECK(!cur_ps_tree_);
+
   writer_->Flush();
 }
 
@@ -156,7 +160,8 @@ void ProcessStatsDataSource::WriteThread(int32_t tid,
   auto* thread = GetOrCreatePsTree()->add_threads();
   thread->set_tid(tid);
   thread->set_tgid(tgid);
-  thread->set_name(ReadProcStatusEntry(proc_status, "Name:").c_str());
+  if (record_thread_names_)
+    thread->set_name(ReadProcStatusEntry(proc_status, "Name:").c_str());
   seen_pids_.emplace(tid);
 }
 
diff --git a/src/traced/probes/process_stats_data_source.h b/src/traced/probes/process_stats_data_source.h
index eb19e6be8..0274487bd 100644
--- a/src/traced/probes/process_stats_data_source.h
+++ b/src/traced/probes/process_stats_data_source.h
@@ -64,6 +64,7 @@ class ProcessStatsDataSource {
   const DataSourceConfig config_;
   TraceWriter::TracePacketHandle cur_packet_;
   protos::pbzero::ProcessTree* cur_ps_tree_ = nullptr;
+  bool record_thread_names_ = false;
 
   // This set contains PIDs as per the Linux kernel notion of a PID (which is
   // really a TID). In practice this set will contain all TIDs for all processes
diff --git a/src/traced/probes/process_stats_data_source_unittest.cc b/src/traced/probes/process_stats_data_source_unittest.cc
index 6766ec266..ce3db7bc9 100644
--- a/src/traced/probes/process_stats_data_source_unittest.cc
+++ b/src/traced/probes/process_stats_data_source_unittest.cc
@@ -73,7 +73,9 @@ TEST_F(ProcessStatsDataSourceTest, WriteOnceProcess) {
 }
 
 TEST_F(ProcessStatsDataSourceTest, DontRescanCachedPIDsAndTIDs) {
-  auto data_source = GetProcessStatsDataSource(DataSourceConfig());
+  DataSourceConfig config;
+  config.mutable_process_stats_config()->set_record_thread_names(true);
+  auto data_source = GetProcessStatsDataSource(config);
   for (int p : {10, 11, 12, 20, 21, 22, 30, 31, 32}) {
     EXPECT_CALL(*data_source, ReadProcPidFile(p, "status"))
         .WillOnce(Invoke([](int32_t pid, const std::string&) {
diff --git a/src/tracing/core/process_stats_config.cc b/src/tracing/core/process_stats_config.cc
index fabf7d684..7266c21a2 100644
--- a/src/tracing/core/process_stats_config.cc
+++ b/src/tracing/core/process_stats_config.cc
@@ -49,6 +49,19 @@ void ProcessStatsConfig::FromProto(
                   "size mismatch");
     quirks_.back() = static_cast<decltype(quirks_)::value_type>(field);
   }
+
+  static_assert(sizeof(scan_all_processes_on_start_) ==
+                    sizeof(proto.scan_all_processes_on_start()),
+                "size mismatch");
+  scan_all_processes_on_start_ =
+      static_cast<decltype(scan_all_processes_on_start_)>(
+          proto.scan_all_processes_on_start());
+
+  static_assert(
+      sizeof(record_thread_names_) == sizeof(proto.record_thread_names()),
+      "size mismatch");
+  record_thread_names_ =
+      static_cast<decltype(record_thread_names_)>(proto.record_thread_names());
   unknown_fields_ = proto.unknown_fields();
 }
 
@@ -60,6 +73,20 @@ void ProcessStatsConfig::ToProto(
     proto->add_quirks(static_cast<decltype(proto->quirks(0))>(it));
     static_assert(sizeof(it) == sizeof(proto->quirks(0)), "size mismatch");
   }
+
+  static_assert(sizeof(scan_all_processes_on_start_) ==
+                    sizeof(proto->scan_all_processes_on_start()),
+                "size mismatch");
+  proto->set_scan_all_processes_on_start(
+      static_cast<decltype(proto->scan_all_processes_on_start())>(
+          scan_all_processes_on_start_));
+
+  static_assert(
+      sizeof(record_thread_names_) == sizeof(proto->record_thread_names()),
+      "size mismatch");
+  proto->set_record_thread_names(
+      static_cast<decltype(proto->record_thread_names())>(
+          record_thread_names_));
   *(proto->mutable_unknown_fields()) = unknown_fields_;
 }