commit 3483798fd9045f93e9095e5b09ffe4f59054c535
author Suren Baghdasaryan <surenb@google.com> Tue Jul 25 15:45:45 2023 -0700
committer Suren Baghdasaryan <surenb@google.com> Thu Jul 27 21:55:48 2023 +0000
tree 39fd49dec3c955608921ef536d0cc36062f23514
parent 35078468722cdee94fe17e57fe5f300d3ee156f4

libprocessgroup: optimize SetAttributeAction::ExecuteForProcess performance

Current implementation of SetAttributeAction::ExecuteForProcess reuses
SetAttributeAction::ExecuteForTask while not utilizing available uid/pid
information. This results in a call to GetPathForTask() which is an
expensive function due to it reading and parsing /proc/$pid/cgroups.
This can be avoided if we utilize available uid/pid info and the fact
that cgroup v2 attributes share the cgroup v2 hierarchy as process
groups, which use a known path template.

Bug: 292636609
Change-Id: I02e3046bd85d0dfebc68ab444f1796bb54cc69c7
Signed-off-by: Suren Baghdasaryan <surenb@google.com>

libprocessgroup/task_profiles.cpp

diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp
index 59b1350..f51b076 100644
--- a/libprocessgroup/task_profiles.cpp
+++ b/libprocessgroup/task_profiles.cpp
@@ -126,6 +126,16 @@
     file_v2_name_ = file_v2_name;
 }
 
+bool ProfileAttribute::GetPathForProcess(uid_t uid, pid_t pid, std::string* path) const {
+    if (controller()->version() == 2) {
+        // all cgroup v2 attributes use the same process group hierarchy
+        *path = StringPrintf("%s/uid_%u/pid_%d/%s", controller()->path(), uid, pid,
+                             file_name().c_str());
+        return true;
+    }
+    return GetPathForTask(pid, path);
+}
+
 bool ProfileAttribute::GetPathForTask(int tid, std::string* path) const {
     std::string subgroup;
     if (!controller()->GetTaskGroup(tid, &subgroup)) {
@@ -209,18 +219,7 @@
 
 #endif
 
-bool SetAttributeAction::ExecuteForProcess(uid_t, pid_t pid) const {
-    return ExecuteForTask(pid);
-}
-
-bool SetAttributeAction::ExecuteForTask(int tid) const {
-    std::string path;
-
-    if (!attribute_->GetPathForTask(tid, &path)) {
-        LOG(ERROR) << "Failed to find cgroup for tid " << tid;
-        return false;
-    }
-
+bool SetAttributeAction::WriteValueToFile(const std::string& path) const {
     if (!WriteStringToFile(value_, path)) {
         if (access(path.c_str(), F_OK) < 0) {
             if (optional_) {
@@ -240,6 +239,28 @@
     return true;
 }
 
+bool SetAttributeAction::ExecuteForProcess(uid_t uid, pid_t pid) const {
+    std::string path;
+
+    if (!attribute_->GetPathForProcess(uid, pid, &path)) {
+        LOG(ERROR) << "Failed to find cgroup for uid " << uid << " pid " << pid;
+        return false;
+    }
+
+    return WriteValueToFile(path);
+}
+
+bool SetAttributeAction::ExecuteForTask(int tid) const {
+    std::string path;
+
+    if (!attribute_->GetPathForTask(tid, &path)) {
+        LOG(ERROR) << "Failed to find cgroup for tid " << tid;
+        return false;
+    }
+
+    return WriteValueToFile(path);
+}
+
 bool SetAttributeAction::ExecuteForUID(uid_t uid) const {
     std::string path;
 

libprocessgroup/task_profiles.h

diff --git a/libprocessgroup/task_profiles.h b/libprocessgroup/task_profiles.h
index ac8918e..4663f64 100644
--- a/libprocessgroup/task_profiles.h
+++ b/libprocessgroup/task_profiles.h
@@ -36,6 +36,7 @@
                        const std::string& file_v2_name) = 0;
     virtual const CgroupController* controller() const = 0;
     virtual const std::string& file_name() const = 0;
+    virtual bool GetPathForProcess(uid_t uid, pid_t pid, std::string* path) const = 0;
     virtual bool GetPathForTask(int tid, std::string* path) const = 0;
     virtual bool GetPathForUID(uid_t uid, std::string* path) const = 0;
 };
@@ -55,6 +56,7 @@
     void Reset(const CgroupController& controller, const std::string& file_name,
                const std::string& file_v2_name) override;
 
+    bool GetPathForProcess(uid_t uid, pid_t pid, std::string* path) const override;
     bool GetPathForTask(int tid, std::string* path) const override;
     bool GetPathForUID(uid_t uid, std::string* path) const override;
 
@@ -133,6 +135,8 @@
     const IProfileAttribute* attribute_;
     std::string value_;
     bool optional_;
+
+    bool WriteValueToFile(const std::string& path) const;
 };
 
 // Set cgroup profile element

libprocessgroup/task_profiles_test.cpp

diff --git a/libprocessgroup/task_profiles_test.cpp b/libprocessgroup/task_profiles_test.cpp
index da74bb0..99d819a 100644
--- a/libprocessgroup/task_profiles_test.cpp
+++ b/libprocessgroup/task_profiles_test.cpp
@@ -111,6 +111,9 @@
         return {};
     }
     const std::string& file_name() const override { return file_name_; }
+    bool GetPathForProcess(uid_t uid, pid_t pid, std::string* path) const override {
+        return GetPathForTask(pid, path);
+    }
     bool GetPathForTask(int tid, std::string* path) const override {
 #ifdef __ANDROID__
         CHECK(CgroupGetControllerPath(CGROUPV2_CONTROLLER_NAME, path));