Skip to content

Commit

Permalink
libcontainer/cgroups/fs: remove todo since strings.Fields performs well
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Stavrospanakakis committed Sep 18, 2024
1 parent 1590491 commit 4eef116
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 3 deletions.
2 changes: 1 addition & 1 deletion libcontainer/cgroups/fs/cpuacct.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions libcontainer/cgroups/fs/cpuacct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
4 changes: 2 additions & 2 deletions libcontainer/cgroups/fs/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit 4eef116

Please sign in to comment.