Android systrace: Optimize memory dumps.

systrace/atrace_helper/jni/atrace_process_dump.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ATRACE_PROCESS_DUMP_H_
+#define ATRACE_PROCESS_DUMP_H_
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <set>
+#include <string>
+
+#include "process_info.h"
+
+// Utility to collect processes, thread names, per-process memory stats and
+// other minor metrics from /proc filesystem. It's aimed to extend systrace
+// with more actionable number to hit performance issues.
+class AtraceProcessDump {
+ public:
+  // Config is either:
+  // comma-separated list of process names.
+  // "apps" - all Android apps launched from Zygote.
+  // "all" - all non-kernel processes.
+  void SetFullDumpConfig(const std::string& config);
+  void EnableGraphicsStats() { graphics_stats_ = true; }
+  void EnablePrintSmaps() { print_smaps_ = true; }
+  void SetDumpCount(int count) { dump_count_ = count; }
+  void SetDumpInterval(int interval_ms) { dump_interval_ms_ = interval_ms; }
+
+  void RunAndPrintJson(FILE* stream);
+  void Stop() { CloseTimer(); }

[Primiano Tucci (use gerrit), 2017/06/26 10:07:20]

why this is inlined?
coding style says that only setters and trivial methods can be inlined, and in
that case they should be named lowercase(). This one calls an actual function,
so it looks like it should have its own definition in the .cc ifle

+
+ private:
+  void SerializeSnapshot(
+      const InstantProcessInfo* snapshot, const std::set<int>& smaps_pids);
+  void SerializeProcessInfo(
+      const PersistentProcessInfo::ProcessMap* processes);
+
+  void SetupTimer();
+  bool WaitForTimer();
+  void CloseTimer();
+
+  int dump_count_;
+  int dump_interval_ms_;
+
+  int timer_fd_;
+  FILE* out_;
+
+  int self_pid_;
+  bool graphics_stats_ = false;
+  bool print_smaps_ = false;
+  bool full_dump_all_ = false;

[Primiano Tucci (use gerrit), 2017/06/26 10:07:20]

it feels this should really be an enum: with 3 values {kOnlyWhitelistedProcs,
kAllProcesses, kAllJavaApps} or something similar

+  bool full_dump_apps_ = false;
+  std::set<std::string> full_dump_whitelist_;
+};
+
+#endif  // ATRACE_PROCESS_DUMP_H_