Use faster, but less information methods to calculate memory dumps on macOS.

components/tracing/common/process_metrics_memory_dump_provider.cc

Index: components/tracing/common/process_metrics_memory_dump_provider.cc
diff --git a/components/tracing/common/process_metrics_memory_dump_provider.cc b/components/tracing/common/process_metrics_memory_dump_provider.cc
index a9fe6b5aa74e9083309eb3d3adb74a07a51c90b3..188dfc3ec41a7a6ed6e290550e3c3d1dcf4114e8 100644
--- a/components/tracing/common/process_metrics_memory_dump_provider.cc
+++ b/components/tracing/common/process_metrics_memory_dump_provider.cc
@@ -36,6 +36,7 @@
 #include <mach/mach.h>
 
 #include "base/numerics/safe_math.h"
+#include "base/process/process_metrics.h"
 #endif  // defined(OS_MACOSX)
 
 #if defined(OS_WIN)
@@ -387,31 +388,20 @@ bool GetDyldRegions(std::vector<VMRegion>* regions) {
   return true;
 }
 
-void PopulateByteStats(VMRegion* region, const vm_region_submap_info_64& info) {
-  uint32_t share_mode = info.share_mode;
-  if (share_mode == SM_COW && info.ref_count == 1)
-    share_mode = SM_PRIVATE;
-
-  uint64_t dirty_bytes = info.pages_dirtied * PAGE_SIZE;
-  uint64_t clean_bytes =
-      (info.pages_resident - info.pages_reusable - info.pages_dirtied) *
-      PAGE_SIZE;
-  switch (share_mode) {
+void PopulateByteStats(VMRegion* region,
+                       const vm_region_top_info_data_t& info) {
+  uint64_t dirty_bytes =
+      (info.private_pages_resident + info.shared_pages_resident) * PAGE_SIZE;
+  switch (info.share_mode) {
     case SM_LARGE_PAGE:
     case SM_PRIVATE:
-      region->byte_stats_private_dirty_resident = dirty_bytes;
-      region->byte_stats_private_clean_resident = clean_bytes;
-      break;
     case SM_COW:
       region->byte_stats_private_dirty_resident = dirty_bytes;
-      region->byte_stats_shared_clean_resident = clean_bytes;
-      break;
     case SM_SHARED:
     case SM_PRIVATE_ALIASED:
     case SM_TRUESHARED:
     case SM_SHARED_ALIASED:
       region->byte_stats_shared_dirty_resident = dirty_bytes;
-      region->byte_stats_shared_clean_resident = clean_bytes;
       break;
     case SM_EMPTY:
       break;
@@ -421,39 +411,45 @@ void PopulateByteStats(VMRegion* region, const vm_region_submap_info_64& info) {
   }
 }
 
-// Creates VMRegions from mach_vm_region_recurse. Returns whether the operation
+// Creates VMRegions using mach vm syscalls. Returns whether the operation
 // succeeded.
 bool GetAllRegions(std::vector<VMRegion>* regions) {
   const int pid = getpid();
   task_t task = mach_task_self();
   mach_vm_size_t size = 0;
-  vm_region_submap_info_64 info;
-  natural_t depth = 1;
-  mach_msg_type_number_t count = sizeof(info);
   mach_vm_address_t address = MACH_VM_MIN_ADDRESS;
   while (true) {
-    memset(&info, 0, sizeof(info));
-    kern_return_t kr = mach_vm_region_recurse(
-        task, &address, &size, &depth,
-        reinterpret_cast<vm_region_info_t>(&info), &count);
-    if (kr == KERN_INVALID_ADDRESS)  // nothing else left
+    base::CheckedNumeric<mach_vm_address_t> numeric(address);

[Mark Mentovai, 2017/04/21 12:33:52]

“numeric” again

[erikchen, 2017/04/21 18:20:25]

Done.

+    numeric += size;
+    if (!numeric.IsValid())
+      return false;
+    address = numeric.ValueOrDie();
+    mach_vm_address_t address_copy = address;
+
+    vm_region_top_info_data_t info;
+    base::MachVMRegionResult result =
+        base::GetTopInfo(task, &size, &address, &info);
+    if (result == base::MachVMRegionResult::Error)
+      return false;
+    if (result == base::MachVMRegionResult::Finished)
       break;
-    if (kr != KERN_SUCCESS)  // something bad
+
+    vm_region_basic_info_64 basic_info;
+    mach_vm_size_t dummy_size = 0;
+    result = base::GetBasicInfo(task, &dummy_size, &address_copy, &basic_info);
+    if (result == base::MachVMRegionResult::Error)
       return false;
-    if (info.is_submap) {
-      size = 0;
-      ++depth;
-      continue;
-    }
+    if (result == base::MachVMRegionResult::Finished)
+      break;
 
     VMRegion region;
     PopulateByteStats(&region, info);
 
-    if (info.protection & VM_PROT_READ)
+    if (basic_info.protection & VM_PROT_READ)
       region.protection_flags |= VMRegion::kProtectionFlagsRead;
-    if (info.protection & VM_PROT_WRITE)
+    if (basic_info.protection & VM_PROT_WRITE)
       region.protection_flags |= VMRegion::kProtectionFlagsWrite;
-    if (info.protection & VM_PROT_EXECUTE)
+    if (basic_info.protection & VM_PROT_EXECUTE)
       region.protection_flags |= VMRegion::kProtectionFlagsExec;
 
     char buffer[MAXPATHLEN];
@@ -461,16 +457,13 @@ bool GetAllRegions(std::vector<VMRegion>* regions) {
     if (length != 0)
       region.mapped_file.assign(buffer, length);
 
-    region.byte_stats_swapped = info.pages_swapped_out * PAGE_SIZE;
+    // There's no way to get swapped bytes without doing a very-expensive
+    // syscalls that craws every single page in the memory object. Set as 0

[Primiano Tucci (use gerrit), 2017/04/21 11:36:31]

I guess you intended s/craws/crawls/ ?

[erikchen, 2017/04/21 18:20:25]

Done.

+    // here, but know that it's inaccurate.
+    region.byte_stats_swapped = 0;

[Primiano Tucci (use gerrit), 2017/04/21 11:36:31]

This one seems not too useful, as VMRegion is zero-initialized. Also, if you do
this for documentation purposes I'd also do the same for consistency to the
*clean* fields.
What I'd do here is:
- drop the = 0 assignment.
- extend the comment to say: "There's no way to get too much details about vm
regions (swap, clean memory) without doing...."

[erikchen, 2017/04/21 18:20:25]

Done.

     region.start_address = address;
     region.size_in_bytes = size;
     regions->push_back(region);
-
-    base::CheckedNumeric<mach_vm_address_t> numeric(address);
-    numeric += size;
-    if (!numeric.IsValid())
-      return false;
-    address = numeric.ValueOrDie();
   }
   return true;
 }