[Paint Preview] Split PaintPreviewCompositor memory metrics from the general...

[Paint Preview] Split PaintPreviewCompositor memory metrics from the general Utility process metrics

Change-Id: I6067aad3959c054181389c1e1b82e464eacd9a4b
Bug: 1135217
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441263Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Commit-Queue: Yashar Dabiran <yashard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815285}

chrome/browser/DEPS

@@ -252,6 +252,7 @@ include_rules = [
   "+components/services/heap_profiling",
   "+components/services/language_detection/public/cpp",
   "+components/services/language_detection/public/mojom",
+  "+components/services/paint_preview_compositor/public/mojom",
   "+components/services/patch/content",
   "+components/services/patch/public",
   "+components/services/print_compositor/public",

chrome/browser/metrics/process_memory_metrics_emitter.cc

@@ -27,6 +27,7 @@
 #include "components/performance_manager/public/graph/page_node.h"
 #include "components/performance_manager/public/graph/process_node.h"
 #include "components/performance_manager/public/performance_manager.h"
+#include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h"
 #include "content/public/browser/audio_service_info.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/content_switches.h"
@@ -620,36 +621,7 @@ void EmitGpuMemoryMetrics(const GlobalMemoryDump::ProcessDump& pmd,
   builder.Record(ukm_recorder);
 }
 
-void EmitUtilityMemoryMetrics(const GlobalMemoryDump::ProcessDump& pmd,
-                              ukm::SourceId ukm_source_id,
-                              ukm::UkmRecorder* ukm_recorder,
-                              const base::Optional<base::TimeDelta>& uptime,
-                              bool record_uma) {
-  Memory_Experimental builder(ukm_source_id);
-  builder.SetProcessType(static_cast<int64_t>(
-      memory_instrumentation::mojom::ProcessType::UTILITY));
-  EmitProcessUmaAndUkm(pmd, HistogramProcessType::kUtility, uptime, record_uma,
-                       &builder);
-
-  builder.Record(ukm_recorder);
-}
-
-void EmitAudioServiceMemoryMetrics(
-    const GlobalMemoryDump::ProcessDump& pmd,
-    ukm::SourceId ukm_source_id,
-    ukm::UkmRecorder* ukm_recorder,
-    const base::Optional<base::TimeDelta>& uptime,
-    bool record_uma) {
-  Memory_Experimental builder(ukm_source_id);
-  builder.SetProcessType(static_cast<int64_t>(
-      memory_instrumentation::mojom::ProcessType::UTILITY));
-  EmitProcessUmaAndUkm(pmd, HistogramProcessType::kAudioService, uptime,
-                       record_uma, &builder);
-
-  builder.Record(ukm_recorder);
-}
-
-void EmitNetworkServiceMemoryMetrics(
+void EmitUtilityMemoryMetrics(HistogramProcessType ptype,
                               const GlobalMemoryDump::ProcessDump& pmd,
                               ukm::SourceId ukm_source_id,
                               ukm::UkmRecorder* ukm_recorder,
@@ -658,8 +630,7 @@ void EmitNetworkServiceMemoryMetrics(
   Memory_Experimental builder(ukm_source_id);
   builder.SetProcessType(static_cast<int64_t>(
       memory_instrumentation::mojom::ProcessType::UTILITY));
-  EmitProcessUmaAndUkm(pmd, HistogramProcessType::kNetworkService, uptime,
-                       record_uma, &builder);
+  EmitProcessUmaAndUkm(pmd, ptype, uptime, record_uma, &builder);
 
   builder.Record(ukm_recorder);
 }
@@ -890,20 +861,22 @@ void ProcessMemoryMetricsEmitter::CollateResults() {
         break;
       }
       case memory_instrumentation::mojom::ProcessType::UTILITY: {
+        HistogramProcessType ptype;
         if (pmd.pid() == content::GetProcessIdForAudioService()) {
-          EmitAudioServiceMemoryMetrics(
-              pmd, ukm::UkmRecorder::GetNewSourceID(), GetUkmRecorder(),
-              GetProcessUptime(now, pmd.pid()), emit_metrics_for_all_processes);
+          ptype = HistogramProcessType::kAudioService;
         } else if (pmd.service_name() ==
                    network::mojom::NetworkService::Name_) {
-          EmitNetworkServiceMemoryMetrics(
-              pmd, ukm::UkmRecorder::GetNewSourceID(), GetUkmRecorder(),
-              GetProcessUptime(now, pmd.pid()), emit_metrics_for_all_processes);
+          ptype = HistogramProcessType::kNetworkService;
+        } else if (pmd.service_name() ==
+                   paint_preview::mojom::PaintPreviewCompositorCollection::
+                       Name_) {
+          ptype = HistogramProcessType::kPaintPreviewCompositor;
         } else {
+          ptype = HistogramProcessType::kUtility;
+        }
         EmitUtilityMemoryMetrics(
-              pmd, ukm::UkmRecorder::GetNewSourceID(), GetUkmRecorder(),
+            ptype, pmd, ukm::UkmRecorder::GetNewSourceID(), GetUkmRecorder(),
             GetProcessUptime(now, pmd.pid()), emit_metrics_for_all_processes);
-        }
         break;
       }
       case memory_instrumentation::mojom::ProcessType::PLUGIN:

chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc

@@ -14,14 +14,17 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "build/build_config.h"
 #include "chrome/browser/metrics/renderer_uptime_tracker.h"
+#include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h"
 #include "components/ukm/test_ukm_recorder.h"
 #include "content/public/test/browser_task_environment.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/browser_metrics.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using GlobalMemoryDump = memory_instrumentation::GlobalMemoryDump;
 using GlobalMemoryDumpPtr = memory_instrumentation::mojom::GlobalMemoryDumpPtr;
+using HistogramProcessType = memory_instrumentation::HistogramProcessType;
 using ProcessMemoryDumpPtr =
     memory_instrumentation::mojom::ProcessMemoryDumpPtr;
 using OSMemDumpPtr = memory_instrumentation::mojom::OSMemDumpPtr;
@@ -473,45 +476,87 @@ MetricMap GetExpectedAudioServiceMetrics() {
   });
 }
 
+void PopulatePaintPreviewCompositorMetrics(GlobalMemoryDumpPtr& global_dump,
+                                           MetricMap& metrics_mb) {
+  auto process_memory_dump =
+      memory_instrumentation::mojom::ProcessMemoryDump::New();
+  process_memory_dump->service_name =
+      paint_preview::mojom::PaintPreviewCompositorCollection::Name_;
+  ProcessMemoryDumpPtr pmd(std::move(process_memory_dump));
+  pmd->process_type = ProcessType::UTILITY;
+  OSMemDumpPtr os_dump =
+      GetFakeOSMemDump(GetResidentValue(metrics_mb) * 1024,
+                       metrics_mb["PrivateMemoryFootprint"] * 1024,
+                       metrics_mb["SharedMemoryFootprint"] * 1024
+#if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_ANDROID)
+                       // accessing PrivateSwapFootprint on other OSes will
+                       // modify metrics_mb to create the value, which leads to
+                       // expectation failures.
+                       ,
+                       metrics_mb["PrivateSwapFootprint"] * 1024
+#endif
+      );
+  pmd->os_dump = std::move(os_dump);
+  global_dump->process_dumps.push_back(std::move(pmd));
+}
+
+MetricMap GetExpectedPaintPreviewCompositorMetrics() {
+  return MetricMap({
+    {"ProcessType", static_cast<int64_t>(ProcessType::UTILITY)},
+#if !defined(OS_MAC)
+        {"Resident", 10},
+#endif
+        {"PrivateMemoryFootprint", 30}, {"SharedMemoryFootprint", 35},
+#if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_ANDROID)
+        {"PrivateSwapFootprint", 50},
+#endif
+  });
+}
+
 void PopulateMetrics(GlobalMemoryDumpPtr& global_dump,
-                     ProcessType ptype,
+                     HistogramProcessType ptype,
                      MetricMap& metrics_mb) {
   switch (ptype) {
-    case ProcessType::BROWSER:
-      PopulateBrowserMetrics(global_dump, metrics_mb);
+    case HistogramProcessType::kAudioService:
+      PopulateAudioServiceMetrics(global_dump, metrics_mb);
       return;
-    case ProcessType::RENDERER:
-      PopulateRendererMetrics(global_dump, metrics_mb, 101);
+    case HistogramProcessType::kBrowser:
+      PopulateBrowserMetrics(global_dump, metrics_mb);
       return;
-    case ProcessType::GPU:
+    case HistogramProcessType::kGpu:
       PopulateGpuMetrics(global_dump, metrics_mb);
       return;
-    case ProcessType::UTILITY:
-      PopulateAudioServiceMetrics(global_dump, metrics_mb);
+    case HistogramProcessType::kPaintPreviewCompositor:
+      PopulatePaintPreviewCompositorMetrics(global_dump, metrics_mb);
+      return;
+    case HistogramProcessType::kRenderer:
+      PopulateRendererMetrics(global_dump, metrics_mb, 101);
       return;
-    case ProcessType::PLUGIN:
-    case ProcessType::OTHER:
-    case ProcessType::ARC:
+    case HistogramProcessType::kExtension:
+    case HistogramProcessType::kNetworkService:
+    case HistogramProcessType::kUtility:
       break;
   }
 
   // We shouldn't reach here.
-  FAIL() << "Unknown process type case " << ptype << ".";
+  CHECK(false);
 }
 
-MetricMap GetExpectedProcessMetrics(ProcessType ptype) {
+MetricMap GetExpectedProcessMetrics(HistogramProcessType ptype) {
   switch (ptype) {
-    case ProcessType::BROWSER:
+    case HistogramProcessType::kAudioService:
+      return GetExpectedAudioServiceMetrics();
+    case HistogramProcessType::kBrowser:
       return GetExpectedBrowserMetrics();
-    case ProcessType::RENDERER:
-      return GetExpectedRendererMetrics();
-    case ProcessType::GPU:
+    case HistogramProcessType::kGpu:
       return GetExpectedGpuMetrics();
-    case ProcessType::UTILITY:
-      return GetExpectedAudioServiceMetrics();
-    case ProcessType::PLUGIN:
-    case ProcessType::OTHER:
-    case ProcessType::ARC:
+    case HistogramProcessType::kPaintPreviewCompositor:
+      return GetExpectedPaintPreviewCompositorMetrics();
+    case HistogramProcessType::kRenderer:
+      return GetExpectedRendererMetrics();
+    case HistogramProcessType::kExtension:
+    case HistogramProcessType::kNetworkService:
+    case HistogramProcessType::kUtility:
       break;
   }
 
@@ -585,7 +630,7 @@ ProcessInfoVector GetProcessInfo(ukm::TestUkmRecorder& ukm_recorder) {
 }  // namespace
 
 class ProcessMemoryMetricsEmitterTest
-    : public testing::TestWithParam<ProcessType> {
+    : public testing::TestWithParam<HistogramProcessType> {
  public:
   ProcessMemoryMetricsEmitterTest() {}
   ~ProcessMemoryMetricsEmitterTest() override {}
@@ -640,12 +685,14 @@ TEST_P(ProcessMemoryMetricsEmitterTest, CollectsSingleProcessUKMs) {
   CheckMemoryUkmEntryMetrics(expected_entries);
 }
 
-INSTANTIATE_TEST_SUITE_P(SinglePtype,
+INSTANTIATE_TEST_SUITE_P(
+    SinglePtype,
     ProcessMemoryMetricsEmitterTest,
-                         testing::Values(ProcessType::BROWSER,
-                                         ProcessType::RENDERER,
-                                         ProcessType::GPU,
-                                         ProcessType::UTILITY));
+    testing::Values(HistogramProcessType::kBrowser,
+                    HistogramProcessType::kRenderer,
+                    HistogramProcessType::kGpu,
+                    HistogramProcessType::kPaintPreviewCompositor,
+                    HistogramProcessType::kAudioService));
 
 TEST_F(ProcessMemoryMetricsEmitterTest, CollectsExtensionProcessUKMs) {
   MetricMap expected_metrics = GetExpectedRendererMetrics();
@@ -668,10 +715,17 @@ TEST_F(ProcessMemoryMetricsEmitterTest, CollectsExtensionProcessUKMs) {
 }
 
 TEST_F(ProcessMemoryMetricsEmitterTest, CollectsManyProcessUKMsSingleDump) {
-  std::vector<ProcessType> entries_ptypes = {
-      ProcessType::BROWSER,  ProcessType::RENDERER, ProcessType::GPU,
-      ProcessType::UTILITY,  ProcessType::UTILITY,  ProcessType::GPU,
-      ProcessType::RENDERER, ProcessType::BROWSER,
+  std::vector<HistogramProcessType> entries_ptypes = {
+      HistogramProcessType::kBrowser,
+      HistogramProcessType::kRenderer,
+      HistogramProcessType::kGpu,
+      HistogramProcessType::kAudioService,
+      HistogramProcessType::kPaintPreviewCompositor,
+      HistogramProcessType::kPaintPreviewCompositor,
+      HistogramProcessType::kAudioService,
+      HistogramProcessType::kGpu,
+      HistogramProcessType::kRenderer,
+      HistogramProcessType::kBrowser,
   };
 
   GlobalMemoryDumpPtr global_dump(
@@ -693,11 +747,15 @@ TEST_F(ProcessMemoryMetricsEmitterTest, CollectsManyProcessUKMsSingleDump) {
 }
 
 TEST_F(ProcessMemoryMetricsEmitterTest, CollectsManyProcessUKMsManyDumps) {
-  std::vector<std::vector<ProcessType>> entries_ptypes = {
-      {ProcessType::BROWSER, ProcessType::RENDERER, ProcessType::GPU,
-       ProcessType::UTILITY},
-      {ProcessType::UTILITY, ProcessType::GPU, ProcessType::RENDERER,
-       ProcessType::BROWSER},
+  std::vector<std::vector<HistogramProcessType>> entries_ptypes = {
+      {HistogramProcessType::kBrowser, HistogramProcessType::kRenderer,
+       HistogramProcessType::kGpu,
+       HistogramProcessType::kPaintPreviewCompositor,
+       HistogramProcessType::kAudioService},
+      {HistogramProcessType::kBrowser, HistogramProcessType::kRenderer,
+       HistogramProcessType::kGpu,
+       HistogramProcessType::kPaintPreviewCompositor,
+       HistogramProcessType::kAudioService},
   };
 
   std::vector<MetricMap> entries_metrics;

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

@@ -18,6 +18,7 @@ const char kBrowserHistogramName[] = "Browser";
 const char kExtensionHistogramName[] = "Extension";
 const char kGpuHistogramName[] = "Gpu";
 const char kNetworkServiceHistogramName[] = "NetworkService";
+const char kPaintPreviewCompositorHistogramName[] = "PaintPreviewCompositor";
 const char kRendererHistogramName[] = "Renderer";
 const char kUtilityHistogramName[] = "Utility";
 
@@ -37,6 +38,8 @@ const char* HistogramProcessTypeToString(HistogramProcessType type) {
       return kGpuHistogramName;
     case HistogramProcessType::kNetworkService:
       return kNetworkServiceHistogramName;
+    case HistogramProcessType::kPaintPreviewCompositor:
+      return kPaintPreviewCompositorHistogramName;
     case HistogramProcessType::kRenderer:
       return kRendererHistogramName;
     case HistogramProcessType::kUtility:

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

@@ -29,6 +29,7 @@ enum class HistogramProcessType {
   kExtension,
   kGpu,
   kNetworkService,
+  kPaintPreviewCompositor,
   kRenderer,
   kUtility,
 };

tools/metrics/histograms/histograms_xml/memory/histograms.xml

@@ -1631,6 +1631,65 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="Memory.PaintPreviewCompositor.PrivateMemoryFootprint"
+    units="MB" expires_after="2021-04-10">
+  <owner>ckitagawa@chromium.org</owner>
+  <owner>fredmello@chromium.org</owner>
+  <owner>mahmoudi@chromium.org</owner>
+  <summary>
+    A rough estimate of the private memory footprint of the paint preview
+    compositor process.
+
+    Recorded at Poisson sampled time intervals with a mean of 5 minutes on
+    Android and 30 minutes on other platforms.
+  </summary>
+</histogram>
+
+<histogram name="Memory.PaintPreviewCompositor.PrivateSwapFootprint" units="MB"
+    expires_after="2021-04-10">
+  <owner>ckitagawa@chromium.org</owner>
+  <owner>fredmello@chromium.org</owner>
+  <owner>mahmoudi@chromium.org</owner>
+  <summary>
+    An amount of private memory of the paint preview compositor process placed
+    in swap (VmSwap).
+
+    Recorded at Poisson sampled time intervals with a mean of 5 minutes on
+    Android and 30 minutes on other platforms.
+  </summary>
+</histogram>
+
+<histogram name="Memory.PaintPreviewCompositor.ResidentSet" units="MiB"
+    expires_after="2021-04-10">
+  <owner>ckitagawa@chromium.org</owner>
+  <owner>fredmello@chromium.org</owner>
+  <owner>mahmoudi@chromium.org</owner>
+  <summary>
+    The size of the resident memory in a paint preview compositor process. This
+    is influenced by factors we control (e.g. memory that is not accessed can be
+    swapped) and factors we don't control (e.g. an unrelated process using a lot
+    of memory can force memory in our process to be swapped). Recorded once on
+    Windows/Linux/ChromeOS/Android.
+
+    Recorded at Poisson sampled time intervals with a mean of 5 minutes on
+    Android and 30 minutes on other platforms.
+  </summary>
+</histogram>
+
+<histogram name="Memory.PaintPreviewCompositor.SharedMemoryFootprint"
+    units="MB" expires_after="2021-04-10">
+  <owner>ckitagawa@chromium.org</owner>
+  <owner>fredmello@chromium.org</owner>
+  <owner>mahmoudi@chromium.org</owner>
+  <summary>
+    A rough estimate of the shared memory footprint of the paint preview
+    compositor process.
+
+    Recorded at Poisson sampled time intervals with a mean of 5 minutes on
+    Android and 30 minutes on other platforms.
+  </summary>
+</histogram>
+
 <histogram name="Memory.ParkableString.CompressedSizeKb" units="KB"
     expires_after="M87">
   <owner>lizeb@chromium.org</owner>