Merge "Optimize code that only uses PageMap call." am: 1e7b753c36 am: bdcd4f8e85
am: 7a7b844499

Change-Id: Ia88d5a89f1934ed2666bdd5e080a436a644002f8
diff --git a/include/meminfo/procmeminfo.h b/include/meminfo/procmeminfo.h
index 1fb4151..f782ec5 100644
--- a/include/meminfo/procmeminfo.h
+++ b/include/meminfo/procmeminfo.h
@@ -45,6 +45,9 @@
     // vector.
     const std::vector<Vma>& MapsWithPageIdle();
 
+    // Same as Maps() except, do not read the usage stats for each map.
+    const std::vector<Vma>& MapsWithoutUsageStats();
+
     // Collect all 'vma' or 'maps' from /proc/<pid>/smaps and store them in 'maps_'. Returns a
     // constant reference to the vma vector after the collection is done.
     //
@@ -88,7 +91,7 @@
     ~ProcMemInfo() = default;
 
   private:
-    bool ReadMaps(bool get_wss, bool use_pageidle = false);
+    bool ReadMaps(bool get_wss, bool use_pageidle = false, bool get_usage_stats = true);
     bool ReadVmaStats(int pagemap_fd, Vma& vma, bool get_wss, bool use_pageidle);
 
     pid_t pid_;
diff --git a/libmeminfo_test.cpp b/libmeminfo_test.cpp
index 5451ca3..4c2be91 100644
--- a/libmeminfo_test.cpp
+++ b/libmeminfo_test.cpp
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
@@ -60,6 +61,103 @@
     EXPECT_FALSE(maps.empty());
 }
 
+TEST(ProcMemInfo, MapsUsageNotEmpty) {
+    ProcMemInfo proc_mem(pid);
+    const std::vector<Vma>& maps = proc_mem.Maps();
+    EXPECT_FALSE(maps.empty());
+    uint64_t total_pss = 0;
+    uint64_t total_rss = 0;
+    uint64_t total_uss = 0;
+    for (auto& map : maps) {
+        ASSERT_NE(0, map.usage.vss);
+        total_rss += map.usage.rss;
+        total_pss += map.usage.pss;
+        total_uss += map.usage.uss;
+    }
+
+    // Crude check that stats are actually being read.
+    EXPECT_NE(0, total_rss) << "RSS zero for all maps, that is not possible.";
+    EXPECT_NE(0, total_pss) << "PSS zero for all maps, that is not possible.";
+    EXPECT_NE(0, total_uss) << "USS zero for all maps, that is not possible.";
+}
+
+TEST(ProcMemInfo, MapsUsageEmpty) {
+    ProcMemInfo proc_mem(pid);
+    const std::vector<Vma>& maps = proc_mem.MapsWithoutUsageStats();
+    EXPECT_FALSE(maps.empty());
+    // Verify that all usage stats are zero in every map.
+    for (auto& map : maps) {
+        ASSERT_EQ(0, map.usage.vss);
+        ASSERT_EQ(0, map.usage.rss);
+        ASSERT_EQ(0, map.usage.pss);
+        ASSERT_EQ(0, map.usage.uss);
+        ASSERT_EQ(0, map.usage.swap);
+        ASSERT_EQ(0, map.usage.swap_pss);
+        ASSERT_EQ(0, map.usage.private_clean);
+        ASSERT_EQ(0, map.usage.private_dirty);
+        ASSERT_EQ(0, map.usage.shared_clean);
+        ASSERT_EQ(0, map.usage.shared_dirty);
+    }
+}
+
+TEST(ProcMemInfo, PageMapPresent) {
+    static constexpr size_t kNumPages = 20;
+    size_t pagesize = getpagesize();
+    void* ptr = mmap(nullptr, pagesize * (kNumPages + 2), PROT_READ | PROT_WRITE,
+                     MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    ASSERT_NE(MAP_FAILED, ptr);
+
+    // Unmap the first page and the last page so that we guarantee this
+    // map is in a map by itself.
+    ASSERT_EQ(0, munmap(ptr, pagesize));
+    uintptr_t addr = reinterpret_cast<uintptr_t>(ptr) + pagesize;
+    ASSERT_EQ(0, munmap(reinterpret_cast<void*>(addr + kNumPages * pagesize), pagesize));
+
+    ProcMemInfo proc_mem(getpid());
+    const std::vector<Vma>& maps = proc_mem.MapsWithoutUsageStats();
+    ASSERT_FALSE(maps.empty());
+
+    // Find the vma associated with our previously created map.
+    const Vma* test_vma = nullptr;
+    for (const Vma& vma : maps) {
+        if (vma.start == addr) {
+            test_vma = &vma;
+            break;
+        }
+    }
+    ASSERT_TRUE(test_vma != nullptr) << "Cannot find test map.";
+
+    // Verify that none of the pages are listed as present.
+    std::vector<uint64_t> pagemap;
+    ASSERT_TRUE(proc_mem.PageMap(*test_vma, &pagemap));
+    ASSERT_EQ(kNumPages, pagemap.size());
+    for (size_t i = 0; i < pagemap.size(); i++) {
+        EXPECT_FALSE(android::meminfo::page_present(pagemap[i]))
+                << "Page " << i << " is present and it should not be.";
+    }
+
+    // Make some of the pages present and verify that we see them
+    // as present.
+    uint8_t* data = reinterpret_cast<uint8_t*>(addr);
+    data[0] = 1;
+    data[pagesize * 5] = 1;
+    data[pagesize * 11] = 1;
+
+    ASSERT_TRUE(proc_mem.PageMap(*test_vma, &pagemap));
+    ASSERT_EQ(kNumPages, pagemap.size());
+    for (size_t i = 0; i < pagemap.size(); i++) {
+        if (i == 0 || i == 5 || i == 11) {
+            EXPECT_TRUE(android::meminfo::page_present(pagemap[i]))
+                    << "Page " << i << " is not present and it should be.";
+        } else {
+            EXPECT_FALSE(android::meminfo::page_present(pagemap[i]))
+                    << "Page " << i << " is present and it should not be.";
+        }
+    }
+
+    ASSERT_EQ(0, munmap(reinterpret_cast<void*>(addr), kNumPages * pagesize));
+}
+
 TEST(ProcMemInfo, WssEmpty) {
     // If we created the object for getting usage,
     // the working set must be empty
diff --git a/procmeminfo.cpp b/procmeminfo.cpp
index a8b43c1..6f68ab4 100644
--- a/procmeminfo.cpp
+++ b/procmeminfo.cpp
@@ -130,6 +130,14 @@
     return maps_;
 }
 
+const std::vector<Vma>& ProcMemInfo::MapsWithoutUsageStats() {
+    if (maps_.empty() && !ReadMaps(get_wss_, false, false)) {
+        LOG(ERROR) << "Failed to read maps for Process " << pid_;
+    }
+
+    return maps_;
+}
+
 const std::vector<Vma>& ProcMemInfo::Smaps(const std::string& path) {
     if (!maps_.empty()) {
         return maps_;
@@ -213,29 +221,30 @@
     std::string pagemap_file = ::android::base::StringPrintf("/proc/%d/pagemap", pid_);
     ::android::base::unique_fd pagemap_fd(
             TEMP_FAILURE_RETRY(open(pagemap_file.c_str(), O_RDONLY | O_CLOEXEC)));
-    if (pagemap_fd < 0) {
+    if (pagemap_fd == -1) {
         PLOG(ERROR) << "Failed to open " << pagemap_file;
         return false;
     }
 
     uint64_t nr_pages = (vma.end - vma.start) / getpagesize();
-    pagemap->reserve(nr_pages);
+    pagemap->resize(nr_pages);
 
-    uint64_t idx = vma.start / getpagesize();
-    uint64_t last = idx + nr_pages;
-    uint64_t val;
-    for (; idx < last; idx++) {
-        if (pread64(pagemap_fd, &val, sizeof(uint64_t), idx * sizeof(uint64_t)) < 0) {
-            PLOG(ERROR) << "Failed to read page frames from page map for pid: " << pid_;
-            return false;
-        }
-        pagemap->emplace_back(val);
+    size_t bytes_to_read = sizeof(uint64_t) * nr_pages;
+    off64_t start_addr = (vma.start / getpagesize()) * sizeof(uint64_t);
+    ssize_t bytes_read = pread64(pagemap_fd, pagemap->data(), bytes_to_read, start_addr);
+    if (bytes_read == -1) {
+        PLOG(ERROR) << "Failed to read page frames from page map for pid: " << pid_;
+        return false;
+    } else if (static_cast<size_t>(bytes_read) != bytes_to_read) {
+        LOG(ERROR) << "Failed to read page frames from page map for pid: " << pid_
+                   << ": read bytes " << bytes_read << " expected bytes " << bytes_to_read;
+        return false;
     }
 
     return true;
 }
 
-bool ProcMemInfo::ReadMaps(bool get_wss, bool use_pageidle) {
+bool ProcMemInfo::ReadMaps(bool get_wss, bool use_pageidle, bool get_usage_stats) {
     // 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).
@@ -256,6 +265,10 @@
         return false;
     }
 
+    if (!get_usage_stats) {
+        return true;
+    }
+
     std::string pagemap_file = ::android::base::StringPrintf("/proc/%d/pagemap", pid_);
     ::android::base::unique_fd pagemap_fd(
             TEMP_FAILURE_RETRY(open(pagemap_file.c_str(), O_RDONLY | O_CLOEXEC)));