memory-infra: Start cleaning up ProcessMetricsMemoryDumpProvider

ProcessMetricsMemoryDumpProvider uses a lot of ifdefs and is quite
confusing. Start pulling out the code into os_metrics.cc which will
have separate implementations for each platform.

Bug: 728199
Change-Id: I2cbc2b2e8860dce3ea1b819affb4dee2170fd5f7
Reviewed-on: https://chromium-review.googlesource.com/555497
Commit-Queue: Hector Dearman <hjd@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484927}

services/resource_coordinator/BUILD.gn

@@ -76,6 +76,7 @@ source_set("tests") {
     "coordination_unit/process_coordination_unit_impl_unittest.cc",
     "memory_instrumentation/coordinator_impl_unittest.cc",
     "memory_instrumentation/process_map_unittest.cc",
+    "public/cpp/memory_instrumentation/os_metrics_unittest.cc",
     "public/cpp/memory_instrumentation/process_metrics_memory_dump_provider_unittest.cc",
     "public/cpp/tracing/chrome_trace_event_agent_unittest.cc",
     "tracing/agent_registry_unittest.cc",

services/resource_coordinator/public/cpp/BUILD.gn

@@ -12,6 +12,11 @@ component("resource_coordinator_cpp") {
     "memory_instrumentation/coordinator.h",
     "memory_instrumentation/memory_instrumentation.cc",
     "memory_instrumentation/memory_instrumentation.h",
+    "memory_instrumentation/os_metrics.cc",
+    "memory_instrumentation/os_metrics.h",
+    "memory_instrumentation/os_metrics_linux.cc",
+    "memory_instrumentation/os_metrics_mac.cc",
+    "memory_instrumentation/os_metrics_win.cc",
     "memory_instrumentation/process_metrics_memory_dump_provider.cc",
     "memory_instrumentation/process_metrics_memory_dump_provider.h",
     "resource_coordinator_features.cc",
@@ -22,6 +27,12 @@ component("resource_coordinator_cpp") {
     "tracing/chrome_trace_event_agent.h",
   ]
 
+  if (is_android) {
+    set_sources_assignment_filter([])
+    sources += [ "memory_instrumentation/os_metrics_linux.cc" ]
+    set_sources_assignment_filter(sources_assignment_filter)
+  }
+
   defines = [ "SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_IMPLEMENTATION" ]
 
   deps = [

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

+// Copyright 2017 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 "services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics.h"
+
+namespace memory_instrumentation {
+
+mojom::RawOSMemDumpPtr GetOSMemoryDump(base::ProcessId pid) {
+  mojom::RawOSMemDumpPtr dump = mojom::RawOSMemDump::New();
+  FillOSMemoryDump(pid, dump.get());
+  return dump;
+}
+
+}  // namespace memory_instrumentation

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

+// Copyright 2017 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.
+#ifndef SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_MEMORY_INSTRUMENTATION_OS_METRICS_H_
+#define SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_MEMORY_INSTRUMENTATION_OS_METRICS_H_
+
+#include "base/process/process_handle.h"
+#include "services/resource_coordinator/public/cpp/resource_coordinator_export.h"
+#include "services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom.h"
+
+namespace memory_instrumentation {
+
+mojom::RawOSMemDumpPtr SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT
+GetOSMemoryDump(base::ProcessId pid);
+
+void FillOSMemoryDump(base::ProcessId pid, mojom::RawOSMemDump* dump);
+
+}  // namespace memory_instrumentation
+
+#endif  // SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_MEMORY_INSTRUMENTATION_OS_METRICS_H_

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

+// Copyright 2017 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 <fcntl.h>
+#include <stdint.h>
+#include <memory>
+
+#include "base/files/file_util.h"
+#include "base/files/scoped_file.h"
+#include "base/format_macros.h"
+#include "base/process/process_metrics.h"
+#include "base/strings/string_number_conversions.h"
+#include "build/build_config.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics.h"
+
+namespace memory_instrumentation {
+
+namespace {
+
+const uint32_t kMaxLineSize = 4096;
+
+base::ScopedFD OpenStatm(base::ProcessId pid) {
+  std::string name =
+      "/proc/" +
+      (pid == base::kNullProcessId ? "self" : base::IntToString(pid)) +
+      "/statm";
+  base::ScopedFD fd = base::ScopedFD(open(name.c_str(), O_RDONLY));
+  DCHECK(fd.is_valid());
+  return fd;
+}
+
+bool GetResidentAndSharedPagesFromStatmFile(int fd,
+                                            uint64_t* resident_pages,
+                                            uint64_t* shared_pages) {
+  lseek(fd, 0, SEEK_SET);
+  char line[kMaxLineSize];
+  int res = read(fd, line, kMaxLineSize - 1);
+  if (res <= 0)
+    return false;
+  line[res] = '\0';
+  int num_scanned =
+      sscanf(line, "%*s %" SCNu64 " %" SCNu64, resident_pages, shared_pages);
+  return num_scanned == 2;
+}
+
+std::unique_ptr<base::ProcessMetrics> CreateProcessMetrics(
+    base::ProcessId pid) {
+  if (pid == base::kNullProcessId) {
+    return base::ProcessMetrics::CreateCurrentProcessMetrics();
+  }
+  return base::ProcessMetrics::CreateProcessMetrics(pid);
+}
+
+}  // namespace
+
+void FillOSMemoryDump(base::ProcessId pid, mojom::RawOSMemDump* dump) {
+  base::ScopedFD autoclose = OpenStatm(pid);
+  int statm_fd = autoclose.get();
+
+  if (statm_fd == -1)
+    return;
+
+  uint64_t resident_pages;
+  uint64_t shared_pages;
+  bool success = GetResidentAndSharedPagesFromStatmFile(
+      statm_fd, &resident_pages, &shared_pages);
+
+  if (!success)
+    return;
+
+  auto process_metrics = CreateProcessMetrics(pid);
+
+  const static size_t page_size = base::GetPageSize();
+  uint64_t rss_anon_bytes = (resident_pages - shared_pages) * page_size;
+  uint64_t vm_swap_bytes = (resident_pages - shared_pages) * page_size;
+
+  dump->platform_private_footprint.rss_anon_bytes = rss_anon_bytes;
+  dump->platform_private_footprint.vm_swap_bytes = vm_swap_bytes;
+  dump->resident_set_kb = process_metrics->GetWorkingSetSize() / 1024;
+}
+
+}  // namespace memory_instrumentation

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

+// Copyright 2017 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 "services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics.h"
+
+#include "base/process/process_metrics.h"
+
+namespace memory_instrumentation {
+
+void FillOSMemoryDump(base::ProcessId pid, mojom::RawOSMemDump* dump) {
+  // Creating process metrics for child processes in mac or windows requires
+  // additional information like ProcessHandle or port provider.
+  DCHECK_EQ(base::kNullProcessId, pid);
+  auto process_metrics = base::ProcessMetrics::CreateCurrentProcessMetrics();
+
+  size_t private_bytes;
+  size_t shared_bytes;
+  size_t resident_bytes;
+  size_t locked_bytes;
+  if (!process_metrics->GetMemoryBytes(&private_bytes, &shared_bytes,
+                                       &resident_bytes, &locked_bytes)) {
+    return;
+  }
+  base::ProcessMetrics::TaskVMInfo info = process_metrics->GetTaskVMInfo();
+  dump->platform_private_footprint.phys_footprint_bytes = info.phys_footprint;
+  dump->platform_private_footprint.internal_bytes = info.internal;
+  dump->platform_private_footprint.compressed_bytes = info.compressed;
+  dump->resident_set_kb = resident_bytes / 1024;
+}
+
+}  // namespace memory_instrumentation

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

+// Copyright 2017 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 "services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics.h"
+
+#include "base/process/process_handle.h"
+#include "build/build_config.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace memory_instrumentation {
+
+TEST(OSMetricsTest, GivesNonZeroResults) {
+  base::ProcessId pid = base::kNullProcessId;
+  mojom::RawOSMemDumpPtr dump = GetOSMemoryDump(pid);
+#if defined(OS_LINUX) || defined(OS_ANDROID)
+  EXPECT_GT(dump->platform_private_footprint.rss_anon_bytes, 0u);
+#elif defined(OS_WIN)
+  EXPECT_GT(dump->platform_private_footprint.private_bytes, 0u);
+#elif defined(OS_MACOSX)
+  EXPECT_GT(dump->platform_private_footprint.internal_bytes, 0u);
+#endif
+}
+
+}  // namespace memory_instrumentation

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

+// Copyright 2017 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 "services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics.h"
+
+#include "base/process/process_metrics.h"
+
+namespace memory_instrumentation {
+
+void FillOSMemoryDump(base::ProcessId pid, mojom::RawOSMemDump* dump) {
+  // Creating process metrics for child processes in mac or windows requires
+  // additional information like ProcessHandle or port provider.
+  DCHECK_EQ(base::kNullProcessId, pid);
+  auto process_metrics = base::ProcessMetrics::CreateCurrentProcessMetrics();
+
+  size_t private_bytes = 0;
+  process_metrics->GetMemoryBytes(&private_bytes, nullptr);
+  dump->platform_private_footprint.private_bytes = private_bytes;
+  dump->resident_set_kb = process_metrics->GetWorkingSetSize() / 1024;
+}
+
+}  // namespace memory_instrumentation

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

@@ -23,6 +23,7 @@
 #include "base/trace_event/process_memory_maps.h"
 #include "base/trace_event/process_memory_totals.h"
 #include "build/build_config.h"
+#include "services/resource_coordinator/public/cpp/memory_instrumentation/os_metrics.h"
 
 #if defined(OS_MACOSX)
 #include <libproc.h>
@@ -181,20 +182,6 @@ uint32_t ReadLinuxProcSmapsFile(FILE* smaps_file,
   return num_valid_regions;
 }
 
-bool GetResidentAndSharedPagesFromStatmFile(int fd,
-                                            uint64_t* resident_pages,
-                                            uint64_t* shared_pages) {
-  lseek(fd, 0, SEEK_SET);
-  char line[kMaxLineSize];
-  int res = read(fd, line, kMaxLineSize - 1);
-  if (res <= 0)
-    return false;
-  line[res] = '\0';
-  int num_scanned =
-      sscanf(line, "%*s %" SCNu64 " %" SCNu64, resident_pages, shared_pages);
-  return num_scanned == 2;
-}
-
 #endif  // defined(OS_LINUX) || defined(OS_ANDROID)
 
 std::unique_ptr<base::ProcessMetrics> CreateProcessMetrics(
@@ -250,6 +237,18 @@ bool ProcessMetricsMemoryDumpProvider::DumpProcessMemoryMaps(
     pmd->set_has_process_mmaps();
   return res;
 }
+
+bool GetResidentPagesFromStatmFile(int fd, uint64_t* resident_pages) {
+  lseek(fd, 0, SEEK_SET);
+  char line[kMaxLineSize];
+  int res = read(fd, line, kMaxLineSize - 1);
+  if (res <= 0)
+    return false;
+  line[res] = '\0';
+  int num_scanned = sscanf(line, "%*s %" SCNu64, resident_pages);
+  return num_scanned == 1;
+}
+
 #endif  // defined(OS_LINUX) || defined(OS_ANDROID)
 
 #if defined(OS_WIN)
@@ -612,6 +611,22 @@ bool ProcessMetricsMemoryDumpProvider::OnMemoryDump(
 bool ProcessMetricsMemoryDumpProvider::DumpProcessTotals(
     const base::trace_event::MemoryDumpArgs& args,
     base::trace_event::ProcessMemoryDump* pmd) {
+// On Windows add extra region if necessary:
+#if defined(OS_WIN)
+  if (args.level_of_detail ==
+      base::trace_event::MemoryDumpLevelOfDetail::DETAILED) {
+    uint64_t pss_bytes = 0;
+    bool res = process_metrics_->GetProportionalSetSizeBytes(&pss_bytes);
+    if (res) {
+      base::trace_event::ProcessMemoryMaps::VMRegion region;
+      region.byte_stats_proportional_resident = pss_bytes;
+      pmd->process_mmaps()->AddVMRegion(region);
+      pmd->set_has_process_mmaps();
+    }
+  }
+#endif  // defined(OS_WIN)
+
+// On Mac set a few extra values on process_totals:
 #if defined(OS_MACOSX)
   size_t private_bytes;
   size_t shared_bytes;
@@ -621,66 +636,30 @@ bool ProcessMetricsMemoryDumpProvider::DumpProcessTotals(
                                         &resident_bytes, &locked_bytes)) {
     return false;
   }
-  uint64_t rss_bytes = resident_bytes;
   pmd->process_totals()->SetExtraFieldInBytes("private_bytes", private_bytes);
   pmd->process_totals()->SetExtraFieldInBytes("shared_bytes", shared_bytes);
   pmd->process_totals()->SetExtraFieldInBytes("locked_bytes", locked_bytes);
+#endif  // defined(OS_MACOSX)
 
-  base::trace_event::ProcessMemoryTotals::PlatformPrivateFootprint footprint;
-  base::ProcessMetrics::TaskVMInfo info = process_metrics_->GetTaskVMInfo();
-  footprint.phys_footprint_bytes = info.phys_footprint;
-  footprint.internal_bytes = info.internal;
-  footprint.compressed_bytes = info.compressed;
+  mojom::RawOSMemDumpPtr dump = GetOSMemoryDump(process_);
 
-  pmd->process_totals()->SetPlatformPrivateFootprint(footprint);
-#else
-  uint64_t rss_bytes = process_metrics_->GetWorkingSetSize();
-#endif  // defined(OS_MACOSX)
-  if (rss_bytes_for_testing)
+  uint64_t rss_bytes = dump->resident_set_kb * 1024;
+  if (rss_bytes_for_testing) {
     rss_bytes = rss_bytes_for_testing;
+  }
 
-  // rss_bytes will be 0 if the process ended while dumping.
-  if (!rss_bytes)
+  // resident set size will be 0 if the process ended while dumping.
+  if (rss_bytes == 0u) {
     return false;
-
-  uint64_t peak_rss_bytes = 0;
-
-#if defined(OS_LINUX) || defined(OS_ANDROID)
-  base::trace_event::ProcessMemoryTotals::PlatformPrivateFootprint footprint;
-
-  base::ScopedFD autoclose;
-  int statm_fd = fast_polling_statm_fd_.get();
-  if (statm_fd == -1) {
-    autoclose = OpenStatm();
-    statm_fd = autoclose.get();
   }
-  if (statm_fd == -1)
-    return false;
-  const static size_t page_size = base::GetPageSize();
-  uint64_t resident_pages;
-  uint64_t shared_pages;
-  bool success = GetResidentAndSharedPagesFromStatmFile(
-      statm_fd, &resident_pages, &shared_pages);
-  if (!success)
-    return false;
 
-  footprint.rss_anon_bytes = (resident_pages - shared_pages) * page_size;
-  footprint.vm_swap_bytes = process_metrics_->GetVmSwapBytes();
-  pmd->process_totals()->SetPlatformPrivateFootprint(footprint);
-#endif  // defined(OS_LINUX) || defined(OS_ANDROID)
-
-#if defined(OS_WIN)
-  {
-    size_t private_bytes;
-    base::trace_event::ProcessMemoryTotals::PlatformPrivateFootprint footprint;
-    process_metrics_->GetMemoryBytes(&private_bytes, nullptr);
-    footprint.private_bytes = private_bytes;
-    pmd->process_totals()->SetPlatformPrivateFootprint(footprint);
-  }
-#endif
+  pmd->process_totals()->set_resident_set_bytes(rss_bytes);
+  pmd->process_totals()->SetPlatformPrivateFootprint(
+      dump->platform_private_footprint);
+  pmd->set_has_process_totals();
+  pmd->process_totals()->set_peak_resident_set_bytes(GetPeakResidentSetBytes());
 
-#if !defined(OS_IOS)
-  peak_rss_bytes = process_metrics_->GetPeakWorkingSetSize();
+// On Linux and Android reset rss peak if necessary
 #if defined(OS_LINUX) || defined(OS_ANDROID)
   if (is_rss_peak_resettable_) {
     std::string clear_refs_file =
@@ -698,30 +677,20 @@ bool ProcessMetricsMemoryDumpProvider::DumpProcessTotals(
     }
     close(clear_refs_fd);
   }
-#elif defined(OS_WIN)
-  if (args.level_of_detail ==
-      base::trace_event::MemoryDumpLevelOfDetail::DETAILED) {
-    uint64_t pss_bytes = 0;
-    bool res = process_metrics_->GetProportionalSetSizeBytes(&pss_bytes);
-    if (res) {
-      base::trace_event::ProcessMemoryMaps::VMRegion region;
-      region.byte_stats_proportional_resident = pss_bytes;
-      pmd->process_mmaps()->AddVMRegion(region);
-      pmd->set_has_process_mmaps();
-    }
-  }
-
 #endif
-#endif  // !defined(OS_IOS)
-
-  pmd->process_totals()->set_resident_set_bytes(rss_bytes);
-  pmd->set_has_process_totals();
-  pmd->process_totals()->set_peak_resident_set_bytes(peak_rss_bytes);
 
   // Returns true even if other metrics failed, since rss is reported.
   return true;
 }
 
+uint64_t ProcessMetricsMemoryDumpProvider::GetPeakResidentSetBytes() {
+#if defined(OS_IOS)
+  return 0u;
+#else
+  return process_metrics_->GetPeakWorkingSetSize();
+#endif
+}
+
 #if defined(OS_LINUX) || defined(OS_ANDROID)
 base::ScopedFD ProcessMetricsMemoryDumpProvider::OpenStatm() {
   std::string name =
@@ -750,9 +719,7 @@ void ProcessMetricsMemoryDumpProvider::PollFastMemoryTotal(
   }
 
   uint64_t resident_pages = 0;
-  uint64_t ignored_shared_pages = 0;
-  if (!GetResidentAndSharedPagesFromStatmFile(statm_fd, &resident_pages,
-                                              &ignored_shared_pages))
+  if (!GetResidentPagesFromStatmFile(statm_fd, &resident_pages))
     return;
 
   static size_t page_size = base::GetPageSize();

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

@@ -13,6 +13,7 @@
 #include "base/trace_event/memory_dump_provider.h"
 #include "build/build_config.h"
 #include "services/resource_coordinator/public/cpp/resource_coordinator_export.h"
+#include "services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom.h"
 
 namespace base {
 class ProcessMetrics;
@@ -66,6 +67,7 @@ class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT
                          base::trace_event::ProcessMemoryDump* pmd);
   bool DumpProcessMemoryMaps(const base::trace_event::MemoryDumpArgs& args,
                              base::trace_event::ProcessMemoryDump* pmd);
+  uint64_t GetPeakResidentSetBytes();
 
   static uint64_t rss_bytes_for_testing;
   static FactoryFunction factory_for_testing;

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

@@ -17,6 +17,7 @@
 #include "base/trace_event/process_memory_maps.h"
 #include "base/trace_event/process_memory_totals.h"
 #include "base/trace_event/trace_event_argument.h"
+#include "services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(OS_MACOSX)
@@ -411,4 +412,5 @@ TEST(ProcessMetricsMemoryDumpProviderTest, TestMachOReading) {
 }
 
 #endif  // defined(OS_MACOSX)
+
 }  // namespace memory_instrumentation