[tracing] Improve the memory maps dumper generation format.

This CL reworks the trace generation format for the memory maps
dumper according to latest discussion in the design doc
"Memory Dumping: Mmap and Smap data dumping details"
(http://goo.gl/e6WRnQ).
Furthermore this CL changes the output format of the total dumper to
dump a hex string rather than a float, in order to be consistent with
the mmaps dumper.

See http://pastebin.com/EHdNDMXN for an example of the generated JSON.

BUG=460884

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

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

7 files changed, 66 insertions, 41 deletions

diff --git a/base/trace_event/process_memory_maps.cc b/base/trace_event/process_memory_maps.cc
index bf3c5a0..d553ee8 100644
--- a/base/trace_event/process_memory_maps.cc
+++ b/base/trace_event/process_memory_maps.cc
@@ -4,6 +4,8 @@
 
 #include "base/trace_event/process_memory_maps.h"
 
+#include "base/format_macros.h"
+#include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event_argument.h"
 
 namespace base {
@@ -21,19 +23,25 @@ ProcessMemoryMaps::~ProcessMemoryMaps() {
 }
 
 void ProcessMemoryMaps::AsValueInto(TracedValue* value) const {
+  static const char kHexFmt[] = "%" PRIx64;
+
+  // Refer to the design doc goo.gl/sxfFY8 for the semantic of these fields.
   value->BeginArray("vm_regions");
   for (const auto& region : vm_regions_) {
     value->BeginDictionary();
 
-    value->SetDouble("start_address", region.start_address);
-    value->SetDouble("size_in_bytes", region.size_in_bytes);
-    value->SetInteger("protection_flags", region.protection_flags);
-    value->SetString("mapped_file", region.mapped_file);
-    value->SetDouble("mapped_file_offset", region.mapped_file_offset);
-
-    value->BeginDictionary("byte_stats");
-    value->SetDouble("resident", region.byte_stats_resident);
-    value->SetDouble("anonymous", region.byte_stats_anonymous);
+    value->SetString("sa", StringPrintf(kHexFmt, region.start_address));
+    value->SetString("sz", StringPrintf(kHexFmt, region.size_in_bytes));
+    value->SetInteger("pf", region.protection_flags);
+    value->SetString("mf", region.mapped_file);
+
+    value->BeginDictionary("bs");  // byte stats
+    value->SetString(
+        "pss", StringPrintf(kHexFmt, region.byte_stats_proportional_resident));
+    value->SetString("prv",
+                     StringPrintf(kHexFmt, region.byte_stats_private_resident));
+    value->SetString("shr",
+                     StringPrintf(kHexFmt, region.byte_stats_shared_resident));
     value->EndDictionary();
 
     value->EndDictionary();
diff --git a/base/trace_event/process_memory_maps.h b/base/trace_event/process_memory_maps.h
index 70f6610..dc1892f 100644
--- a/base/trace_event/process_memory_maps.h
+++ b/base/trace_event/process_memory_maps.h
@@ -28,9 +28,13 @@ class BASE_EXPORT ProcessMemoryMaps {
     uint64 size_in_bytes;
     uint32 protection_flags;
     std::string mapped_file;
-    uint64 mapped_file_offset;
-    uint64 byte_stats_resident;
-    uint64 byte_stats_anonymous;
+
+    // private_resident + shared_resident = resident set size.
+    uint64 byte_stats_private_resident;
+    uint64 byte_stats_shared_resident;
+
+    // For multiprocess accounting.
+    uint64 byte_stats_proportional_resident;
   };
 
   ProcessMemoryMaps();
diff --git a/base/trace_event/process_memory_maps_dump_provider.cc b/base/trace_event/process_memory_maps_dump_provider.cc
index fbe0eb1..d728bd3 100644
--- a/base/trace_event/process_memory_maps_dump_provider.cc
+++ b/base/trace_event/process_memory_maps_dump_provider.cc
@@ -60,7 +60,7 @@ bool ParseSmapsHeader(std::istream* smaps,
     region->protection_flags |=
         ProcessMemoryMaps::VMRegion::kProtectionFlagsExec;
   }
-  *smaps >> std::hex >> region->mapped_file_offset;
+  *smaps >> ignored;  // Ignore mapped file offset.
   *smaps >> ignored;  // Ignore device maj-min (fc:01 in the example above).
   *smaps >> ignored;  // Ignore inode number (1234 in the example above).
 
@@ -73,9 +73,15 @@ bool ParseSmapsHeader(std::istream* smaps,
   return res;
 }
 
+uint64 ReadCounterBytes(std::istream* smaps) {
+  uint64 counter_value = 0;
+  *smaps >> std::dec >> counter_value;
+  return counter_value * 1024;
+}
+
 uint32 ParseSmapsCounter(std::istream* smaps,
                          ProcessMemoryMaps::VMRegion* region) {
-  // e.g., "RSS:  0 Kb\n"
+  // A smaps counter lines looks as follows: "RSS:  0 Kb\n"
   uint32 res = 0;
   std::string counter_name;
   *smaps >> counter_name;
@@ -83,13 +89,17 @@ uint32 ParseSmapsCounter(std::istream* smaps,
   // TODO(primiano): "Swap" should also be accounted as resident. Check
   // whether Rss isn't already counting swapped and fix below if that is
   // the case.
-  if (counter_name == "Rss:") {
-    *smaps >> std::dec >> region->byte_stats_resident;
-    region->byte_stats_resident *= 1024;
+  if (counter_name == "Pss:") {
+    region->byte_stats_proportional_resident = ReadCounterBytes(smaps);
+    res = 1;
+  } else if (counter_name == "Private_Dirty:" ||
+             counter_name == "Private_Clean:") {
+    // For Private and Shared counters keep the sum of the dirty + clean stats.
+    region->byte_stats_private_resident += ReadCounterBytes(smaps);
     res = 1;
-  } else if (counter_name == "Anonymous:") {
-    *smaps >> std::dec >> region->byte_stats_anonymous;
-    region->byte_stats_anonymous *= 1024;
+  } else if (counter_name == "Shared_Dirty:" ||
+             counter_name == "Shared_Clean:") {
+    region->byte_stats_shared_resident += ReadCounterBytes(smaps);
     res = 1;
   }
 
@@ -111,7 +121,7 @@ uint32 ReadLinuxProcSmapsFile(std::istream* smaps, ProcessMemoryMaps* pmm) {
   if (!smaps->good())
     return 0;
 
-  const uint32 kNumExpectedCountersPerRegion = 2;
+  const uint32 kNumExpectedCountersPerRegion = 5;
   uint32 counters_parsed_for_current_region = 0;
   uint32 num_valid_regions = 0;
   ProcessMemoryMaps::VMRegion region;
diff --git a/base/trace_event/process_memory_maps_dump_provider_unittest.cc b/base/trace_event/process_memory_maps_dump_provider_unittest.cc
index 02fd136..0bf81ac 100644
--- a/base/trace_event/process_memory_maps_dump_provider_unittest.cc
+++ b/base/trace_event/process_memory_maps_dump_provider_unittest.cc
@@ -36,12 +36,12 @@ const char kTestSmaps1[] =
     "VmFlags: rd ex mr mw me dw sd\n"
     "ff000000-ff800000 -w-p 00001080 fc:01 0            /file/name with space\n"
     "Size:                  0 kB\n"
-    "Rss:                 128 kB\n"
+    "Rss:                 192 kB\n"
     "Pss:                 128 kB\n"
-    "Shared_Clean:        124 kB\n"
-    "Shared_Dirty:          0 kB\n"
-    "Private_Clean:        68 kB\n"
-    "Private_Dirty:         0 kB\n"
+    "Shared_Clean:        120 kB\n"
+    "Shared_Dirty:          4 kB\n"
+    "Private_Clean:        60 kB\n"
+    "Private_Dirty:         8 kB\n"
     "Referenced:          296 kB\n"
     "Anonymous:             0 kB\n"
     "AnonHugePages:         0 kB\n"
@@ -91,7 +91,7 @@ const char kTestSmaps2[] =
     "7fe7ce79c000-7fe7ce7a8000 ---p 00000000 00:00 0 \n"
     "Size:                 48 kB\n"
     "Rss:                  40 kB\n"
-    "Pss:                   0 kB\n"
+    "Pss:                  32 kB\n"
     "Shared_Clean:         16 kB\n"
     "Shared_Dirty:         12 kB\n"
     "Private_Clean:         8 kB\n"
@@ -141,17 +141,17 @@ TEST(ProcessMemoryMapsDumpProviderTest, ParseProcSmaps) {
   EXPECT_EQ(0x004be000UL - 0x00400000UL, regions_1[0].size_in_bytes);
   EXPECT_EQ(kProtR | kProtX, regions_1[0].protection_flags);
   EXPECT_EQ("/file/1", regions_1[0].mapped_file);
-  EXPECT_EQ(0UL, regions_1[0].mapped_file_offset);
-  EXPECT_EQ(296 * 1024UL, regions_1[0].byte_stats_resident);
-  EXPECT_EQ(68 * 1024UL, regions_1[0].byte_stats_anonymous);
+  EXPECT_EQ(162 * 1024UL, regions_1[0].byte_stats_proportional_resident);
+  EXPECT_EQ((228 + 0) * 1024UL, regions_1[0].byte_stats_shared_resident);
+  EXPECT_EQ((0 + 68) * 1024UL, regions_1[0].byte_stats_private_resident);
 
   EXPECT_EQ(0xff000000UL, regions_1[1].start_address);
   EXPECT_EQ(0xff800000UL - 0xff000000UL, regions_1[1].size_in_bytes);
   EXPECT_EQ(kProtW, regions_1[1].protection_flags);
   EXPECT_EQ("/file/name with space", regions_1[1].mapped_file);
-  EXPECT_EQ(0x00001080UL, regions_1[1].mapped_file_offset);
-  EXPECT_EQ(128 * 1024UL, regions_1[1].byte_stats_resident);
-  EXPECT_EQ(0UL, regions_1[1].byte_stats_anonymous);
+  EXPECT_EQ(128 * 1024UL, regions_1[1].byte_stats_proportional_resident);
+  EXPECT_EQ((120 + 4) * 1024UL, regions_1[1].byte_stats_shared_resident);
+  EXPECT_EQ((60 + 8) * 1024UL, regions_1[1].byte_stats_private_resident);
 
   // Parse the 2nd smaps file.
   ProcessMemoryDump pmd_2;
@@ -165,9 +165,9 @@ TEST(ProcessMemoryMapsDumpProviderTest, ParseProcSmaps) {
   EXPECT_EQ(0x7fe7ce7a8000UL - 0x7fe7ce79c000UL, regions_2[0].size_in_bytes);
   EXPECT_EQ(0U, regions_2[0].protection_flags);
   EXPECT_EQ("", regions_2[0].mapped_file);
-  EXPECT_EQ(0UL, regions_2[0].mapped_file_offset);
-  EXPECT_EQ(40 * 1024UL, regions_2[0].byte_stats_resident);
-  EXPECT_EQ(16 * 1024UL, regions_2[0].byte_stats_anonymous);
+  EXPECT_EQ(32 * 1024UL, regions_2[0].byte_stats_proportional_resident);
+  EXPECT_EQ((16 + 12) * 1024UL, regions_2[0].byte_stats_shared_resident);
+  EXPECT_EQ((8 + 4) * 1024UL, regions_2[0].byte_stats_private_resident);
 }
 #endif  // defined(OS_LINUX) || defined(OS_ANDROID)
 
diff --git a/base/trace_event/process_memory_totals.cc b/base/trace_event/process_memory_totals.cc
index 41ad788..9b0c377 100644
--- a/base/trace_event/process_memory_totals.cc
+++ b/base/trace_event/process_memory_totals.cc
@@ -4,13 +4,16 @@
 
 #include "base/trace_event/process_memory_totals.h"
 
+#include "base/format_macros.h"
+#include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event_argument.h"
 
 namespace base {
 namespace trace_event {
 
 void ProcessMemoryTotals::AsValueInto(TracedValue* value) const {
-  value->SetDouble("resident_set_bytes", resident_set_bytes_);
+  value->SetString("resident_set_bytes",
+                   StringPrintf("%" PRIx64, resident_set_bytes_));
 }
 
 }  // namespace trace_event
diff --git a/base/trace_event/process_memory_totals.h b/base/trace_event/process_memory_totals.h
index ef0289e..29c94c9 100644
--- a/base/trace_event/process_memory_totals.h
+++ b/base/trace_event/process_memory_totals.h
@@ -13,7 +13,7 @@ namespace trace_event {
 
 class TracedValue;
 
-// Dump provider which collects process-wide memory stats.
+// Data model for process-wide memory stats.
 class BASE_EXPORT ProcessMemoryTotals {
  public:
   ProcessMemoryTotals() : resident_set_bytes_(0) {}
diff --git a/base/trace_event/process_memory_totals_dump_provider_unittest.cc b/base/trace_event/process_memory_totals_dump_provider_unittest.cc
index c37e612..372db63 100644
--- a/base/trace_event/process_memory_totals_dump_provider_unittest.cc
+++ b/base/trace_event/process_memory_totals_dump_provider_unittest.cc
@@ -12,18 +12,18 @@ namespace base {
 namespace trace_event {
 
 TEST(ProcessMemoryTotalsDumpProviderTest, DumpRSS) {
-  auto mdptp = ProcessMemoryTotalsDumpProvider::GetInstance();
+  auto pmtdp = ProcessMemoryTotalsDumpProvider::GetInstance();
   scoped_ptr<ProcessMemoryDump> pmd_before(new ProcessMemoryDump());
   scoped_ptr<ProcessMemoryDump> pmd_after(new ProcessMemoryDump());
 
   ProcessMemoryTotalsDumpProvider::rss_bytes_for_testing = 1024;
-  mdptp->DumpInto(pmd_before.get());
+  pmtdp->DumpInto(pmd_before.get());
 
   // Pretend that the RSS of the process increased of +1M.
   const size_t kAllocSize = 1048576;
   ProcessMemoryTotalsDumpProvider::rss_bytes_for_testing += kAllocSize;
 
-  mdptp->DumpInto(pmd_after.get());
+  pmtdp->DumpInto(pmd_after.get());
 
   ProcessMemoryTotalsDumpProvider::rss_bytes_for_testing = 0;