[base] Use ByteCount in SmapsRollup

Bug: 429140103
Change-Id: I33859fb23a3378ce3f81b9c4dc6d6358bd678e2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6764412
Reviewed-by: Francois Pierre Doray <[email protected]>
Commit-Queue: Thiabaud Engelbrecht <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1500159}
NOKEYCHECK=True
GitOrigin-RevId: 4e8c8ee1af3fd270ef21b2a21ecfa9b5e4f8616b

Author: Thiabaud Engelbrecht

android/self_compaction_manager.cc

@@ -83,10 +83,6 @@ bool IsMadvisePageoutSupported() {
 // be more than enough.
 constexpr base::TimeDelta kCompactionTimeout = base::Seconds(10);
 
-uint64_t BytesToMiB(uint64_t v) {
-  return v / 1024 / 1024;
-}
-
 uint64_t MiBToBytes(uint64_t v) {
   return v * 1024 * 1024;
 }
@@ -515,11 +511,11 @@ void SelfCompactionManager::CompactionMetric::MaybeRecordCompactionMetrics() {
 }
 
 void SelfCompactionManager::CompactionMetric::RecordCompactionMetric(
-    size_t value_bytes,
+    ByteCount value_bytes,
     std::string_view metric_name,
     std::string_view suffix) {
   UmaHistogramMemoryMB(GetMetricName(metric_name, suffix),
-                       static_cast<int>(BytesToMiB(value_bytes)));
+                       static_cast<int>(value_bytes.InMiB()));
 }
 
 void SelfCompactionManager::CompactionMetric::RecordCompactionMetrics(
@@ -533,12 +529,13 @@ void SelfCompactionManager::CompactionMetric::RecordCompactionMetrics(
 }
 
 void SelfCompactionManager::CompactionMetric::RecordCompactionDiffMetric(
-    size_t before_value_bytes,
-    size_t after_value_bytes,
+    ByteCount before_value_bytes,
+    ByteCount after_value_bytes,
     std::string_view name,
     std::string_view suffix) {
-  size_t diff_non_negative = std::max(before_value_bytes, after_value_bytes) -
-                             std::min(before_value_bytes, after_value_bytes);
+  ByteCount diff_non_negative =
+      std::max(before_value_bytes, after_value_bytes) -
+      std::min(before_value_bytes, after_value_bytes);
   const std::string full_suffix = StrCat(
       {"Diff.", suffix, ".",
        before_value_bytes < after_value_bytes ? "Increase" : "Decrease"});

android/self_compaction_manager.h

@@ -8,6 +8,7 @@
 // TODO(thiabaud): remove this include once we have separated the locks
 // between these two classes.
 #include "base/android/pre_freeze_background_memory_trimmer.h"
+#include "base/byte_count.h"
 #include "base/debug/proc_maps_linux.h"
 #include "base/no_destructor.h"
 #include "base/profiler/sample_metadata.h"
@@ -68,14 +69,14 @@ class BASE_EXPORT SelfCompactionManager {
                               std::string_view suffix) const;
     void RecordCompactionMetrics(const debug::SmapsRollup& value,
                                  std::string_view suffix);
-    void RecordCompactionMetric(size_t value_bytes,
+    void RecordCompactionMetric(ByteCount value_bytes,
                                 std::string_view metric_name,
                                 std::string_view suffix);
     void RecordCompactionDiffMetrics(const debug::SmapsRollup& before,
                                      const debug::SmapsRollup& after,
                                      std::string_view suffix);
-    void RecordCompactionDiffMetric(size_t before_value_bytes,
-                                    size_t after_value_bytes,
+    void RecordCompactionDiffMetric(ByteCount before_value_bytes,
+                                    ByteCount after_value_bytes,
                                     std::string_view name,
                                     std::string_view suffix);
 

debug/proc_maps_linux.cc

@@ -188,7 +188,7 @@ std::optional<SmapsRollup> ParseSmapsRollup(const std::string& buffer) {
   std::vector<std::string> lines =
       SplitString(buffer, "\n", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
 
-  std::unordered_map<std::string, size_t> tmp;
+  std::unordered_map<std::string, ByteCount> tmp;
   for (const auto& line : lines) {
     // This should be more than enough space for any output we get (but we also
     // verify the size below).
@@ -200,7 +200,7 @@ std::optional<SmapsRollup> ParseSmapsRollup(const std::string& buffer) {
       // here. |resize| does not count the length of the nul-byte, and we want
       // to trim off the trailing colon at the end, so we use |strlen - 1| here.
       key.resize(strlen(key.c_str()) - 1);
-      tmp[key] = val * 1024;
+      tmp[key] = KiB(val);
     }
   }
 

debug/proc_maps_linux.h

@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "base/base_export.h"
+#include "base/byte_count.h"
 
 namespace base::debug {
 
@@ -101,16 +102,15 @@ BASE_EXPORT bool ReadProcMaps(std::string* proc_maps);
 BASE_EXPORT bool ParseProcMaps(const std::string& input,
                                std::vector<MappedMemoryRegion>* regions);
 
-// All values are in bytes.
 struct SmapsRollup {
-  size_t rss = 0;
-  size_t pss = 0;
-  size_t pss_anon = 0;
-  size_t pss_file = 0;
-  size_t pss_shmem = 0;
-  size_t private_dirty = 0;
-  size_t swap = 0;
-  size_t swap_pss = 0;
+  ByteCount rss = ByteCount(0);
+  ByteCount pss = ByteCount(0);
+  ByteCount pss_anon = ByteCount(0);
+  ByteCount pss_file = ByteCount(0);
+  ByteCount pss_shmem = ByteCount(0);
+  ByteCount private_dirty = ByteCount(0);
+  ByteCount swap = ByteCount(0);
+  ByteCount swap_pss = ByteCount(0);
 };
 
 // Attempts to read /proc/self/smaps_rollup. Returns nullopt on error.

debug/proc_maps_linux_unittest.cc

@@ -372,9 +372,9 @@ TEST(SmapsRollupTest, ReadAndParse) {
 
   SmapsRollup smaps_rollup = result.value();
 
-  EXPECT_GT(smaps_rollup.rss, 0u);
-  EXPECT_GT(smaps_rollup.pss, 0u);
-  EXPECT_GT(smaps_rollup.private_dirty, 0u);
+  EXPECT_GT(smaps_rollup.rss.InKiB(), 0u);
+  EXPECT_GT(smaps_rollup.pss.InKiB(), 0u);
+  EXPECT_GT(smaps_rollup.private_dirty.InKiB(), 0u);
 }
 
 TEST(SmapsRollupTest, Valid) {
@@ -411,14 +411,14 @@ Locked:                0 kB
 
   SmapsRollup smaps_rollup = result.value();
 
-  EXPECT_EQ(smaps_rollup.rss, 1024 * 1908u);
-  EXPECT_EQ(smaps_rollup.pss, 1024 * 573u);
-  EXPECT_EQ(smaps_rollup.private_dirty, 1024 * 104u);
-  EXPECT_EQ(smaps_rollup.pss_anon, 1024 * 100u);
-  EXPECT_EQ(smaps_rollup.pss_file, 1024 * 469u);
-  EXPECT_EQ(smaps_rollup.pss_shmem, 1024 * 12u);
-  EXPECT_EQ(smaps_rollup.swap, 1024 * 10u);
-  EXPECT_EQ(smaps_rollup.swap_pss, 1024 * 20u);
+  EXPECT_EQ(smaps_rollup.rss.InKiB(), 1908u);
+  EXPECT_EQ(smaps_rollup.pss.InKiB(), 573u);
+  EXPECT_EQ(smaps_rollup.private_dirty.InKiB(), 104u);
+  EXPECT_EQ(smaps_rollup.pss_anon.InKiB(), 100u);
+  EXPECT_EQ(smaps_rollup.pss_file.InKiB(), 469u);
+  EXPECT_EQ(smaps_rollup.pss_shmem.InKiB(), 12u);
+  EXPECT_EQ(smaps_rollup.swap.InKiB(), 10u);
+  EXPECT_EQ(smaps_rollup.swap_pss.InKiB(), 20u);
 }
 
 }  // namespace base::debug