libprocessgroup: optimize SetAttributeAction::ExecuteForProcess perfo…

…rmance 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 Merged-In: I02e3046bd85d0dfebc68ab444f1796bb54cc69c7 Signed-off-by: Suren Baghdasaryan <[email protected]>
StyxProject-Next · Jul 27, 2023 · 961c01c · 961c01c
1 parent 2ffbeae
commit 961c01c
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 12 deletions.
diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp
@@ -126,6 +126,16 @@ void ProfileAttribute::Reset(const CgroupController& controller, const std::stri
     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 @@ bool SetTimerSlackAction::ExecuteForTask(int) const {
 
 #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 @@ bool SetAttributeAction::ExecuteForTask(int tid) const {
     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;
 

diff --git a/libprocessgroup/task_profiles.h b/libprocessgroup/task_profiles.h
@@ -36,6 +36,7 @@ class IProfileAttribute {
                        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 @@ class ProfileAttribute : public IProfileAttribute {
     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 @@ class SetAttributeAction : public ProfileAction {
     const IProfileAttribute* attribute_;
     std::string value_;
     bool optional_;
+
+    bool WriteValueToFile(const std::string& path) const;
 };
 
 // Set cgroup profile element

diff --git a/libprocessgroup/task_profiles_test.cpp b/libprocessgroup/task_profiles_test.cpp
@@ -111,6 +111,9 @@ class ProfileAttributeMock : public IProfileAttribute {
         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));