From 1e81ee13638c5ca03157db674009d4cf7c2a1263 Mon Sep 17 00:00:00 2001
From: Suren Baghdasaryan <surenb@google.com>
Date: Tue, 25 Jul 2023 15:45:45 -0700
Subject: [PATCH] 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
Merged-In: I02e3046bd85d0dfebc68ab444f1796bb54cc69c7
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
(cherry picked from commit 961c01ce23bb886583ca8cac1640806346c09a7f)
---
 libprocessgroup/task_profiles.cpp      | 45 +++++++++++++++++++-------
 libprocessgroup/task_profiles.h        |  4 +++
 libprocessgroup/task_profiles_test.cpp |  3 ++
 3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp
index 59b135013..f51b07671 100644
--- 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
index ac8918e65..4663f64e2 100644
--- 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
index da74bb012..99d819a7c 100644
--- 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));