memory-infra: Switch to MemoryPeakDetector and simplify MemoryDumpScheduler

base/trace_event/memory_dump_scheduler.h

Index: base/trace_event/memory_dump_scheduler.h
diff --git a/base/trace_event/memory_dump_scheduler.h b/base/trace_event/memory_dump_scheduler.h
index ab8441bc20031242e5ac5df07f37eac12f66dc23..215dfc020509f581347b244380153333245dc0d2 100644
--- a/base/trace_event/memory_dump_scheduler.h
+++ b/base/trace_event/memory_dump_scheduler.h
@@ -5,154 +5,67 @@
 #ifndef BASE_TRACE_EVENT_MEMORY_DUMP_SCHEDULER_H
 #define BASE_TRACE_EVENT_MEMORY_DUMP_SCHEDULER_H
 
-#include <memory>
+#include <stdint.h>
+
+#include <vector>
 
 #include "base/base_export.h"
-#include "base/gtest_prod_util.h"
+#include "base/callback.h"
 #include "base/memory/ref_counted.h"
-#include "base/timer/timer.h"
 #include "base/trace_event/memory_dump_request_args.h"
 
 namespace base {
-class SingleThreadTaskRunner;
+class SequencedTaskRunner;
 
 namespace trace_event {
 
-class MemoryDumpManager;
-
 // Schedules global dump requests based on the triggers added. The methods of
 // this class are NOT thread safe and the client has to take care of invoking
 // all the methods of the class safely.
 class BASE_EXPORT MemoryDumpScheduler {
  public:
-  static MemoryDumpScheduler* GetInstance();
-
-  // Initializes the scheduler. NOT thread safe.
-  void Setup(MemoryDumpManager* mdm_,
-             scoped_refptr<SingleThreadTaskRunner> polling_task_runner);
-
-  // Adds triggers for scheduling global dumps. Both periodic and peak triggers
-  // cannot be added together. At the moment the periodic support is limited to
-  // at most one periodic trigger per dump mode and peak triggers are limited to
-  // at most one. All intervals should be an integeral multiple of the smallest
-  // interval specified. NOT thread safe.
-  void AddTrigger(MemoryDumpType trigger_type,
-                  MemoryDumpLevelOfDetail level_of_detail,
-                  uint32_t min_time_between_dumps_ms);
-
-  // Starts periodic dumps. NOT thread safe and triggers must be added before
-  // enabling.
-  void EnablePeriodicTriggerIfNeeded();
-
-  // Starts polling memory total. NOT thread safe and triggers must be added
-  // before enabling.
-  void EnablePollingIfNeeded();
-
-  // Resets time for triggering dump to account for minimum time between the
-  // dumps. NOT thread safe.
-  void NotifyDumpTriggered();
-
-  // Disables all triggers. NOT thread safe. This should be called before
-  // polling thread is stopped to stop polling cleanly.
-  void DisableAllTriggers();
-
- private:
-  friend class MemoryDumpManagerTest;
-  friend class MemoryDumpSchedulerPollingTest;
-  FRIEND_TEST_ALL_PREFIXES(MemoryDumpManagerTest, TestPollingOnDumpThread);
-  FRIEND_TEST_ALL_PREFIXES(MemoryDumpSchedulerPollingTest, NotifyDumpTriggered);
-
-  // Helper class to schdule periodic memory dumps.
-  struct BASE_EXPORT PeriodicTriggerState {
-    PeriodicTriggerState();
-    ~PeriodicTriggerState();
-
-    bool is_configured;
-
-    RepeatingTimer timer;
-    uint32_t dump_count;
-    uint32_t min_timer_period_ms;
-    uint32_t light_dumps_rate;
-    uint32_t heavy_dumps_rate;
-
-    uint32_t light_dump_period_ms;
-    uint32_t heavy_dump_period_ms;
-
-    DISALLOW_COPY_AND_ASSIGN(PeriodicTriggerState);
-  };
+  using PeriodicCallback = RepeatingCallback<void(MemoryDumpLevelOfDetail)>;
 
-  struct BASE_EXPORT PollingTriggerState {
-    enum State {
-      CONFIGURED,  // Polling trigger was added.
-      ENABLED,     // Polling is running.
-      DISABLED     // Polling is disabled.
+  // Passed to Start().
+  struct Config {
+    struct Trigger {
+      MemoryDumpLevelOfDetail level_of_detail;
+      uint32_t period_ms;
     };
 
-    static const uint32_t kMaxNumMemorySamples = 50;
+    Config();
+    Config(const Config&);
+    ~Config();
 
-    PollingTriggerState();
-    ~PollingTriggerState();
-
-    // Helper to clear the tracked memory totals and poll count from last dump.
-    void ResetTotals();
-
-    State current_state;
-    MemoryDumpLevelOfDetail level_of_detail;
-
-    uint32_t polling_interval_ms;
-
-    // Minimum numer of polls after the last dump at which next dump can be
-    // triggered.
-    int min_polls_between_dumps;
-    int num_polls_from_last_dump;
+    std::vector<Trigger> triggers;
+    PeriodicCallback callback;
+  };
 
-    uint64_t last_dump_memory_total;
-    int64_t memory_increase_threshold;
-    uint64_t last_memory_totals_kb[kMaxNumMemorySamples];
-    uint32_t last_memory_totals_kb_index;
+  static MemoryDumpScheduler* GetInstance();
 
-    DISALLOW_COPY_AND_ASSIGN(PollingTriggerState);
-  };
+  void Start(Config, scoped_refptr<SequencedTaskRunner> task_runner);
+  void Stop();
+  bool is_enabled_for_testing() const { return task_runner_ ? true : false; }

[hjd, 2017/04/11 10:02:30]

task_runner_ ? true : false;
is this the idiom? Naively bool(task_runner_) seems cleaner if it does the same
thing.

[Primiano Tucci (use gerrit), 2017/04/11 11:43:08]

Ah I thought it wouldn't have triggered the bool cast operator instead it does.
C++.

 
+ private:
+  friend class MemoryDumpSchedulerTest;
   MemoryDumpScheduler();
   ~MemoryDumpScheduler();
 
-  // Helper to set polling disabled.
-  void DisablePollingOnPollingThread();
-
-  // Periodically called by the timer.
-  void RequestPeriodicGlobalDump();
-
-  // Called for polling memory usage and trigger dumps if peak is detected.
-  void PollMemoryOnPollingThread();
-
-  // Returns true if peak memory value is detected.
-  bool ShouldTriggerDump(uint64_t current_memory_total);
-
-  // Helper to detect peaks in memory usage.
-  bool IsCurrentSamplePeak(uint64_t current_memory_total);
-
-  // Must be set before enabling tracing.
-  static void SetPollingIntervalForTesting(uint32_t interval);
-
-  // True if periodic dumping is enabled.
-  bool IsPeriodicTimerRunningForTesting();
-
-  MemoryDumpManager* mdm_;
-
-  // Accessed on the thread of the client before enabling and only accessed on
-  // the thread that called "EnablePeriodicTriggersIfNeeded()" after enabling.
-  std::unique_ptr<PeriodicTriggerState> periodic_state_;
-
-  // Accessed on the thread of the client before enabling and only accessed on
-  // the polling thread after enabling.
-  std::unique_ptr<PollingTriggerState> polling_state_;
+  void StartInternal(Config);
+  void StopInternal();
+  void Tick(uint32_t expected_generation);
 
-  // Accessed on the thread of the client only.
-  scoped_refptr<SingleThreadTaskRunner> polling_task_runner_;
+  // Accessed only by the public methods (never from the task runner itself).
+  scoped_refptr<SequencedTaskRunner> task_runner_;
 
-  // True when the scheduler is setup. Accessed on the thread of client only.
-  bool is_setup_;
+  // These fields instead are only accessed from within the task runner.
+  uint32_t period_ms_;   // 0 == disabled.
+  uint32_t generation_;  // Used to invalidate outstanding tasks after Stop().
+  uint32_t tick_count_;
+  uint32_t light_dump_rate_;
+  uint32_t heavy_dump_rate_;
+  PeriodicCallback callback_;
 
   DISALLOW_COPY_AND_ASSIGN(MemoryDumpScheduler);
 };