From 4eef1160232656e60b2fca0f425d253def66554d Mon Sep 17 00:00:00 2001
From: Stavros Panakakis <stavrospanakakis@gmail.com>
Date: Sun, 15 Sep 2024 17:00:34 +0300
Subject: [PATCH] libcontainer/cgroups/fs: remove todo since strings.Fields
 performs well

Initially, this was a commit to switch from strings.Fields to
strings.SplitN in getCpuUsageBreakdown, since strings.Fields
was probably slower than strings.SplitN in some old Go versions.

Afterwards, strings.Cut was also considered for potential
speed improvements.

After writing a benchmark test, we learned that:
 - strings.Fields performance is now adequate;
 - strings.SplitN is slower than strings.Fields;
 - strings.Cut had <5% performance gain from strings.Fields;

So, remove the TODO and keep the benchmark test.

Signed-off-by: Stavros Panakakis <stavrospanakakis@gmail.com>
---
 libcontainer/cgroups/fs/cpuacct.go      |  2 +-
 libcontainer/cgroups/fs/cpuacct_test.go | 14 ++++++++++++++
 libcontainer/cgroups/fs/util_test.go    |  4 ++--
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go
index d3bd7e111c7..69f8f9d8cdd 100644
--- a/libcontainer/cgroups/fs/cpuacct.go
+++ b/libcontainer/cgroups/fs/cpuacct.go
@@ -91,7 +91,7 @@ func getCpuUsageBreakdown(path string) (uint64, uint64, error) {
 	if err != nil {
 		return 0, 0, err
 	}
-	// TODO: use strings.SplitN instead.
+
 	fields := strings.Fields(data)
 	if len(fields) < 4 || fields[0] != userField || fields[2] != systemField {
 		return 0, 0, malformedLine(path, file, data)
diff --git a/libcontainer/cgroups/fs/cpuacct_test.go b/libcontainer/cgroups/fs/cpuacct_test.go
index 70b237a0b61..55d91848cbd 100644
--- a/libcontainer/cgroups/fs/cpuacct_test.go
+++ b/libcontainer/cgroups/fs/cpuacct_test.go
@@ -95,3 +95,17 @@ func TestCpuacctStatsWithoutUsageAll(t *testing.T) {
 			expectedStats, actualStats.CpuStats.CpuUsage)
 	}
 }
+
+func BenchmarkGetCpuUsageBreakdown(b *testing.B) {
+	path := tempDir(b, "cpuacct")
+	writeFileContents(b, path, map[string]string{
+		"cpuacct.stat": cpuAcctStatContents,
+	})
+
+	for i := 0; i < b.N; i++ {
+		_, _, err := getCpuUsageBreakdown(path)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
diff --git a/libcontainer/cgroups/fs/util_test.go b/libcontainer/cgroups/fs/util_test.go
index 85842b73532..87c70ed1d9d 100644
--- a/libcontainer/cgroups/fs/util_test.go
+++ b/libcontainer/cgroups/fs/util_test.go
@@ -18,7 +18,7 @@ func init() {
 }
 
 // tempDir creates a new test directory for the specified subsystem.
-func tempDir(t *testing.T, subsystem string) string {
+func tempDir(t testing.TB, subsystem string) string {
 	path := filepath.Join(t.TempDir(), subsystem)
 	// Ensure the full mock cgroup path exists.
 	if err := os.Mkdir(path, 0o755); err != nil {
@@ -29,7 +29,7 @@ func tempDir(t *testing.T, subsystem string) string {
 
 // writeFileContents writes the specified contents on the mock of the specified
 // cgroup files.
-func writeFileContents(t *testing.T, path string, fileContents map[string]string) {
+func writeFileContents(t testing.TB, path string, fileContents map[string]string) {
 	for file, contents := range fileContents {
 		err := cgroups.WriteFile(path, file, contents)
 		if err != nil {