commit 1243ddeac9e35eaf1ca8f665eff24a89226b2f01
author Yifan Hong <elsk@google.com> Thu Sep 14 17:49:30 2017 -0700
committer Yifan Hong <elsk@google.com> Thu Sep 14 19:16:18 2017 -0700
tree a750576eadeed74049cb554313d9916441168496
parent 7a3b46c2f5622759aeffaf4c2f0f867973469efd

lshal: ListCommand::getPidInfo should be cached.

The function parses a big file and its result should be cached.
Add ListCommand::getPidInfoCached, which is a cached version
of this function.

Test: lshal_test
Change-Id: I03d8f149bf67b1512be431a3c4c29e83f7743a2f

cmds/lshal/ListCommand.cpp

diff --git a/cmds/lshal/ListCommand.cpp b/cmds/lshal/ListCommand.cpp
index 8b59fb8..509353b 100644
--- a/cmds/lshal/ListCommand.cpp
+++ b/cmds/lshal/ListCommand.cpp
@@ -85,7 +85,7 @@
     }), pids->end());
 }
 
-bool scanBinderContext(pid_t pid,
+static bool scanBinderContext(pid_t pid,
         const std::string &contextName,
         std::function<void(const std::string&)> eachLine) {
     std::ifstream ifs("/d/binder/proc/" + std::to_string(pid));
@@ -171,6 +171,16 @@
     });
 }
 
+const PidInfo* ListCommand::getPidInfoCached(pid_t serverPid) {
+    auto pair = mCachedPidInfos.insert({serverPid, PidInfo{}});
+    if (pair.second /* did insertion take place? */) {
+        if (!getPidInfo(serverPid, &pair.first->second)) {
+            return nullptr;
+        }
+    }
+    return &pair.first->second;
+}
+
 // Must process hwbinder services first, then passthrough services.
 void ListCommand::forEachTable(const std::function<void(Table &)> &f) {
     f(mServicesTable);

cmds/lshal/ListCommand.h

diff --git a/cmds/lshal/ListCommand.h b/cmds/lshal/ListCommand.h
index 049533a..5af7ec2 100644
--- a/cmds/lshal/ListCommand.h
+++ b/cmds/lshal/ListCommand.h
@@ -85,7 +85,11 @@
     Status fetchBinderized(const sp<::android::hidl::manager::V1_0::IServiceManager> &manager);
     Status fetchAllLibraries(const sp<::android::hidl::manager::V1_0::IServiceManager> &manager);
 
+    // Get relevant information for a PID by parsing files under /d/binder.
+    // It is a virtual member function so that it can be mocked.
     virtual bool getPidInfo(pid_t serverPid, PidInfo *info) const;
+    // Retrieve from mCachedPidInfos and call getPidInfo if necessary.
+    const PidInfo* getPidInfoCached(pid_t serverPid);
 
     void dumpTable(const NullableOStream<std::ostream>& out) const;
     void dumpVintf(const NullableOStream<std::ostream>& out) const;
@@ -129,6 +133,9 @@
     // If an entry exist and not empty, it contains the cached content of /proc/{pid}/cmdline.
     std::map<pid_t, std::string> mCmdlines;
 
+    // Cache for getPidInfo.
+    std::map<pid_t, PidInfo> mCachedPidInfos;
+
     RegisteredOptions mOptions;
     // All selected columns
     std::vector<TableColumnType> mSelectedColumns;

cmds/lshal/test.cpp

diff --git a/cmds/lshal/test.cpp b/cmds/lshal/test.cpp
index bb23b2c..7d87d6d 100644
--- a/cmds/lshal/test.cpp
+++ b/cmds/lshal/test.cpp
@@ -196,6 +196,9 @@
         return ListCommand::dumpVintf(out);
     }
     void internalPostprocess() { ListCommand::postprocess(); }
+    const PidInfo* getPidInfoCached(pid_t serverPid) {
+        return ListCommand::getPidInfoCached(serverPid);
+    }
 
     MOCK_METHOD0(postprocess, void());
     MOCK_CONST_METHOD2(getPidInfo, bool(pid_t, PidInfo*));
@@ -355,6 +358,13 @@
     sp<MockServiceManager> passthruManager;
 };
 
+TEST_F(ListTest, GetPidInfoCached) {
+    EXPECT_CALL(*mockList, getPidInfo(5, _)).Times(1);
+
+    EXPECT_NE(nullptr, mockList->getPidInfoCached(5));
+    EXPECT_NE(nullptr, mockList->getPidInfoCached(5));
+}
+
 TEST_F(ListTest, Fetch) {
     EXPECT_EQ(0u, mockList->fetch());
     std::array<std::string, 6> transports{{"hwbinder", "hwbinder", "passthrough",