[tracing] Implement polling in MemoryDumpManager

base/trace_event/memory_dump_scheduler.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 BASE_TRACE_EVENT_MEMORY_DUMP_SCHEDULER_H
+#define BASE_TRACE_EVENT_MEMORY_DUMP_SCHEDULER_H
+
+#include "base/base_export.h"
+#include "base/gtest_prod_util.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;
+
+namespace trace_event {
+
+class MemoryDumpManager;
+
+// Schedules global dump requests based on the triggers added.
+class BASE_EXPORT MemoryDumpScheduler {
+ public:
+  MemoryDumpScheduler(
+      MemoryDumpManager* mdm_,
+      scoped_refptr<SingleThreadTaskRunner> polling_task_runner);
+  ~MemoryDumpScheduler();
+
+  // 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.
+  void AddTrigger(MemoryDumpType trigger_type,
+                  MemoryDumpLevelOfDetail level_of_detail,
+                  uint32_t min_time_between_dumps_ms);
+
+  // Starts periodic dumps.
+  void NotifyPeriodicTriggerSupported();
+
+  // Starts polling memory total.
+  void NotifyPollingSupported();
+
+  // Disables all triggers.
+  void DisableAllTriggers();
+
+ private:
+  friend class MemoryDumpManagerTest;
+  FRIEND_TEST_ALL_PREFIXES(MemoryDumpManagerTest, TestPollingOnDumpThread);
+
+  // Helper class to schdule periodic memory dumps.
+  class PeriodicDumpScheduler {
+   public:
+    explicit PeriodicDumpScheduler(MemoryDumpManager* mdm);
+    ~PeriodicDumpScheduler();
+
+    // Adds periodic dump time periods.
+    void AddTrigger(MemoryDumpLevelOfDetail level_of_detail,
+                    uint32_t min_time_between_dumps_ms);
+
+    // Starts periodic timer on notification.
+    void NotifySupported();
+
+    // Disables periodic timer.
+    void Disable();
+
+    bool IsTimerRunning() { return timer_.IsRunning(); }
+
+    // Periodically called by the timer.
+    void RequestPeriodicGlobalDump();
+
+    // True if periodic scheduler is enabled in this session.
+    bool is_setup() const { return is_setup_; }

[Primiano Tucci (use gerrit), 2017/02/03 20:08:20]

s/is_setup/has_triggers/ ? or (is_configured, to keep it consistent with the
other one)

[ssid, 2017/02/14 02:05:40]

Done.

+
+   private:
+    MemoryDumpManager* mdm_;
+
+    bool is_setup_;
+
+    // Timer for periodic dumps.
+    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 = 0;

[Primiano Tucci (use gerrit), 2017/02/03 20:08:20]

why these 2 are initialized here, but not the others (dump_count and friends) ?
can you just initialize everything in the ctor?

[ssid, 2017/02/14 02:05:40]

Sorry this was mistake.

+    uint32_t heavy_dump_period_ms = 0;
+
+    DISALLOW_COPY_AND_ASSIGN(PeriodicDumpScheduler);
+  };
+
+  class PeakDumpScheduler {

[Primiano Tucci (use gerrit), 2017/02/03 20:08:20]

honestly I am not fully sold you need two classes here. it seems you are
forwarding most of the methods, which makes the code look more complex than what
it is.
How about just having two structs, with the timing parametrs for polling and
triggers?
something like

class MemoryDumpScheduler {

private:
  struct PeakPollConfig {  
    int num_polls_....
    ..     
  };
  struct PeriodicCOnfig {
    int ....
  }
}

[ssid, 2017/02/14 02:05:40]

yes I agree this should be the case. I did aim for it initially then changed my
mind because there were too many references to the fields here. I had to use
"peak_poll_state_.light_dump_period_ms" at each access to this data. This just
felt too long and harder to read the implementation than going to one extra
function call because of the class.
Made it struct, slightly unhappy about too many reference to the members, but
doesn't look bad.

+   public:
+    PeakDumpScheduler(
+        MemoryDumpManager* mdm,
+        scoped_refptr<SingleThreadTaskRunner> polling_task_runner);
+    ~PeakDumpScheduler();
+
+    // Sets up peak dump triggers for global memory dumps according to
+    // the trigger.
+    void AddTrigger(MemoryDumpLevelOfDetail level_of_detail,
+                    uint32_t min_time_between_dumps_ms);
+
+    // Called for polling memory usage and trigger dumps if peak is detected.
+    void PollMemoryOnPollingThread();
+
+    // Starts polling for peak detection.
+    void NotifySupported();
+
+    // Disables polling for peak detection.
+    void Disable();
+
+    // True if peak dumps scheduler is enabled in this session.
+    bool is_setup() const { return is_setup_; }
+
+   private:
+    // Returns true if peak memory value is detected.
+    bool ShouldTriggerDump(uint64_t current_memory_total);
+
+    MemoryDumpManager* mdm_;
+
+    bool is_setup_;
+    MemoryDumpLevelOfDetail dump_mode_;
+
+    scoped_refptr<SingleThreadTaskRunner> polling_task_runner_;
+    uint32_t polling_interval_ = 0;
+
+    int min_polls_between_dumps_ = 0;
+    int num_polls_from_last_dump_ = 0;
+
+    // Threshold on increase in memory from last dump beyond which a new dump
+    // must be triggered.
+    int64_t memory_increase_threshold_ = 50 * 1024 * 1024;  // 50MiB
+    uint64_t last_dump_memory_total_ = 0;
+
+    DISALLOW_COPY_AND_ASSIGN(PeakDumpScheduler);
+  };
+
+  // Must be set before enabling tracing.
+  static void SetPollingIntervalForTesting(uint32_t interval);
+
+  bool IsPeriodicTimerRunningForTesting();
+
+  MemoryDumpManager* mdm_;
+
+  PeriodicDumpScheduler periodic_dump_scheduler_;
+  PeakDumpScheduler peak_dump_scheduler_;
+
+  DISALLOW_COPY_AND_ASSIGN(MemoryDumpScheduler);
+};
+
+}  // namespace trace_event
+}  // namespace base
+
+#endif  // BASE_TRACE_EVENT_MEMORY_DUMP_SCHEDULER_H