memory-infra: Switch to MemoryPeakDetector and simplify MemoryDumpScheduler

base/trace_event/memory_dump_manager_unittest.cc

 // Copyright 2015 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.
 
 #include "base/trace_event/memory_dump_manager.h"
 
 #include <stdint.h>
 
 #include <memory>
 #include <utility>
@@ skipping 226 matching lines @@
   // Turns a Closure into a MemoryDumpCallback, keeping track of the callback
   // result and taking care of posting the closure on the correct task runner.
   void DumpCallbackAdapter(scoped_refptr<SingleThreadTaskRunner> task_runner,
                            Closure closure,
                            uint64_t dump_guid,
                            bool success) {
     last_callback_success_ = success;
     task_runner->PostTask(FROM_HERE, closure);
   }
 
-  void PollFastMemoryTotal(uint64_t* memory_total) {
-    mdm_->PollFastMemoryTotal(memory_total);
-  }
-
  protected:
   void InitializeMemoryDumpManager(bool is_coordinator) {
     mdm_->set_dumper_registrations_ignored_for_testing(true);
     delegate_ = new MemoryDumpManagerDelegateForTesting(is_coordinator);
     mdm_->Initialize(base::WrapUnique(delegate_));
   }
 
   void RequestGlobalDumpAndWait(MemoryDumpType dump_type,
                                 MemoryDumpLevelOfDetail level_of_detail) {
     RunLoop run_loop;
@@ skipping 10 matching lines @@
   }
 
   void EnableTracingWithTraceConfig(const std::string& trace_config) {
     TraceLog::GetInstance()->SetEnabled(TraceConfig(trace_config),
                                         TraceLog::RECORDING_MODE);
   }
 
   void DisableTracing() { TraceLog::GetInstance()->SetDisabled(); }
 
   bool IsPeriodicDumpingEnabled() const {
-    return MemoryDumpScheduler::GetInstance()
-        ->IsPeriodicTimerRunningForTesting();
+    return MemoryDumpScheduler::GetInstance()->is_enabled_for_testing();
   }
 
   int GetMaxConsecutiveFailuresCount() const {
     return MemoryDumpManager::kMaxConsecutiveFailuresCount;
   }
 
   const MemoryDumpProvider::Options kDefaultOptions;
   std::unique_ptr<MemoryDumpManager> mdm_;
   MemoryDumpManagerDelegateForTesting* delegate_;
   bool last_callback_success_;
@@ skipping 466 matching lines @@
 
   DisableTracing();
 }
 
 TEST_F(MemoryDumpManagerTest, TestPollingOnDumpThread) {
   InitializeMemoryDumpManager(false /* is_coordinator */);
   std::unique_ptr<MockMemoryDumpProvider> mdp1(new MockMemoryDumpProvider());
   std::unique_ptr<MockMemoryDumpProvider> mdp2(new MockMemoryDumpProvider());
   mdp1->enable_mock_destructor = true;
   mdp2->enable_mock_destructor = true;
-
-  EXPECT_CALL(*mdp1, SuspendFastMemoryPolling()).Times(1);
-  EXPECT_CALL(*mdp2, SuspendFastMemoryPolling()).Times(1);
   EXPECT_CALL(*mdp1, Destructor());
   EXPECT_CALL(*mdp2, Destructor());
 
   MemoryDumpProvider::Options options;
   options.is_fast_polling_supported = true;
   RegisterDumpProvider(mdp1.get(), nullptr, options);
 
   RunLoop run_loop;
-  scoped_refptr<SingleThreadTaskRunner> test_task_runner =
-      ThreadTaskRunnerHandle::Get();
+  auto test_task_runner = ThreadTaskRunnerHandle::Get();
   auto quit_closure = run_loop.QuitClosure();
+  MemoryDumpManager* mdm = mdm_.get();
 
-  const int kPollsToQuit = 10;
-  int call_count = 0;
-  MemoryDumpManager* mdm = mdm_.get();
-  const auto poll_function1 = [&call_count, &test_task_runner, quit_closure,
-                               &mdp2, mdm, &options, kPollsToQuit,
-                               this](uint64_t* total) -> void {
-    ++call_count;
-    if (call_count == 1)
-      RegisterDumpProvider(mdp2.get(), nullptr, options, kMDPName);
-    else if (call_count == 4)
-      mdm->UnregisterAndDeleteDumpProviderSoon(std::move(mdp2));
-    else if (call_count == kPollsToQuit)
-      test_task_runner->PostTask(FROM_HERE, quit_closure);
-
-    // Record increase of 1 GiB of memory at each call.
-    *total = static_cast<uint64_t>(call_count) * 1024 * 1024 * 1024;
-  };
   EXPECT_CALL(*mdp1, PollFastMemoryTotal(_))
-      .Times(testing::AtLeast(kPollsToQuit))
-      .WillRepeatedly(Invoke(poll_function1));
-
-  // Depending on the order of PostTask calls the mdp2 might be registered after
-  // all polls or in between polls.
-  EXPECT_CALL(*mdp2, PollFastMemoryTotal(_))
-      .Times(Between(0, kPollsToQuit - 1))
+      .WillOnce(Invoke([&mdp2, options, this](uint64_t*) {
+        RegisterDumpProvider(mdp2.get(), nullptr, options);
+      }))
+      .WillOnce(Return())
+      .WillOnce(Invoke([mdm, &mdp2](uint64_t*) {
+        mdm->UnregisterAndDeleteDumpProviderSoon(std::move(mdp2));
+      }))
+      .WillOnce(Invoke([test_task_runner, quit_closure](uint64_t*) {
+        test_task_runner->PostTask(FROM_HERE, quit_closure);
+      }))
       .WillRepeatedly(Return());
 
-  MemoryDumpScheduler::SetPollingIntervalForTesting(1);
+  EXPECT_CALL(*mdp1, SuspendFastMemoryPolling()).Times(1);

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

Why not 

EXPECT_CALL(*mdp2, SuspendFastMemoryPolling()).Times(1);

Actually just realized ~ProcessMetricsMemoryDumpProvider() does not close the
fast_polling_statm_fd_

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

Because I realized that there is no point calling SuspendFastMemoryPolling() at
unregistration, as the MDP will just be destroyed, and the FD will be closed in
its dtor. So I kept it only for the stop case.

I made the comment and the code more explicit though.
Unless i a missing something major, It does.
primiano-from-the-past was quite parnoid and asked you to use a base::ScopedFD
fast_polling_statm_fd_ :)
it should be auto closed.

[ssid, 2017/04/11 17:47:30]

Haha true!

+
+  // |mdp2| should invoke exactly twice:
+  // - once after the registrarion, when |mdp1| hits the first Return()
+  // - the 2nd time when |mdp1| unregisters |mdp1|. The unregistration is
+  //   posted and will necessarily happen after the polling task.
+  EXPECT_CALL(*mdp2, PollFastMemoryTotal(_)).Times(2).WillRepeatedly(Return());
+
   EnableTracingWithTraceConfig(
-      TraceConfigMemoryTestUtil::GetTraceConfig_PeakDetectionTrigger(3));
-
-  int last_poll_to_request_dump = -2;
-  EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _))
-      .Times(testing::AtLeast(2))
-      .WillRepeatedly(Invoke([&last_poll_to_request_dump, &call_count](
-                                 const MemoryDumpRequestArgs& args,
-                                 const MemoryDumpCallback& callback) -> void {
-        // Minimum number of polls between dumps must be 3 (polling interval is
-        // 1ms).
-        EXPECT_GE(call_count - last_poll_to_request_dump, 3);
-        last_poll_to_request_dump = call_count;
-      }));
-
+      TraceConfigMemoryTestUtil::GetTraceConfig_PeakDetectionTrigger(1));
   run_loop.Run();
   DisableTracing();
   mdm_->UnregisterAndDeleteDumpProviderSoon(std::move(mdp1));
 }
 
 // If a thread (with a dump provider living on it) is torn down during a dump
 // its dump provider should be skipped but the dump itself should succeed.
 TEST_F(MemoryDumpManagerTest, TearDownThreadWhileDumping) {
   InitializeMemoryDumpManager(false /* is_coordinator */);
   std::vector<std::unique_ptr<TestIOThread>> threads;
@@ skipping 145 matching lines @@
   // only explicitly triggered dumps.
   EnableTracingWithTraceConfig(
       TraceConfigMemoryTestUtil::GetTraceConfig_EmptyTriggers());
   EXPECT_FALSE(IsPeriodicDumpingEnabled());
   DisableTracing();
 
   // Enabling memory-infra with the new (JSON) TraceConfig in a coordinator
   // process with a fully defined trigger config should cause periodic dumps to
   // be performed in the correct order.
   RunLoop run_loop;
+  auto test_task_runner = ThreadTaskRunnerHandle::Get();
   auto quit_closure = run_loop.QuitClosure();
 
   const int kHeavyDumpRate = 5;
   const int kLightDumpPeriodMs = 1;
   const int kHeavyDumpPeriodMs = kHeavyDumpRate * kLightDumpPeriodMs;
   // The expected sequence with light=1ms, heavy=5ms is H,L,L,L,L,H,...
   testing::InSequence sequence;
   EXPECT_CALL(delegate, RequestGlobalMemoryDump(IsDetailedDump(), _));
   EXPECT_CALL(delegate, RequestGlobalMemoryDump(IsLightDump(), _))
       .Times(kHeavyDumpRate - 1);
   EXPECT_CALL(delegate, RequestGlobalMemoryDump(IsDetailedDump(), _));
   EXPECT_CALL(delegate, RequestGlobalMemoryDump(IsLightDump(), _))
       .Times(kHeavyDumpRate - 2);
   EXPECT_CALL(delegate, RequestGlobalMemoryDump(IsLightDump(), _))
-      .WillOnce(Invoke([quit_closure](const MemoryDumpRequestArgs& args,
-                                      const MemoryDumpCallback& callback) {
-        ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, quit_closure);
-      }));
+      .WillOnce(Invoke(
+          [test_task_runner, quit_closure](const MemoryDumpRequestArgs& args,
+                                           const MemoryDumpCallback& callback) {
+            test_task_runner->PostTask(FROM_HERE, quit_closure);
+          }));
 
   // Swallow all the final spurious calls until tracing gets disabled.
   EXPECT_CALL(delegate, RequestGlobalMemoryDump(_, _)).Times(AnyNumber());
 
   EnableTracingWithTraceConfig(
       TraceConfigMemoryTestUtil::GetTraceConfig_PeriodicTriggers(
           kLightDumpPeriodMs, kHeavyDumpPeriodMs));
   run_loop.Run();
   DisableTracing();
 }
@@ skipping 232 matching lines @@
   EXPECT_FALSE(IsPeriodicDumpingEnabled());
   RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
                            MemoryDumpLevelOfDetail::BACKGROUND);
   DisableTracing();
 }
 
 TEST_F(MemoryDumpManagerTest, TestBackgroundTracingSetup) {
   InitializeMemoryDumpManager(true /* is_coordinator */);
 
   RunLoop run_loop;
+  auto test_task_runner = ThreadTaskRunnerHandle::Get();
   auto quit_closure = run_loop.QuitClosure();
 
   testing::InSequence sequence;
   EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(IsBackgroundDump(), _))
       .Times(5);
   EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(IsBackgroundDump(), _))
-      .WillOnce(Invoke([quit_closure](const MemoryDumpRequestArgs& args,
-                                      const MemoryDumpCallback& callback) {
-        ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, quit_closure);
-      }));
+      .WillOnce(Invoke(
+          [test_task_runner, quit_closure](const MemoryDumpRequestArgs& args,
+                                           const MemoryDumpCallback& callback) {
+            test_task_runner->PostTask(FROM_HERE, quit_closure);
+          }));
   EXPECT_CALL(*delegate_, RequestGlobalMemoryDump(_, _)).Times(AnyNumber());
 
   EnableTracingWithTraceConfig(
       TraceConfigMemoryTestUtil::GetTraceConfig_BackgroundTrigger(
           1 /* period_ms */));
 
   // Only background mode dumps should be allowed with the trace config.
   last_callback_success_ = false;
   RequestGlobalDumpAndWait(MemoryDumpType::EXPLICITLY_TRIGGERED,
                            MemoryDumpLevelOfDetail::LIGHT);
@@ skipping 20 matching lines @@
   thread.Start();
   RegisterDumpProvider(&mdp1, thread.task_runner(), kDefaultOptions,
                        "BlacklistTestDumpProvider");
   // Unregistering on wrong thread should not crash.
   mdm_->UnregisterDumpProvider(&mdp1);
   thread.Stop();
 }
 
 }  // namespace trace_event
 }  // namespace base