Restore "Optimize pkg/util/cgroup code to allocate less frequently"

Originally in PR #18816

This reverts commit 2e8f8b1.

pkg/util/cgroups/cgroup_shared.go

@@ -8,8 +8,8 @@
 package cgroups
 
 import (
+	"bytes"
 	"strconv"
-	"strings"
 )
 
 const (
@@ -22,20 +22,28 @@ const (
 // So "0,1,5-8" represents processors 0, 1, 5, 6, 7, 8.
 // The function returns the count of CPUs, in this case 6.
 func ParseCPUSetFormat(line string) uint64 {
+	lineRaw := []byte(line)
 	var numCPUs uint64
 
-	lineSlice := strings.Split(line, ",")
-	for _, l := range lineSlice {
-		lineParts := strings.Split(l, "-")
-		if len(lineParts) == 2 {
-			p0, _ := strconv.Atoi(lineParts[0])
-			p1, _ := strconv.Atoi(lineParts[1])
+	var currentSegment []byte
+	for len(lineRaw) != 0 {
+		nextStart := bytes.IndexByte(lineRaw, ',')
+		if nextStart == -1 {
+			currentSegment = lineRaw
+			lineRaw = nil
+		} else {
+			currentSegment = lineRaw[:nextStart]
+			lineRaw = lineRaw[nextStart+1:]
+		}
+
+		if split := bytes.IndexByte(currentSegment, '-'); split != -1 {
+			p0, _ := strconv.Atoi(string(currentSegment[:split]))
+			p1, _ := strconv.Atoi(string(currentSegment[split+1:]))
 			numCPUs += uint64(p1 - p0 + 1)
-		} else if len(lineParts) == 1 {
-			numCPUs++
+		} else {
+			numCPUs += 1
 		}
 	}
-
 	return numCPUs
 }
 

pkg/util/cgroups/cgroupv1_cpu.go

@@ -42,7 +42,12 @@ func (c *cgroupV1) parseCPUController(stats *CPUStats) {
 		reportError(err)
 	}
 
-	if err := parse2ColumnStats(c.fr, c.pathFor("cpu", "cpu.stat"), 0, 1, func(key, value string) error {
+	if err := parse2ColumnStats(c.fr, c.pathFor("cpu", "cpu.stat"), 0, 1, func(keyRaw, valueRaw []byte) error {
+
+		// the go compiler will avoid a copy here
+		key := string(keyRaw)
+		value := string(valueRaw)
+
 		intVal, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
 			reportError(newValueError(value, err))
@@ -95,8 +100,8 @@ func (c *cgroupV1) parseCPUAcctController(stats *CPUStats) {
 func (c *cgroupV1) parseCPUSetController(stats *CPUStats) {
 	// Normally there's only one line, but as the parser works line by line anyway, we do support multiple lines
 	var cpuCount uint64
-	err := parseFile(c.fr, c.pathFor("cpuset", "cpuset.cpus"), func(line string) error {
-		cpuCount += ParseCPUSetFormat(line)
+	err := parseFile(c.fr, c.pathFor("cpuset", "cpuset.cpus"), func(lineRaw []byte) error {
+		cpuCount += ParseCPUSetFormat(string(lineRaw))
 		return nil
 	})
 
@@ -107,11 +112,16 @@ func (c *cgroupV1) parseCPUSetController(stats *CPUStats) {
 	}
 }
 
-func parseV1CPUAcctStatFn(stats *CPUStats) func(key, val string) error {
-	return func(key, val string) error {
-		intVal, err := strconv.ParseUint(val, 10, 64)
+func parseV1CPUAcctStatFn(stats *CPUStats) func(keyRaw, valRaw []byte) error {
+	return func(keyRaw, valRaw []byte) error {
+
+		// the go compiler will avoid a copy here
+		key := string(keyRaw)
+		valString := string(valRaw)
+
+		intVal, err := strconv.ParseUint(valString, 10, 64)
 		if err != nil {
-			reportError(newValueError(val, err))
+			reportError(newValueError(valString, err))
 			return nil
 		}
 

pkg/util/cgroups/cgroupv1_cpu_test.go

@@ -103,3 +103,26 @@ func TestCgroupV1CPUStats(t *testing.T) {
 		CPUCount:         pointer.Ptr(uint64(8)),
 	}, *stats))
 }
+
+func BenchmarkGetCPUStatsV1(b *testing.B) {
+	cfs := newCgroupMemoryFS("/test/fs/cgroup")
+	cfs.enableControllers("cpu")
+	cfs.enableControllers("cpuacct", "cpuset")
+
+	stats := &CPUStats{}
+	// Test failure if controller missing (cpu is missing)
+	cgFoo1 := cfs.createCgroupV1("foo1", containerCgroupKubePod(false))
+
+	// Test reading files in CPU controllers, all files present
+	createCgroupV1FakeCPUFiles(cfs, cgFoo1)
+
+	b.ResetTimer()
+	b.ReportAllocs()
+	for i := 0; i < b.N; i++ {
+		err := cgFoo1.GetCPUStats(stats)
+		if err != nil {
+			b.Errorf("error in GetCPUStats: %v", err)
+			b.FailNow()
+		}
+	}
+}

pkg/util/cgroups/cgroupv1_memory.go

@@ -26,7 +26,11 @@ func (c *cgroupV1) GetMemoryStats(stats *MemoryStats) error {
 		return &ControllerNotFoundError{Controller: "memory"}
 	}
 
-	if err := parse2ColumnStats(c.fr, c.pathFor("memory", "memory.stat"), 0, 1, func(key, value string) error {
+	if err := parse2ColumnStats(c.fr, c.pathFor("memory", "memory.stat"), 0, 1, func(keyRaw, valueRaw []byte) error {
+		// the go compiler is smart enough to not turn this into a copy
+		key := string(keyRaw)
+		value := string(valueRaw)
+
 		intVal, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
 			reportError(newValueError(value, err))

pkg/util/cgroups/cgroupv1_memory_test.go

@@ -117,3 +117,29 @@ func TestCgroupV1MemoryStats(t *testing.T) {
 		Peak:         pointer.Ptr(uint64(400000000)),
 	}, *stats))
 }
+
+func BenchmarkMemory(b *testing.B) {
+	cfs := newCgroupMemoryFS("/test/fs/cgroup")
+
+	var err error
+	stats := &MemoryStats{}
+
+	// Test failure if controller missing (memory is missing)
+	cgFoo1 := cfs.createCgroupV1("foo1", containerCgroupKubePod(false))
+
+	// Test reading files in memory controller, all files missing (memsw not counted as considered optional)
+	tr.reset()
+	cfs.enableControllers("memory")
+
+	// Test reading files in memory controller, all files present
+	createCgroupV1FakeMemoryFiles(cfs, cgFoo1)
+
+	b.ReportAllocs()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		err = cgFoo1.GetMemoryStats(stats)
+		if err != nil {
+			b.Errorf("error getting memory stats: %v", err)
+		}
+	}
+}

pkg/util/cgroups/cgroupv2_cpu.go

@@ -54,8 +54,8 @@ func (c *cgroupV2) parseCPUController(stats *CPUStats) {
 func (c *cgroupV2) parseCPUSetController(stats *CPUStats) {
 	// Normally there's only one line, but as the parser works line by line anyway, we do support multiple lines
 	var cpuCount uint64
-	err := parseFile(c.fr, c.pathFor("cpuset.cpus.effective"), func(line string) error {
-		cpuCount += ParseCPUSetFormat(line)
+	err := parseFile(c.fr, c.pathFor("cpuset.cpus.effective"), func(lineRaw []byte) error {
+		cpuCount += ParseCPUSetFormat(string(lineRaw))
 		return nil
 	})
 
@@ -66,8 +66,11 @@ func (c *cgroupV2) parseCPUSetController(stats *CPUStats) {
 	}
 }
 
-func parseV2CPUStat(stats *CPUStats) func(key, value string) error {
-	return func(key, value string) error {
+func parseV2CPUStat(stats *CPUStats) func(keyRaw, valueRaw []byte) error {
+	return func(keyRaw, valueRaw []byte) error {
+		key := string(keyRaw)
+		value := string(valueRaw)
+
 		// Do not stop parsing the file if we cannot parse a single value
 		intVal, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
@@ -98,8 +101,11 @@ func parseV2CPUStat(stats *CPUStats) func(key, value string) error {
 	}
 }
 
-func parseV2CPUMax(stats *CPUStats) func(key, value string) error {
-	return func(limit, period string) error {
+func parseV2CPUMax(stats *CPUStats) func(keyRaw, valueRaw []byte) error {
+	return func(limitRaw, periodRaw []byte) error {
+		period := string(periodRaw)
+		limit := string(limitRaw)
+
 		periodVal, err := strconv.ParseUint(period, 10, 64)
 		if err == nil {
 			stats.SchedulerPeriod = pointer.Ptr(periodVal * uint64(time.Microsecond))

pkg/util/cgroups/cgroupv2_cpu_test.go

@@ -146,7 +146,7 @@ func TestParseV2CPUStat(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			stats := CPUStats{}
-			err := parseV2CPUStat(&stats)(tt.inputKey, tt.inputVal)
+			err := parseV2CPUStat(&stats)([]byte(tt.inputKey), []byte(tt.inputVal))
 			assert.ErrorIs(t, err, tt.wantErr)
 			assert.Empty(t, cmp.Diff(tt.want, &stats))
 		})
@@ -191,7 +191,7 @@ func TestParseV2CPUMax(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			stats := CPUStats{}
-			err := parseV2CPUMax(&stats)(tt.inputKey, tt.inputVal)
+			err := parseV2CPUMax(&stats)([]byte(tt.inputKey), []byte(tt.inputVal))
 			assert.ErrorIs(t, err, tt.wantErr)
 			assert.Empty(t, cmp.Diff(tt.want, &stats))
 		})

pkg/util/cgroups/cgroupv2_memory.go

@@ -24,7 +24,10 @@ func (c *cgroupV2) GetMemoryStats(stats *MemoryStats) error {
 
 	var kernelStack, slab *uint64
 
-	if err := parse2ColumnStats(c.fr, c.pathFor("memory.stat"), 0, 1, func(key, value string) error {
+	if err := parse2ColumnStats(c.fr, c.pathFor("memory.stat"), 0, 1, func(keyRaw, valueRaw []byte) error {
+		key := string(keyRaw)
+		value := string(valueRaw)
+
 		intVal, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
 			reportError(newValueError(value, err))
@@ -113,7 +116,11 @@ func (c *cgroupV2) GetMemoryStats(stats *MemoryStats) error {
 	}
 	nilIfZero(&stats.SwapLimit)
 
-	if err := parse2ColumnStats(c.fr, c.pathFor("memory.events"), 0, 1, func(key, value string) error {
+	if err := parse2ColumnStats(c.fr, c.pathFor("memory.events"), 0, 1, func(keyRaw, valueRaw []byte) error {
+		// the go compiler will avoid a copy here
+		key := string(keyRaw)
+		value := string(valueRaw)
+
 		intVal, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
 			reportError(newValueError(value, err))

pkg/util/cgroups/errors.go

@@ -42,13 +42,6 @@ type FileSystemError struct {
 	Err      error
 }
 
-func newFileSystemError(path string, err error) *FileSystemError {
-	return &FileSystemError{
-		FilePath: path,
-		Err:      err,
-	}
-}
-
 func (e *FileSystemError) Error() string {
 	return fmt.Sprintf("fs error, path: %s, err: %s", e.FilePath, e.Err.Error())
 }