Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c6497eb1 authored by Sandeep Patil's avatar Sandeep Patil
Browse files

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: default avatarSandeep Patil <sspatil@google.com>
parent f1291993

libmeminfo/include/meminfo/procmeminfo.h

+1 −1
Original line number Diff line number Diff line
@@ -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;

libmeminfo/libmeminfo_test.cpp

+38 −0
Original line number Diff line number Diff line
@@ -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;

libmeminfo/procmeminfo.cpp

+36 −5
Original line number Diff line number Diff line
@@ -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;

        }