[tracing] Improve the memory maps dumper generation format.

base/trace_event/process_memory_maps_dump_provider.cc

 // Copyright 2015 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 "base/trace_event/process_memory_maps_dump_provider.h"
 
 #include <cctype>
 #include <fstream>
 
 #include "base/logging.h"
@@ skipping 42 matching lines @@
         ProcessMemoryMaps::VMRegion::kProtectionFlagsRead;
   }
   if (protection_flags[1] == 'w') {
     region->protection_flags |=
         ProcessMemoryMaps::VMRegion::kProtectionFlagsWrite;
   }
   if (protection_flags[2] == 'x') {
     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).
 
   while (smaps->peek() == ' ')
     smaps->ignore(1);
   char mapped_file[kMaxLineSize];
   smaps->getline(mapped_file, sizeof(mapped_file));
   region->mapped_file = mapped_file;
 
   return res;
 }
 
 uint32 ParseSmapsCounter(std::istream* smaps,
                          ProcessMemoryMaps::VMRegion* region) {
   // e.g., "RSS:  0 Kb\n"
   uint32 res = 0;
   std::string counter_name;
   *smaps >> counter_name;
+  uint64 counter_value = 0;
 
   // 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:") {
+    *smaps >> std::dec >> counter_value;
+    region->byte_stats_proportional_resident = (counter_value * 1024);

[petrcermak, 2015/03/20 11:23:34]

Why do you *assign* to byte_stats_proportional_resident but *add* to
byte_stats_private_resident and byte_stats_shared_resident? If that's
intentional, you should probably add a comment explaining it.

[petrcermak, 2015/03/20 11:23:34]

There seems to be a little bit of code duplication here now. How about factoring
some code out into a function:

uint64 ExtractSmapsCounterBytes(std::istream* smaps) {
  uint64 counter_value;
  *smaps >> std::dec >> counter_value;
  return counter_value * 1024;
}

This would allow you to get rid of the counter_value variable and simplify the
if statement:

if (counter_name == "Pss:") {
  region->byte_states_proportional_resident += ExtractSmapsCounterBytes(smaps);
  res = 1;
} else if ...

[petrcermak, 2015/03/20 11:23:34]

No reason for parentheses around 'counter_value * 1024' (same below).

[Primiano Tucci (use gerrit), 2015/03/20 13:59:14]

Done.

[Primiano Tucci (use gerrit), 2015/03/20 13:59:14]

Done.

[Primiano Tucci (use gerrit), 2015/03/20 13:59:14]

I thought that was clear by the fact that Private/Shared has two items (shared
and clean) which are added up.
Added comment.

     res = 1;
-  } else if (counter_name == "Anonymous:") {
-    *smaps >> std::dec >> region->byte_stats_anonymous;
-    region->byte_stats_anonymous *= 1024;
+  } else if (counter_name == "Private_Dirty:" ||
+             counter_name == "Private_Clean:") {
+    *smaps >> std::dec >> counter_value;
+    region->byte_stats_private_resident += (counter_value * 1024);
+    res = 1;
+  } else if (counter_name == "Shared_Dirty:" ||
+             counter_name == "Shared_Clean:") {
+    *smaps >> std::dec >> counter_value;
+    region->byte_stats_shared_resident += (counter_value * 1024);
     res = 1;
   }
 
 #ifndef NDEBUG
   // Paranoid check against changes of the Kernel /proc interface.
   if (res) {
     std::string unit;
     *smaps >> unit;
     DCHECK_EQ("kB", unit);
   }
 #endif
 
   smaps->ignore(kMaxLineSize, '\n');
 
   return res;
 }
 
 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;
   bool should_add_current_region = false;
   for (;;) {
     int next = smaps->peek();
     if (next == std::ifstream::traits_type::eof() || next == '\n')
       break;
     if (isxdigit(next) && !isupper(next)) {
       region = {0};
@@ skipping 53 matching lines @@
 
   return false;
 }
 
 const char* ProcessMemoryMapsDumpProvider::GetFriendlyName() const {
   return kDumperFriendlyName;
 }
 
 }  // namespace trace_event
 }  // namespace base