Optimize code that only uses PageMap call.
Some code only wants to use PageMap to determine whether pages are
mapped in to memory. Modify the code to optimize this path.
Changes:
- Add a function that doesn't read all of usage stats data for every
map. This operation is extremely expensive, and some code doesn't
care about it.
- Optimize the PageMap call to do a single read instead of a single
read per page.
- Add unit tests for these changes.
Bug: 136245508
Test: Ran new unit tests.
Test: Ran procrank and verified data looks good.
Test: Ran DexDiag art tests.
Change-Id: I37d03f2584551d26cb20be3abacdb958111d4eca
diff --git a/libmeminfo/include/meminfo/procmeminfo.h b/libmeminfo/include/meminfo/procmeminfo.h
index 1fb4151..f782ec5 100644
--- a/libmeminfo/include/meminfo/procmeminfo.h
+++ b/libmeminfo/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/libmeminfo_test.cpp b/libmeminfo/libmeminfo_test.cpp
index 5451ca3..4c2be91 100644
--- a/libmeminfo/libmeminfo_test.cpp
+++ b/libmeminfo/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/libmeminfo/procmeminfo.cpp b/libmeminfo/procmeminfo.cpp
index a8b43c1..6f68ab4 100644
--- a/libmeminfo/procmeminfo.cpp
+++ b/libmeminfo/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)));
