From 1be06760ed66ae86182e79347c95886a284bf4b8 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 | 15 +++++++++++++++
 libcontainer/cgroups/fs/util_test.go    |  4 ++--
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go
index d3bd7e111..69f8f9d8c 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 70b237a0b..d7d926d50 100644
--- a/libcontainer/cgroups/fs/cpuacct_test.go
+++ b/libcontainer/cgroups/fs/cpuacct_test.go
@@ -95,3 +95,18 @@ 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,
+	})
+
+	b.ResetTimer()
+	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 85842b735..87c70ed1d 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 {