[tracing] Reland of Throttle rate of heavy dumps and support to reque…

…st light/heavy dumps

Reason for revert: telemetry test failure.

Fix: This CL adds the missing IPC macros for passing the struct which
caused the test failure.

Original message:

Currently all the dump providers dump detailed dumps every periodic
dump. This makes the trace files huge. So, this CL throttles down the
rate of heavy dumps of all the dump providers and removes temporary
hacks. Along with this, this CL also adds support to request light and
heavy dumps by adding MemoryDumpArgs as param to RequestGlobalDump()

TBR=pfeldman@chromium.org, nduca@chromium.org
BUG=499731

Committed: https://crrev.com/33f2d3a5b276b70040adfd21edf364bd666b0aa2
Cr-Commit-Position: refs/heads/master@{#342145}

patch from issue 1267963002 at patchset 80001 (http://crrev.com/1267963002#ps80001)

Review URL: https://codereview.chromium.org/1282453002

Cr-Commit-Position: refs/heads/master@{#342436}

base/trace_event/memory_dump_manager.cc

@@ -43,7 +43,7 @@ namespace {
 const char kTraceCategory[] = TRACE_DISABLED_BY_DEFAULT("memory-infra");
 
 // Throttle mmaps at a rate of once every kHeavyMmapsDumpsRate standard dumps.
-const int kHeavyMmapsDumpsRate = 8;  // 250 ms * 8 = 2000 ms.
+const int kHeavyDumpsRate = 8;  // 250 ms * 8 = 2000 ms.
 const int kDumpIntervalMs = 250;
 const int kTraceEventNumArgs = 1;
 const char* kTraceEventArgNames[] = {"dumps"};
@@ -52,16 +52,17 @@ const unsigned char kTraceEventArgTypes[] = {TRACE_VALUE_TYPE_CONVERTABLE};
 StaticAtomicSequenceNumber g_next_guid;
 uint32 g_periodic_dumps_count = 0;
 MemoryDumpManager* g_instance_for_testing = nullptr;
-MemoryDumpProvider* g_mmaps_dump_provider = nullptr;
 
 void RequestPeriodicGlobalDump() {
-  MemoryDumpType dump_type = g_periodic_dumps_count == 0
-                                 ? MemoryDumpType::PERIODIC_INTERVAL_WITH_MMAPS
-                                 : MemoryDumpType::PERIODIC_INTERVAL;
-  if (++g_periodic_dumps_count == kHeavyMmapsDumpsRate)
+  MemoryDumpArgs::LevelOfDetail dump_level_of_detail =
+      g_periodic_dumps_count == 0 ? MemoryDumpArgs::LevelOfDetail::HIGH
+                                  : MemoryDumpArgs::LevelOfDetail::LOW;
+  if (++g_periodic_dumps_count == kHeavyDumpsRate)
     g_periodic_dumps_count = 0;
 
-  MemoryDumpManager::GetInstance()->RequestGlobalDump(dump_type);
+  MemoryDumpArgs dump_args = {dump_level_of_detail};
+  MemoryDumpManager::GetInstance()->RequestGlobalDump(
+      MemoryDumpType::PERIODIC_INTERVAL, dump_args);
 }
 
 }  // namespace
@@ -118,8 +119,7 @@ void MemoryDumpManager::Initialize() {
 #endif
 
 #if defined(OS_LINUX) || defined(OS_ANDROID)
-  g_mmaps_dump_provider = ProcessMemoryMapsDumpProvider::GetInstance();
-  RegisterDumpProvider(g_mmaps_dump_provider);
+  RegisterDumpProvider(ProcessMemoryMapsDumpProvider::GetInstance());
   RegisterDumpProvider(MallocDumpProvider::GetInstance());
   system_allocator_pool_name_ = MallocDumpProvider::kAllocatedObjects;
 #endif
@@ -180,9 +180,9 @@ void MemoryDumpManager::UnregisterDumpProvider(MemoryDumpProvider* mdp) {
   did_unregister_dump_provider_ = true;
 }
 
-void MemoryDumpManager::RequestGlobalDump(
-    MemoryDumpType dump_type,
-    const MemoryDumpCallback& callback) {
+void MemoryDumpManager::RequestGlobalDump(MemoryDumpType dump_type,
+                                          const MemoryDumpArgs& dump_args,
+                                          const MemoryDumpCallback& callback) {
   // Bail out immediately if tracing is not enabled at all.
   if (!UNLIKELY(subtle::NoBarrier_Load(&memory_tracing_enabled_))) {
     if (!callback.is_null())
@@ -204,15 +204,16 @@ void MemoryDumpManager::RequestGlobalDump(
   if (delegate) {
     // The delegate is in charge to coordinate the request among all the
     // processes and call the CreateLocalDumpPoint on the local process.
-    MemoryDumpRequestArgs args = {guid, dump_type};
+    MemoryDumpRequestArgs args = {guid, dump_type, dump_args};
     delegate->RequestGlobalMemoryDump(args, callback);
   } else if (!callback.is_null()) {
     callback.Run(guid, false /* success */);
   }
 }
 
-void MemoryDumpManager::RequestGlobalDump(MemoryDumpType dump_type) {
-  RequestGlobalDump(dump_type, MemoryDumpCallback());
+void MemoryDumpManager::RequestGlobalDump(MemoryDumpType dump_type,
+                                          const MemoryDumpArgs& dump_args) {
+  RequestGlobalDump(dump_type, dump_args, MemoryDumpCallback());
 }
 
 void MemoryDumpManager::CreateProcessDump(const MemoryDumpRequestArgs& args,
@@ -273,15 +274,6 @@ void MemoryDumpManager::ContinueAsyncProcessDump(
     mdp = mdp_info->dump_provider;
     if (mdp_info->disabled) {
       skip_dump = true;
-    } else if (mdp == g_mmaps_dump_provider &&
-               pmd_async_state->req_args.dump_type !=
-                   MemoryDumpType::PERIODIC_INTERVAL_WITH_MMAPS &&
-               pmd_async_state->req_args.dump_type !=
-                   MemoryDumpType::EXPLICITLY_TRIGGERED) {
-      // Mmaps dumping is very heavyweight and cannot be performed at the same
-      // rate of other dumps. TODO(primiano): this is a hack and should be
-      // cleaned up as part of crbug.com/499731.
-      skip_dump = true;
     } else if (mdp_info->task_runner &&
                !mdp_info->task_runner->BelongsToCurrentThread()) {
       // It's time to hop onto another thread.
@@ -312,13 +304,9 @@ void MemoryDumpManager::ContinueAsyncProcessDump(
   bool finalize = false;
   bool dump_successful = false;
 
-  // TODO(ssid): Change RequestGlobalDump to use MemoryDumpArgs along with
-  // MemoryDumpType to get request for light / heavy dump, and remove this
-  // constant.
   if (!skip_dump) {
-    MemoryDumpArgs dump_args = {MemoryDumpArgs::LevelOfDetail::HIGH};
-    dump_successful =
-        mdp->OnMemoryDump(dump_args, &pmd_async_state->process_memory_dump);
+    dump_successful = mdp->OnMemoryDump(pmd_async_state->req_args.dump_args,
+                                        &pmd_async_state->process_memory_dump);
   }
 
   {

base/trace_event/memory_dump_manager.h

@@ -60,15 +60,18 @@ class BASE_EXPORT MemoryDumpManager : public TraceLog::EnabledStateObserver {
 
   // Requests a memory dump. The dump might happen or not depending on the
   // filters and categories specified when enabling tracing.
+  // The |dump_args| is used to specify the dump's level of detail.
   // The optional |callback| is executed asynchronously, on an arbitrary thread,
   // to notify about the completion of the global dump (i.e. after all the
   // processes have dumped) and its success (true iff all the dumps were
   // successful).
   void RequestGlobalDump(MemoryDumpType dump_type,
+                         const MemoryDumpArgs& dump_args,
                          const MemoryDumpCallback& callback);
 
   // Same as above (still asynchronous), but without callback.
-  void RequestGlobalDump(MemoryDumpType dump_type);
+  void RequestGlobalDump(MemoryDumpType dump_type,
+                         const MemoryDumpArgs& dump_args);
 
   // TraceLog::EnabledStateObserver implementation.
   void OnTraceLogEnabled() override;

base/trace_event/memory_dump_manager_unittest.cc

@@ -22,6 +22,10 @@ using testing::Return;
 
 namespace base {
 namespace trace_event {
+namespace {
+MemoryDumpArgs high_detail_args = {MemoryDumpArgs::LevelOfDetail::HIGH};
+MemoryDumpArgs low_detail_args = {MemoryDumpArgs::LevelOfDetail::LOW};
+}
 
 // Testing MemoryDumpManagerDelegate which short-circuits dump requests locally
 // instead of performing IPC dances.
@@ -90,12 +94,19 @@ class MockDumpProvider : public MemoryDumpProvider {
  public:
   MockDumpProvider()
       : dump_provider_to_register_or_unregister(nullptr),
-        last_session_state_(nullptr) {}
+        last_session_state_(nullptr),
+        level_of_detail_(MemoryDumpArgs::LevelOfDetail::HIGH) {}
 
   // Ctor used by the RespectTaskRunnerAffinity test.
   explicit MockDumpProvider(
       const scoped_refptr<SingleThreadTaskRunner>& task_runner)
-      : last_session_state_(nullptr), task_runner_(task_runner) {}
+      : last_session_state_(nullptr),
+        task_runner_(task_runner),
+        level_of_detail_(MemoryDumpArgs::LevelOfDetail::HIGH) {}
+
+  // Ctor used by CheckMemoryDumpArgs test.
+  explicit MockDumpProvider(const MemoryDumpArgs::LevelOfDetail level_of_detail)
+      : last_session_state_(nullptr), level_of_detail_(level_of_detail) {}
 
   virtual ~MockDumpProvider() {}
 
@@ -135,12 +146,20 @@ class MockDumpProvider : public MemoryDumpProvider {
     return true;
   }
 
+  // OnMemoryDump() override for the CheckMemoryDumpArgs test.
+  bool OnMemoryDump_CheckMemoryDumpArgs(const MemoryDumpArgs& args,
+                                        ProcessMemoryDump* pmd) {
+    EXPECT_EQ(level_of_detail_, args.level_of_detail);
+    return true;
+  }
+
   // Used by OnMemoryDump_(Un)RegisterExtraDumpProvider.
   MemoryDumpProvider* dump_provider_to_register_or_unregister;
 
  private:
   MemoryDumpSessionState* last_session_state_;
   scoped_refptr<SingleThreadTaskRunner> task_runner_;
+  const MemoryDumpArgs::LevelOfDetail level_of_detail_;
 };
 
 TEST_F(MemoryDumpManagerTest, SingleDumper) {
@@ -150,26 +169,63 @@ TEST_F(MemoryDumpManagerTest, SingleDumper) {
   // Check that the dumper is not called if the memory category is not enabled.
   EnableTracing("foo-and-bar-but-not-memory");
   EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(0);
-  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED);
+  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                          high_detail_args);
   DisableTracing();
 
   // Now repeat enabling the memory category and check that the dumper is
   // invoked this time.
   EnableTracing(kTraceCategory);
   EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(3).WillRepeatedly(Return(true));
   for (int i = 0; i < 3; ++i)
-    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED);
+    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                            high_detail_args);
   DisableTracing();
 
   mdm_->UnregisterDumpProvider(&mdp);
 
   // Finally check the unregister logic (no calls to the mdp after unregister).
   EnableTracing(kTraceCategory);
   EXPECT_CALL(mdp, OnMemoryDump(_, _)).Times(0);
-  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED);
+  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                          high_detail_args);
   TraceLog::GetInstance()->SetDisabled();
 }
 
+TEST_F(MemoryDumpManagerTest, CheckMemoryDumpArgs) {
+  // Check that requesting dumps with high level of detail actually propagates
+  // to OnMemoryDump() call on dump providers.
+  MockDumpProvider mdp_high_detail(MemoryDumpArgs::LevelOfDetail::HIGH);
+  mdm_->RegisterDumpProvider(&mdp_high_detail);
+
+  EnableTracing(kTraceCategory);
+  EXPECT_CALL(mdp_high_detail, OnMemoryDump(_, _))
+      .Times(1)
+      .WillRepeatedly(
+          Invoke(&mdp_high_detail,
+                 &MockDumpProvider::OnMemoryDump_CheckMemoryDumpArgs));
+  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                          high_detail_args);
+  DisableTracing();
+  mdm_->UnregisterDumpProvider(&mdp_high_detail);
+
+  // Check that requesting dumps with low level of detail actually propagates to
+  // OnMemoryDump() call on dump providers.
+  MockDumpProvider mdp_low_detail(MemoryDumpArgs::LevelOfDetail::LOW);
+  mdm_->RegisterDumpProvider(&mdp_low_detail);
+
+  EnableTracing(kTraceCategory);
+  EXPECT_CALL(mdp_low_detail, OnMemoryDump(_, _))
+      .Times(1)
+      .WillRepeatedly(
+          Invoke(&mdp_low_detail,
+                 &MockDumpProvider::OnMemoryDump_CheckMemoryDumpArgs));
+  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                          low_detail_args);
+  DisableTracing();
+  mdm_->UnregisterDumpProvider(&mdp_low_detail);
+}
+
 TEST_F(MemoryDumpManagerTest, SharedSessionState) {
   MockDumpProvider mdp1;
   MockDumpProvider mdp2;
@@ -187,7 +243,8 @@ TEST_F(MemoryDumpManagerTest, SharedSessionState) {
           Invoke(&mdp2, &MockDumpProvider::OnMemoryDump_CheckSessionState));
 
   for (int i = 0; i < 2; ++i)
-    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED);
+    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                            high_detail_args);
 
   DisableTracing();
 }
@@ -201,7 +258,8 @@ TEST_F(MemoryDumpManagerTest, MultipleDumpers) {
   EnableTracing(kTraceCategory);
   EXPECT_CALL(mdp1, OnMemoryDump(_, _)).Times(1).WillRepeatedly(Return(true));
   EXPECT_CALL(mdp2, OnMemoryDump(_, _)).Times(0);
-  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED);
+  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                          high_detail_args);
   DisableTracing();
 
   // Invert: enable mdp1 and disable mdp2.
@@ -210,15 +268,17 @@ TEST_F(MemoryDumpManagerTest, MultipleDumpers) {
   EnableTracing(kTraceCategory);
   EXPECT_CALL(mdp1, OnMemoryDump(_, _)).Times(0);
   EXPECT_CALL(mdp2, OnMemoryDump(_, _)).Times(1).WillRepeatedly(Return(true));
-  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED);
+  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                          high_detail_args);
   DisableTracing();
 
   // Enable both mdp1 and mdp2.
   mdm_->RegisterDumpProvider(&mdp1);
   EnableTracing(kTraceCategory);
   EXPECT_CALL(mdp1, OnMemoryDump(_, _)).Times(1).WillRepeatedly(Return(true));
   EXPECT_CALL(mdp2, OnMemoryDump(_, _)).Times(1).WillRepeatedly(Return(true));
-  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED);
+  mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                          high_detail_args);
   DisableTracing();
 }
 
@@ -256,7 +316,8 @@ TEST_F(MemoryDumpManagerTest, RespectTaskRunnerAffinity) {
       MemoryDumpCallback callback =
           Bind(&MemoryDumpManagerTest::DumpCallbackAdapter, Unretained(this),
                MessageLoop::current()->task_runner(), run_loop.QuitClosure());
-      mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED, callback);
+      mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                              high_detail_args, callback);
       // This nested message loop (|run_loop|) will be quit if and only if
       // the RequestGlobalDump callback is invoked.
       run_loop.Run();
@@ -303,7 +364,8 @@ TEST_F(MemoryDumpManagerTest, DisableFailingDumpers) {
       .WillRepeatedly(Return(true));
   for (int i = 0; i < 1 + MemoryDumpManager::kMaxConsecutiveFailuresCount;
        i++) {
-    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED);
+    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                            high_detail_args);
   }
 
   DisableTracing();
@@ -333,7 +395,8 @@ TEST_F(MemoryDumpManagerTest, RegisterDumperWhileDumping) {
       .WillRepeatedly(Return(true));
 
   for (int i = 0; i < 4; i++) {
-    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED);
+    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                            high_detail_args);
   }
 
   DisableTracing();
@@ -363,7 +426,8 @@ TEST_F(MemoryDumpManagerTest, UnregisterDumperWhileDumping) {
       .WillRepeatedly(Return(true));
 
   for (int i = 0; i < 4; i++) {
-    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED);
+    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                            high_detail_args);
   }
 
   DisableTracing();
@@ -383,7 +447,8 @@ TEST_F(MemoryDumpManagerTest, CallbackCalledOnFailure) {
     MemoryDumpCallback callback =
         Bind(&MemoryDumpManagerTest::DumpCallbackAdapter, Unretained(this),
              MessageLoop::current()->task_runner(), run_loop.QuitClosure());
-    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED, callback);
+    mdm_->RequestGlobalDump(MemoryDumpType::EXPLICITLY_TRIGGERED,
+                            high_detail_args, callback);
     run_loop.Run();
   }
   EXPECT_FALSE(last_callback_success_);

base/trace_event/memory_dump_provider.h

@@ -17,7 +17,11 @@ class ProcessMemoryDump;
 // should generate on dump request. This is to control the size of dumps
 // generated.
 struct MemoryDumpArgs {
-  enum class LevelOfDetail { LOW, HIGH };
+  enum class LevelOfDetail {
+    LOW,
+    HIGH,
+    LAST = HIGH  // For IPC Macros.
+  };
 
   LevelOfDetail level_of_detail;
 };

base/trace_event/memory_dump_request_args.cc

@@ -19,8 +19,6 @@ const char* MemoryDumpTypeToString(
       return "TASK_END";
     case MemoryDumpType::PERIODIC_INTERVAL:
       return "PERIODIC_INTERVAL";
-    case MemoryDumpType::PERIODIC_INTERVAL_WITH_MMAPS:
-      return "PERIODIC_INTERVAL_WITH_MMAPS";
     case MemoryDumpType::EXPLICITLY_TRIGGERED:
       return "EXPLICITLY_TRIGGERED";
   }

base/trace_event/memory_dump_request_args.h

@@ -10,6 +10,7 @@
 
 #include "base/base_export.h"
 #include "base/callback.h"
+#include "base/trace_event/memory_dump_provider.h"
 
 namespace base {
 namespace trace_event {
@@ -20,8 +21,6 @@ enum class MemoryDumpType {
   TASK_BEGIN,         // Dumping memory at the beginning of a message-loop task.
   TASK_END,           // Dumping memory at the ending of a message-loop task.
   PERIODIC_INTERVAL,  // Dumping memory at periodic intervals.
-  PERIODIC_INTERVAL_WITH_MMAPS,  // As above but w/ heavyweight mmaps dumps.
-                                 // Temporary workaround for crbug.com/499731.
   EXPLICITLY_TRIGGERED,  // Non maskable dump request.
   LAST = EXPLICITLY_TRIGGERED // For IPC macros.
 };
@@ -38,6 +37,8 @@ struct BASE_EXPORT MemoryDumpRequestArgs {
   uint64 dump_guid;
 
   MemoryDumpType dump_type;
+
+  MemoryDumpArgs dump_args;
 };
 
 }  // namespace trace_event