memory-infra: Switch to MemoryPeakDetector and simplify MemoryDumpScheduler

base/trace_event/memory_dump_scheduler_unittest.cc

Index: base/trace_event/memory_dump_scheduler_unittest.cc
diff --git a/base/trace_event/memory_dump_scheduler_unittest.cc b/base/trace_event/memory_dump_scheduler_unittest.cc
index 9af2a3b4306bf1b6ecf08b3cf66a417b61659ee5..a7049a1c3ffc71ce32000118ea9a76cee1387c64 100644
--- a/base/trace_event/memory_dump_scheduler_unittest.cc
+++ b/base/trace_event/memory_dump_scheduler_unittest.cc
@@ -6,96 +6,88 @@
 
 #include <memory>
 
+#include "base/bind.h"
 #include "base/single_thread_task_runner.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/threading/thread.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::testing::Invoke;
+using ::testing::_;
+
 namespace base {
 namespace trace_event {
 
-class MemoryDumpSchedulerPollingTest : public testing::Test {
+namespace {
+
+// Wrapper to use gmock on a callback.
+struct SchedulerCallbackWrapper {
+  MOCK_METHOD1(OnTick, void(MemoryDumpLevelOfDetail));
+};
+
+}  // namespace
+
+class MemoryDumpSchedulerTest : public testing::Test {
  public:
-  static const uint32_t kMinPollsToDump = 5;
+  struct FriendDeleter {
+    void operator()(MemoryDumpScheduler* inst) { delete inst; }
+  };
 
-  MemoryDumpSchedulerPollingTest()
-      : testing::Test(),
-        num_samples_tracked_(
-            MemoryDumpScheduler::PollingTriggerState::kMaxNumMemorySamples) {}
+  MemoryDumpSchedulerTest() : testing::Test() {}
 
   void SetUp() override {
-    MemoryDumpScheduler::SetPollingIntervalForTesting(1);
-    uint32_t kMinPollsToDump = 5;
-    mds_ = MemoryDumpScheduler::GetInstance();
-    mds_->Setup(nullptr, nullptr);
-    mds_->AddTrigger(MemoryDumpType::PEAK_MEMORY_USAGE,
-                     MemoryDumpLevelOfDetail::LIGHT, kMinPollsToDump);
-    mds_->polling_state_->ResetTotals();
-    mds_->polling_state_->current_state =
-        MemoryDumpScheduler::PollingTriggerState::ENABLED;
+    bg_thread_.reset(new Thread("MemoryDumpSchedulerTest Thread"));
+    bg_thread_->Start();
+    scheduler_.reset(new MemoryDumpScheduler());
   }
 
   void TearDown() override {
-    mds_->polling_state_->current_state =
-        MemoryDumpScheduler::PollingTriggerState::DISABLED;
+    bg_thread_.reset();
+    scheduler_.reset();
   }
 
  protected:
-  bool ShouldTriggerDump(uint64_t total) {
-    return mds_->ShouldTriggerDump(total);
-  }
-
-  uint32_t num_samples_tracked_;
-  MemoryDumpScheduler* mds_;
+  std::unique_ptr<MemoryDumpScheduler, FriendDeleter> scheduler_;
+  std::unique_ptr<Thread> bg_thread_;
+  SchedulerCallbackWrapper on_tick_callback_;
 };
 
-TEST_F(MemoryDumpSchedulerPollingTest, PeakDetection) {
-  for (uint32_t i = 0; i < num_samples_tracked_ * 6; ++i) {
-    // Memory is increased in steps and dumps must be triggered at every step.
-    uint64_t total = (2 + (i / (2 * num_samples_tracked_))) * 1024 * 1204;
-    bool did_trigger = ShouldTriggerDump(total);
-    // Dumps must be triggered only at specific iterations.
-    bool should_have_triggered = i == 0;
-    should_have_triggered |=
-        (i > num_samples_tracked_) && (i % (2 * num_samples_tracked_) == 1);
-    if (should_have_triggered) {
-      ASSERT_TRUE(did_trigger) << "Dump wasn't triggered at " << i;
-    } else {
-      ASSERT_FALSE(did_trigger) << "Unexpected dump at " << i;
-    }
-  }
-}
+TEST_F(MemoryDumpSchedulerTest, SingleTrigger) {

[ssid, 2017/04/05 22:04:22]

We should remove some tests from MDM maybe, which has too many tests related to
the timer.

[Primiano Tucci (use gerrit), 2017/04/10 19:16:41]

I simplified a bit the TestPollingOnDumpThread limint that to check that th
traceconfig works properly end to end. DIdn't find anything else too relevant
(but very lkely I missed something, open to suggestions)

[ssid, 2017/04/10 21:35:51]

You are right. They test trace config parsing.

+  const uint32_t kPeriodMs = 1;
+  const auto kLevelOfDetail = MemoryDumpLevelOfDetail::DETAILED;
+  const uint32_t kTicks = 10;
+  WaitableEvent evt(WaitableEvent::ResetPolicy::MANUAL,
+                    WaitableEvent::InitialState::NOT_SIGNALED);
+  MemoryDumpScheduler::Config config;
+  config.triggers.push_back({kLevelOfDetail, kPeriodMs});
+  config.callback =
+      Bind(&SchedulerCallbackWrapper::OnTick, Unretained(&on_tick_callback_));
 
-TEST_F(MemoryDumpSchedulerPollingTest, SlowGrowthDetection) {
-  for (uint32_t i = 0; i < 15; ++i) {
-    // Record 1GiB of increase in each call. Dumps are triggered with 1% w.r.t
-    // system's total memory.
-    uint64_t total = static_cast<uint64_t>(i + 1) * 1024 * 1024 * 1024;
-    bool did_trigger = ShouldTriggerDump(total);
-    bool should_have_triggered = i % kMinPollsToDump == 0;
-    if (should_have_triggered) {
-      ASSERT_TRUE(did_trigger) << "Dump wasn't triggered at " << i;
-    } else {
-      ASSERT_FALSE(did_trigger) << "Unexpected dump at " << i;
-    }
-  }
-}
+  uint32_t num_ticks = 0;
+  EXPECT_CALL(on_tick_callback_, OnTick(_))
+      .WillRepeatedly(Invoke([&num_ticks, &evt, kTicks, kLevelOfDetail](
+                                 MemoryDumpLevelOfDetail level_of_detail) {
+        EXPECT_EQ(kLevelOfDetail, level_of_detail);
+        if (++num_ticks == kTicks)
+          evt.Signal();
+      }));
+  const TimeTicks tstart = TimeTicks::Now();
 
-TEST_F(MemoryDumpSchedulerPollingTest, NotifyDumpTriggered) {
-  for (uint32_t i = 0; i < num_samples_tracked_ * 6; ++i) {
-    uint64_t total = (2 + (i / (2 * num_samples_tracked_))) * 1024 * 1204;
-    if (i % num_samples_tracked_ == 0)
-      mds_->NotifyDumpTriggered();
-    bool did_trigger = ShouldTriggerDump(total);
-    // Dumps should never be triggered since NotifyDumpTriggered() is called
-    // frequently.
-    EXPECT_NE(0u, mds_->polling_state_->last_dump_memory_total);
-    EXPECT_GT(num_samples_tracked_ - 1,
-              mds_->polling_state_->last_memory_totals_kb_index);
-    EXPECT_LT(static_cast<int64_t>(
-                  total - mds_->polling_state_->last_dump_memory_total),
-              mds_->polling_state_->memory_increase_threshold);
-    ASSERT_FALSE(did_trigger && i) << "Unexpected dump at " << i;
-  }
+  // Check that Stop() before Start() doesn't cause any error.
+  scheduler_->Stop();
+
+  scheduler_->Start(config, bg_thread_->task_runner());
+  evt.Wait();
+  const double time_ms = (TimeTicks::Now() - tstart).InMillisecondsF();
+  EXPECT_GE(time_ms, kPeriodMs * kTicks);
+
+  // Check that stopping twice doesn't cause any problems.
+  scheduler_->Stop();
+  scheduler_->Stop();
 }
 
+// TODO add other tests to cover StartStopQuickly and multiple triggers.
+
 }  // namespace trace_event
 }  // namespace base