libmeminfo: defer maps reading only when required for procmeminfo

This restores the original behavior. The main reason it should be this
way is to make the class generic for all things memory under
/proc/<pid>/. For example, with the current behavior, a program that
only needs to read /proc/<pid>/smaps_rollup will end up wasting time and
memory by parsing /proc/<pid>/maps when the object is being constructed.
Same goes for a program that only wants to reset the working set.

The 'ProcMemInfo' object still retains the property that it can only be
used once to read maps and the object must be destroyed + recreated to
update the stats.

Bug: 114325007
Bug: 111694435
Test: libmeminfo_test 1
Test:
 # adb push /google/data/ro/users/ss/sspatil/test-memutils.sh /data/local/tmp/
 # adb push procmem2 /data/local/tmp && adb push procrank2 /data/local/tmp
 # adb root && adb shell
 $ cd /data/local/tmp/
 $ chmod +x test-memutils.sh
 $ ./test-memutils.sh 2>&1 | tee test.log

Change-Id: I856d3b78a0088cff02cbd010b29ffbe0e35f5ee2
Signed-off-by: Sandeep Patil <sspatil@google.com>

3 files changed, 75 insertions, 6 deletions

diff --git a/libmeminfo/include/meminfo/procmeminfo.h b/libmeminfo/include/meminfo/procmeminfo.h
index 0d6f652ab..92375d31c 100644
--- a/libmeminfo/include/meminfo/procmeminfo.h
+++ b/libmeminfo/include/meminfo/procmeminfo.h
@@ -37,7 +37,7 @@ class ProcMemInfo final {
     const std::vector<Vma>& Maps();
     const MemUsage& Usage();
     const MemUsage& Wss();
-    const std::vector<uint16_t>& SwapOffsets() const;
+    const std::vector<uint16_t>& SwapOffsets();
 
     ~ProcMemInfo() = default;
 
diff --git a/libmeminfo/libmeminfo_test.cpp b/libmeminfo/libmeminfo_test.cpp
index e28c35249..b7a4b6b6f 100644
--- a/libmeminfo/libmeminfo_test.cpp
+++ b/libmeminfo/libmeminfo_test.cpp
@@ -245,6 +245,44 @@ TEST_F(ValidatePageAcct, TestPageIdle) {
     }
 }
 
+TEST(TestProcMemInfo, TestMapsEmpty) {
+    ProcMemInfo proc_mem(pid);
+    const std::vector<Vma>& maps = proc_mem.Maps();
+    EXPECT_GT(maps.size(), 0);
+}
+
+TEST(TestProcMemInfo, TestUsageEmpty) {
+    // If we created the object for getting working set,
+    // the usage must be empty
+    ProcMemInfo proc_mem(pid, true);
+    const MemUsage& usage = proc_mem.Usage();
+    EXPECT_EQ(usage.rss, 0);
+    EXPECT_EQ(usage.vss, 0);
+    EXPECT_EQ(usage.pss, 0);
+    EXPECT_EQ(usage.uss, 0);
+    EXPECT_EQ(usage.swap, 0);
+}
+
+TEST(TestProcMemInfoWssReset, TestWssEmpty) {
+    // If we created the object for getting usage,
+    // the working set must be empty
+    ProcMemInfo proc_mem(pid, false);
+    const MemUsage& wss = proc_mem.Wss();
+    EXPECT_EQ(wss.rss, 0);
+    EXPECT_EQ(wss.vss, 0);
+    EXPECT_EQ(wss.pss, 0);
+    EXPECT_EQ(wss.uss, 0);
+    EXPECT_EQ(wss.swap, 0);
+}
+
+TEST(TestProcMemInfoWssReset, TestSwapOffsetsEmpty) {
+    // If we created the object for getting working set,
+    // the swap offsets must be empty
+    ProcMemInfo proc_mem(pid, true);
+    const std::vector<uint16_t>& swap_offsets = proc_mem.SwapOffsets();
+    EXPECT_EQ(swap_offsets.size(), 0);
+}
+
 TEST(ValidateProcMemInfoFlags, TestPageFlags1) {
     // Create proc object using libpagemap
     pm_kernel_t* ker;
diff --git a/libmeminfo/procmeminfo.cpp b/libmeminfo/procmeminfo.cpp
index 378f923b3..8867e147f 100644
--- a/libmeminfo/procmeminfo.cpp
+++ b/libmeminfo/procmeminfo.cpp
@@ -64,13 +64,13 @@ bool ProcMemInfo::ResetWorkingSet(pid_t pid) {
 }
 
 ProcMemInfo::ProcMemInfo(pid_t pid, bool get_wss, uint64_t pgflags, uint64_t pgflags_mask)
-    : pid_(pid), get_wss_(get_wss), pgflags_(pgflags), pgflags_mask_(pgflags_mask) {
-    if (!ReadMaps(get_wss_)) {
+    : pid_(pid), get_wss_(get_wss), pgflags_(pgflags), pgflags_mask_(pgflags_mask) {}
+
+const std::vector<Vma>& ProcMemInfo::Maps() {
+    if (maps_.empty() && !ReadMaps(get_wss_)) {
         LOG(ERROR) << "Failed to read maps for Process " << pid_;
     }
-}
 
-const std::vector<Vma>& ProcMemInfo::Maps() {
     return maps_;
 }
 
@@ -78,7 +78,13 @@ const MemUsage& ProcMemInfo::Usage() {
     if (get_wss_) {
         LOG(WARNING) << "Trying to read process memory usage for " << pid_
                      << " using invalid object";
+        return usage_;
     }
+
+    if (maps_.empty() && !ReadMaps(get_wss_)) {
+        LOG(ERROR) << "Failed to get memory usage for Process " << pid_;
+    }
+
     return usage_;
 }
 
@@ -86,16 +92,39 @@ const MemUsage& ProcMemInfo::Wss() {
     if (!get_wss_) {
         LOG(WARNING) << "Trying to read process working set for " << pid_
                      << " using invalid object";
+        return wss_;
+    }
+
+    if (maps_.empty() && !ReadMaps(get_wss_)) {
+        LOG(ERROR) << "Failed to get working set for Process " << pid_;
     }
 
     return wss_;
 }
 
-const std::vector<uint16_t>& ProcMemInfo::SwapOffsets() const {
+const std::vector<uint16_t>& ProcMemInfo::SwapOffsets() {
+    if (get_wss_) {
+        LOG(WARNING) << "Trying to read process swap offsets for " << pid_
+                     << " using invalid object";
+        return swap_offsets_;
+    }
+
+    if (maps_.empty() && !ReadMaps(get_wss_)) {
+        LOG(ERROR) << "Failed to get swap offsets for Process " << pid_;
+    }
+
     return swap_offsets_;
 }
 
 bool ProcMemInfo::ReadMaps(bool get_wss) {
+    // Each object reads /proc/<pid>/maps only once. This is done to make sure programs that are
+    // running for the lifetime of the system can recycle the objects and don't have to
+    // unnecessarily retain and update this object in memory (which can get significantly large).
+    // E.g. A program that only needs to reset the working set will never all ->Maps(), ->Usage().
+    // E.g. A program that is monitoring smaps_rollup, may never call ->maps(), Usage(), so it
+    // doesn't make sense for us to parse and retain unnecessary memory accounting stats by default.
+    if (!maps_.empty()) return true;
+
     // parse and read /proc/<pid>/maps
     std::string maps_file = ::android::base::StringPrintf("/proc/%d/maps", pid_);
     if (!::android::procinfo::ReadMapFile(
@@ -164,6 +193,7 @@ bool ProcMemInfo::ReadVmaStats(int pagemap_fd, Vma& vma, bool get_wss) {
         uint64_t page_frame = PAGE_PFN(p);
         if (!pinfo.PageFlags(page_frame, &pg_flags[i])) {
             LOG(ERROR) << "Failed to get page flags for " << page_frame << " in process " << pid_;
+            swap_offsets_.clear();
             return false;
         }
 
@@ -172,6 +202,7 @@ bool ProcMemInfo::ReadVmaStats(int pagemap_fd, Vma& vma, bool get_wss) {
 
         if (!pinfo.PageMapCount(page_frame, &pg_counts[i])) {
             LOG(ERROR) << "Failed to get page count for " << page_frame << " in process " << pid_;
+            swap_offsets_.clear();
             return false;
         }