Add RequestPrivateMemoryFootprint method to MemoryInstrumentation.

Most clients only need private memory footprint, but the existing methods would
still query all the MemoryDumpProviders and then filter the results. This CL
adds a new method that avoids querying MemoryDumpProviders.

Bug: 851712
Change-Id: I269be2b5a74c32205ba6f4087a9bb8affb9cffb6
Reviewed-on: https://chromium-review.googlesource.com/1098623Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567417}

chrome/browser/memory_details.cc

@@ -360,9 +360,10 @@ void MemoryDetails::CollectChildInfoOnUIThread() {
   // Using AdaptCallbackForRepeating allows for an easier transition to
   // OnceCallbacks for https://crbug.com/714018.
   memory_instrumentation::MemoryInstrumentation::GetInstance()
-      ->RequestGlobalDump(std::vector<std::string>(),
-                          base::AdaptCallbackForRepeating(base::BindOnce(
-                              &MemoryDetails::DidReceiveMemoryDump, this)));
+      ->RequestPrivateMemoryFootprint(
+          base::kNullProcessId,
+          base::AdaptCallbackForRepeating(
+              base::BindOnce(&MemoryDetails::DidReceiveMemoryDump, this)));
 }
 
 void MemoryDetails::DidReceiveMemoryDump(

chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc

@@ -586,7 +586,7 @@ void DataReductionProxyMetricsObserver::RequestProcessDump(
     memory_instrumentation::MemoryInstrumentation::RequestGlobalDumpCallback
         callback) {
   memory_instrumentation::MemoryInstrumentation::GetInstance()
-      ->RequestGlobalDumpForPid(pid, callback);
+      ->RequestPrivateMemoryFootprint(pid, callback);
 }
 
 }  // namespace data_reduction_proxy

chrome/browser/prerender/prerender_contents.cc

@@ -655,10 +655,10 @@ void PrerenderContents::DestroyWhenUsingTooManyResources() {
   // Using AdaptCallbackForRepeating allows for an easier transition to
   // OnceCallbacks for https://crbug.com/714018.
   memory_instrumentation::MemoryInstrumentation::GetInstance()
-      ->RequestGlobalDumpForPid(process_pid_,
-                                base::AdaptCallbackForRepeating(base::BindOnce(
-                                    &PrerenderContents::DidGetMemoryUsage,
-                                    weak_factory_.GetWeakPtr())));
+      ->RequestPrivateMemoryFootprint(
+          process_pid_, base::AdaptCallbackForRepeating(base::BindOnce(
+                            &PrerenderContents::DidGetMemoryUsage,
+                            weak_factory_.GetWeakPtr())));
 }
 
 void PrerenderContents::DidGetMemoryUsage(

chrome/browser/profiling_host/background_profiling_triggers.cc

@@ -122,7 +122,8 @@ void BackgroundProfilingTriggers::PerformMemoryUsageChecks() {
       [](base::WeakPtr<BackgroundProfilingTriggers> weak_ptr,
          std::vector<base::ProcessId> result) {
         memory_instrumentation::MemoryInstrumentation::GetInstance()
-            ->RequestGlobalDump(
+            ->RequestPrivateMemoryFootprint(
+                base::kNullProcessId,
                 base::Bind(&BackgroundProfilingTriggers::OnReceivedMemoryDump,
                            std::move(weak_ptr), std::move(result)));
       },

chrome/browser/resource_coordinator/render_process_probe.cc

@@ -140,9 +140,11 @@ void RenderProcessProbeImpl::
 
   // Dispatch the memory collection request.
   memory_instrumentation::MemoryInstrumentation::GetInstance()
-      ->RequestGlobalDump(base::BindRepeating(
-          &RenderProcessProbeImpl::ProcessGlobalMemoryDumpAndDispatchOnIOThread,
-          base::Unretained(this), collection_start_time));
+      ->RequestPrivateMemoryFootprint(
+          base::kNullProcessId,
+          base::BindRepeating(&RenderProcessProbeImpl::
+                                  ProcessGlobalMemoryDumpAndDispatchOnIOThread,
+                              base::Unretained(this), collection_start_time));
 
   RenderProcessInfoMap::iterator iter = render_process_info_map_.begin();
   while (iter != render_process_info_map_.end()) {

chrome/browser/task_manager/sampling/task_manager_impl.cc

@@ -586,7 +586,8 @@ void TaskManagerImpl::Refresh() {
     auto callback = base::Bind(&TaskManagerImpl::OnReceivedMemoryDump,
                                weak_ptr_factory_.GetWeakPtr());
     memory_instrumentation::MemoryInstrumentation::GetInstance()
-        ->RequestGlobalDump(std::move(callback));
+        ->RequestPrivateMemoryFootprint(base::kNullProcessId,
+                                        std::move(callback));
   }
 
   if (base::FeatureList::IsEnabled(network::features::kNetworkService) &&

content/browser/tracing/memory_instrumentation_browsertest.cc

@@ -53,15 +53,16 @@ std::unique_ptr<GlobalMemoryDump> DoGlobalDump() {
   std::unique_ptr<GlobalMemoryDump> result = nullptr;
   base::RunLoop run_loop;
   memory_instrumentation::MemoryInstrumentation::GetInstance()
-      ->RequestGlobalDump(base::Bind(
-          [](base::Closure quit_closure,
-             std::unique_ptr<GlobalMemoryDump>* out_result, bool success,
-             std::unique_ptr<GlobalMemoryDump> result) {
-            EXPECT_TRUE(success);
-            *out_result = std::move(result);
-            std::move(quit_closure).Run();
-          },
-          run_loop.QuitClosure(), &result));
+      ->RequestGlobalDump(
+          {}, base::Bind(
+                  [](base::Closure quit_closure,
+                     std::unique_ptr<GlobalMemoryDump>* out_result,
+                     bool success, std::unique_ptr<GlobalMemoryDump> result) {
+                    EXPECT_TRUE(success);
+                    *out_result = std::move(result);
+                    std::move(quit_closure).Run();
+                  },
+                  run_loop.QuitClosure(), &result));
   run_loop.Run();
   return result;
 }

services/resource_coordinator/memory_instrumentation/coordinator_impl.cc

@@ -120,7 +120,8 @@ void CoordinatorImpl::RequestGlobalMemoryDump(
   };
 
   QueuedRequest::Args args(dump_type, level_of_detail, allocator_dump_names,
-                           false /* add_to_trace */, base::kNullProcessId);
+                           false /* add_to_trace */, base::kNullProcessId,
+                           /*memory_footprint_only=*/false);
   RequestGlobalMemoryDumpInternal(args,
                                   base::BindOnce(adapter, std::move(callback)));
 }
@@ -147,7 +148,27 @@ void CoordinatorImpl::RequestGlobalMemoryDumpForPid(
   QueuedRequest::Args args(
       base::trace_event::MemoryDumpType::SUMMARY_ONLY,
       base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND,
-      allocator_dump_names, false /* add_to_trace */, pid);
+      allocator_dump_names, false /* add_to_trace */, pid,
+      /*memory_footprint_only=*/false);
+  RequestGlobalMemoryDumpInternal(args,
+                                  base::BindOnce(adapter, std::move(callback)));
+}
+
+void CoordinatorImpl::RequestPrivateMemoryFootprint(
+    base::ProcessId pid,
+    RequestPrivateMemoryFootprintCallback callback) {
+  // This merely strips out the |dump_guid| argument; this is not relevant
+  // as we are not adding to trace.
+  auto adapter = [](RequestPrivateMemoryFootprintCallback callback,
+                    bool success, uint64_t,
+                    mojom::GlobalMemoryDumpPtr global_memory_dump) {
+    std::move(callback).Run(success, std::move(global_memory_dump));
+  };
+
+  QueuedRequest::Args args(
+      base::trace_event::MemoryDumpType::SUMMARY_ONLY,
+      base::trace_event::MemoryDumpLevelOfDetail::BACKGROUND, {},
+      false /* add_to_trace */, pid, /*memory_footprint_only=*/true);
   RequestGlobalMemoryDumpInternal(args,
                                   base::BindOnce(adapter, std::move(callback)));
 }
@@ -164,7 +185,8 @@ void CoordinatorImpl::RequestGlobalMemoryDumpAndAppendToTrace(
   };
 
   QueuedRequest::Args args(dump_type, level_of_detail, {},
-                           true /* add_to_trace */, base::kNullProcessId);
+                           true /* add_to_trace */, base::kNullProcessId,
+                           /*memory_footprint_only=*/false);
   RequestGlobalMemoryDumpInternal(args,
                                   base::BindOnce(adapter, std::move(callback)));
 }

services/resource_coordinator/memory_instrumentation/coordinator_impl.h

@@ -67,6 +67,9 @@ class CoordinatorImpl : public Coordinator,
       base::ProcessId,
       const std::vector<std::string>& allocator_dump_names,
       RequestGlobalMemoryDumpForPidCallback) override;
+  void RequestPrivateMemoryFootprint(
+      base::ProcessId,
+      RequestPrivateMemoryFootprintCallback) override;
   void RequestGlobalMemoryDumpAndAppendToTrace(
       base::trace_event::MemoryDumpType,
       base::trace_event::MemoryDumpLevelOfDetail,

services/resource_coordinator/memory_instrumentation/queued_request.cc

@@ -10,12 +10,14 @@ QueuedRequest::Args::Args(MemoryDumpType dump_type,
                           MemoryDumpLevelOfDetail level_of_detail,
                           const std::vector<std::string>& allocator_dump_names,
                           bool add_to_trace,
-                          base::ProcessId pid)
+                          base::ProcessId pid,
+                          bool memory_footprint_only)
     : dump_type(dump_type),
       level_of_detail(level_of_detail),
       allocator_dump_names(allocator_dump_names),
       add_to_trace(add_to_trace),
-      pid(pid) {}
+      pid(pid),
+      memory_footprint_only(memory_footprint_only) {}
 QueuedRequest::Args::Args(const Args& args) = default;
 QueuedRequest::Args::~Args() = default;
 

services/resource_coordinator/memory_instrumentation/queued_request.h

@@ -33,7 +33,8 @@ struct QueuedRequest {
          MemoryDumpLevelOfDetail level_of_detail,
          const std::vector<std::string>& allocator_dump_names,
          bool add_to_trace,
-         base::ProcessId pid);
+         base::ProcessId pid,
+         bool memory_footprint_only);
     Args(const Args&);
     ~Args();
 
@@ -42,6 +43,10 @@ struct QueuedRequest {
     const std::vector<std::string> allocator_dump_names;
     const bool add_to_trace;
     const base::ProcessId pid;
+
+    // If this member is |true|, then no MemoryDumpProviders are queried. The
+    // only other relevant member is |pid|.
+    const bool memory_footprint_only;
   };
 
   struct PendingResponse {

services/resource_coordinator/memory_instrumentation/queued_request_dispatcher.cc

@@ -216,18 +216,20 @@ void QueuedRequestDispatcher::SetUpAndDispatch(
     request->responses[client].process_id = client_info.pid;
     request->responses[client].process_type = client_info.process_type;
 
-    // Don't request a chrome memory dump at all if the client wants only the
-    // processes' vm regions, which are retrieved via RequestOSMemoryDump().
+    // Don't request a chrome memory dump at all if the client only wants the
+    // a memory footprint.
     //
     // This must occur before the call to RequestOSMemoryDump, as
     // ClientProcessImpl will [for macOS], delay the calculations for the
     // OSMemoryDump until the Chrome memory dump is finished. See
     // https://bugs.chromium.org/p/chromium/issues/detail?id=812346#c16 for more
     // details.
-    request->pending_responses.insert({client, ResponseType::kChromeDump});
-    client->RequestChromeMemoryDump(
-        request->GetRequestArgs(),
-        base::BindOnce(std::move(chrome_callback), client));
+    if (!request->args.memory_footprint_only) {
+      request->pending_responses.insert({client, ResponseType::kChromeDump});
+      client->RequestChromeMemoryDump(
+          request->GetRequestArgs(),
+          base::BindOnce(std::move(chrome_callback), client));
+    }
 
 // On most platforms each process can dump data about their own process
 // so ask each process to do so Linux is special see below.
@@ -496,11 +498,16 @@ void QueuedRequestDispatcher::Finalize(QueuedRequest* request,
       }
     }
 
-    // Ignore incomplete results (can happen if the client crashes/disconnects).
-    const bool valid =
-        raw_os_dump && raw_chrome_dump &&
-        (request->memory_map_option() == mojom::MemoryMapOption::NONE ||
-         (raw_os_dump && !raw_os_dump->memory_maps.empty()));
+    bool valid = false;
+    if (request->args.memory_footprint_only) {
+      valid = raw_os_dump;
+    } else {
+      // Ignore incomplete results (can happen if the client
+      // crashes/disconnects).
+      valid = raw_os_dump && raw_chrome_dump &&
+              (request->memory_map_option() == mojom::MemoryMapOption::NONE ||
+               (raw_os_dump && !raw_os_dump->memory_maps.empty()));
+    }
 
     if (!valid)
       continue;

services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.cc

@@ -51,11 +51,6 @@ MemoryInstrumentation::~MemoryInstrumentation() {
   g_instance = nullptr;
 }
 
-void MemoryInstrumentation::RequestGlobalDump(
-    RequestGlobalDumpCallback callback) {
-  RequestGlobalDump({}, callback);
-}
-
 void MemoryInstrumentation::RequestGlobalDump(
     const std::vector<std::string>& allocator_dump_names,
     RequestGlobalDumpCallback callback) {
@@ -66,10 +61,12 @@ void MemoryInstrumentation::RequestGlobalDump(
       base::BindRepeating(&WrapGlobalMemoryDump, callback));
 }
 
-void MemoryInstrumentation::RequestGlobalDumpForPid(
+void MemoryInstrumentation::RequestPrivateMemoryFootprint(
     base::ProcessId pid,
     RequestGlobalDumpCallback callback) {
-  RequestGlobalDumpForPid(pid, {}, callback);
+  const auto& coordinator = GetCoordinatorBindingForCurrentThread();
+  coordinator->RequestPrivateMemoryFootprint(
+      pid, base::BindRepeating(&WrapGlobalMemoryDump, callback));
 }
 
 void MemoryInstrumentation::RequestGlobalDumpForPid(

services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.h

@@ -40,16 +40,6 @@ class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT MemoryInstrumentation {
                              const std::string& service_name);
   static MemoryInstrumentation* GetInstance();
 
-  // Requests a global memory dump.
-  // Returns asynchronously, via the callback argument:
-  //  (true, global_dump) if succeeded;
-  //  (false, nullptr) if failed.
-  // The callback (if not null), will be posted on the same thread of the
-  // RequestGlobalDump() call.
-  // TODO(lalitm): this is temporary until we migrate task manager, metrics
-  // etc to use the new method.
-  void RequestGlobalDump(RequestGlobalDumpCallback);
-
   // Requests a global memory dump with |allocator_dump_names| indicating
   // the name of allocator dumps in which the consumer is interested. If
   // |allocator_dump_names| is empty, no dumps will be returned.
@@ -59,16 +49,21 @@ class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT MemoryInstrumentation {
   //  but missing data.
   // The callback (if not null), will be posted on the same thread of the
   // RequestGlobalDump() call.
+  // Note: Even if |allocator_dump_names| is empty, all MemoryDumpProviders will
+  // still be queried.
   void RequestGlobalDump(const std::vector<std::string>& allocator_dump_names,
                          RequestGlobalDumpCallback);
 
-  // Requests a global memory dump.
-  // Returns asynchronously, via the callback argument, the global memory
-  // dump with the process memory dump for the given pid or null if the dump
-  // failed.
+  // Returns asynchronously, via the callback argument:
+  //  (true, global_dump) if succeeded;
+  //  (false, global_dump) if failed, with global_dump being non-null
+  //  but missing data.
   // The callback (if not null), will be posted on the same thread of the
-  // RequestGlobalDumpForPid() call.
-  void RequestGlobalDumpForPid(base::ProcessId pid, RequestGlobalDumpCallback);
+  // RequestPrivateMemoryFootprint() call.
+  // Passing a null |pid| is the same as requesting the PrivateMemoryFootprint
+  // for all processes.
+  void RequestPrivateMemoryFootprint(base::ProcessId pid,
+                                     RequestGlobalDumpCallback);
 
   // Requests a global memory dump with |allocator_dump_names| indicating
   // the name of allocator dumps in which the consumer is interested. If

services/resource_coordinator/public/cpp/memory_instrumentation/tracing_integration_unittest.cc

@@ -123,6 +123,10 @@ class MockCoordinator : public Coordinator, public mojom::Coordinator {
       const std::vector<std::string>& allocator_dump_names,
       RequestGlobalMemoryDumpForPidCallback) override {}
 
+  void RequestPrivateMemoryFootprint(
+      base::ProcessId pid,
+      RequestPrivateMemoryFootprintCallback) override {}
+
   void RequestGlobalMemoryDumpAndAppendToTrace(
       MemoryDumpType dump_type,
       MemoryDumpLevelOfDetail level_of_detail,

services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom

@@ -279,6 +279,12 @@ interface Coordinator {
                                 array<string> allocator_dump_names) =>
       (bool success, GlobalMemoryDump? global_memory_dump);
 
+  // Requesting a null pid is the same as requesting all pids.
+  // The returned dump only contains OS metrics. |chrome_allocator_dumps| will
+  // be empty.
+  RequestPrivateMemoryFootprint(mojo_base.mojom.ProcessId pid) =>
+      (bool success, GlobalMemoryDump? global_memory_dump);
+
   // Broadcasts a dump request to all registered client processes and injects the
   // dump in the trace buffer (if tracing is enabled).
   RequestGlobalMemoryDumpAndAppendToTrace(DumpType dump_type,