Revert "Optimize pkg/util/cgroup code to allocate less frequently (#1…

…8816)" (#18895)

This reverts commit 2397c7c.

pkg/util/cgroups/cgroup_shared.go

@@ -8,8 +8,8 @@
 package cgroups
 
 import (
-	"bytes"
 	"strconv"
+	"strings"
 )
 
 const (
@@ -22,28 +22,20 @@ 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
 
-	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:]))
+	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])
 			numCPUs += uint64(p1 - p0 + 1)
-		} else {
-			numCPUs += 1
+		} else if len(lineParts) == 1 {
+			numCPUs++
 		}
 	}
+
 	return numCPUs
 }
 

pkg/util/cgroups/cgroupv1_cpu.go

@@ -42,12 +42,7 @@ func (c *cgroupV1) parseCPUController(stats *CPUStats) {
 		reportError(err)
 	}
 
-	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)
-
+	if err := parse2ColumnStats(c.fr, c.pathFor("cpu", "cpu.stat"), 0, 1, func(key, value string) error {
 		intVal, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
 			reportError(newValueError(value, err))
@@ -100,8 +95,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(lineRaw []byte) error {
-		cpuCount += ParseCPUSetFormat(string(lineRaw))
+	err := parseFile(c.fr, c.pathFor("cpuset", "cpuset.cpus"), func(line string) error {
+		cpuCount += ParseCPUSetFormat(line)
 		return nil
 	})
 
@@ -112,16 +107,11 @@ func (c *cgroupV1) parseCPUSetController(stats *CPUStats) {
 	}
 }
 
-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)
+func parseV1CPUAcctStatFn(stats *CPUStats) func(key, val string) error {
+	return func(key, val string) error {
+		intVal, err := strconv.ParseUint(val, 10, 64)
 		if err != nil {
-			reportError(newValueError(valString, err))
+			reportError(newValueError(val, err))
 			return nil
 		}
 

pkg/util/cgroups/cgroupv1_cpu_test.go

@@ -103,26 +103,3 @@ 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,11 +26,7 @@ func (c *cgroupV1) GetMemoryStats(stats *MemoryStats) error {
 		return &ControllerNotFoundError{Controller: "memory"}
 	}
 
-	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)
-
+	if err := parse2ColumnStats(c.fr, c.pathFor("memory", "memory.stat"), 0, 1, func(key, value string) error {
 		intVal, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
 			reportError(newValueError(value, err))

pkg/util/cgroups/cgroupv1_memory_test.go

@@ -117,29 +117,3 @@ 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(lineRaw []byte) error {
-		cpuCount += ParseCPUSetFormat(string(lineRaw))
+	err := parseFile(c.fr, c.pathFor("cpuset.cpus.effective"), func(line string) error {
+		cpuCount += ParseCPUSetFormat(line)
 		return nil
 	})
 
@@ -66,11 +66,8 @@ func (c *cgroupV2) parseCPUSetController(stats *CPUStats) {
 	}
 }
 
-func parseV2CPUStat(stats *CPUStats) func(keyRaw, valueRaw []byte) error {
-	return func(keyRaw, valueRaw []byte) error {
-		key := string(keyRaw)
-		value := string(valueRaw)
-
+func parseV2CPUStat(stats *CPUStats) func(key, value string) error {
+	return func(key, value string) error {
 		// Do not stop parsing the file if we cannot parse a single value
 		intVal, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
@@ -101,11 +98,8 @@ func parseV2CPUStat(stats *CPUStats) func(keyRaw, valueRaw []byte) error {
 	}
 }
 
-func parseV2CPUMax(stats *CPUStats) func(keyRaw, valueRaw []byte) error {
-	return func(limitRaw, periodRaw []byte) error {
-		period := string(periodRaw)
-		limit := string(limitRaw)
-
+func parseV2CPUMax(stats *CPUStats) func(key, value string) error {
+	return func(limit, period string) error {
 		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)([]byte(tt.inputKey), []byte(tt.inputVal))
+			err := parseV2CPUStat(&stats)(tt.inputKey, 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)([]byte(tt.inputKey), []byte(tt.inputVal))
+			err := parseV2CPUMax(&stats)(tt.inputKey, tt.inputVal)
 			assert.ErrorIs(t, err, tt.wantErr)
 			assert.Empty(t, cmp.Diff(tt.want, &stats))
 		})

pkg/util/cgroups/cgroupv2_io_test.go

@@ -0,0 +1,116 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2016-present Datadog, Inc.
+
+//go:build linux
+
+package cgroups
+
+import (
+	"testing"
+
+	"github.com/DataDog/datadog-agent/pkg/util/pointer"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
+)
+
+const (
+	sampleCgroupV2IOStat = `259:0 rbytes=278528 wbytes=11623899136 rios=6 wios=2744940 dbytes=0 dios=0
+8:16 rbytes=278528 wbytes=11623899136 rios=6 wios=2744940 dbytes=0 dios=0`
+	sampleCgroupV2IOMax     = "8:16 rbps=2097152 wbps=max riops=max wiops=120"
+	sampleCroupV2IOPressure = `some avg10=0.00 avg60=0.00 avg300=0.00 total=0
+full avg10=0.00 avg60=0.00 avg300=0.00 total=0`
+)
+
+func createCgroupV2FakeIOFiles(cfs *cgroupMemoryFS, cg *cgroupV2) {
+	cfs.setCgroupV2File(cg, "io.stat", sampleCgroupV2IOStat)
+	cfs.setCgroupV2File(cg, "io.max", sampleCgroupV2IOMax)
+	cfs.setCgroupV2File(cg, "io.pressure", sampleCroupV2IOPressure)
+}
+
+func TestCgroupV2IOStats(t *testing.T) {
+	cfs := newCgroupMemoryFS("/test/fs/cgroup")
+
+	var err error
+	stats := &IOStats{}
+
+	// Test failure if controller missing (io is missing)
+	tr.reset()
+	cgFoo1 := cfs.createCgroupV2("foo1", containerCgroupKubePod(true))
+	err = cgFoo1.GetIOStats(stats)
+	assert.ErrorIs(t, err, &ControllerNotFoundError{Controller: "io"})
+
+	// Test reading files in io controller, all files missing
+	tr.reset()
+	cfs.enableControllers("io")
+	err = cgFoo1.GetIOStats(stats)
+	assert.NoError(t, err)
+	assert.Equal(t, 3, len(tr.errors))
+	assert.Empty(t, cmp.Diff(IOStats{}, *stats))
+
+	// Test reading files in io controller, all files present
+	tr.reset()
+	createCgroupV2FakeIOFiles(cfs, cgFoo1)
+	err = cgFoo1.GetIOStats(stats)
+	assert.NoError(t, err)
+	assert.ElementsMatch(t, []error{}, tr.errors)
+	assert.Empty(t, cmp.Diff(IOStats{
+		ReadBytes:       pointer.Ptr(uint64(557056)),
+		WriteBytes:      pointer.Ptr(uint64(23247798272)),
+		ReadOperations:  pointer.Ptr(uint64(12)),
+		WriteOperations: pointer.Ptr(uint64(5489880)),
+		Devices: map[string]DeviceIOStats{
+			"259:0": {
+				ReadBytes:       pointer.Ptr(uint64(278528)),
+				WriteBytes:      pointer.Ptr(uint64(11623899136)),
+				ReadOperations:  pointer.Ptr(uint64(6)),
+				WriteOperations: pointer.Ptr(uint64(2744940)),
+			},
+			"8:16": {
+				ReadBytes:            pointer.Ptr(uint64(278528)),
+				WriteBytes:           pointer.Ptr(uint64(11623899136)),
+				ReadOperations:       pointer.Ptr(uint64(6)),
+				WriteOperations:      pointer.Ptr(uint64(2744940)),
+				ReadBytesLimit:       pointer.Ptr(uint64(2097152)),
+				WriteOperationsLimit: pointer.Ptr(uint64(120)),
+			},
+		},
+		PSISome: PSIStats{
+			Avg10:  pointer.Ptr(0.0),
+			Avg60:  pointer.Ptr(0.0),
+			Avg300: pointer.Ptr(0.0),
+			Total:  pointer.Ptr(uint64(0)),
+		},
+		PSIFull: PSIStats{
+			Avg10:  pointer.Ptr(0.0),
+			Avg60:  pointer.Ptr(0.0),
+			Avg300: pointer.Ptr(0.0),
+			Total:  pointer.Ptr(uint64(0)),
+		},
+	}, *stats))
+
+	// Test reading empty file
+	tr.reset()
+	stats = &IOStats{}
+	cfs.setCgroupV2File(cgFoo1, "io.stat", "")
+	cfs.setCgroupV2File(cgFoo1, "io.max", "")
+	err = cgFoo1.GetIOStats(stats)
+	assert.NoError(t, err)
+	assert.ElementsMatch(t, []error{}, tr.errors)
+	assert.Empty(t, cmp.Diff(IOStats{
+		PSISome: PSIStats{
+			Avg10:  pointer.Ptr(0.0),
+			Avg60:  pointer.Ptr(0.0),
+			Avg300: pointer.Ptr(0.0),
+			Total:  pointer.Ptr(uint64(0)),
+		},
+		PSIFull: PSIStats{
+			Avg10:  pointer.Ptr(0.0),
+			Avg60:  pointer.Ptr(0.0),
+			Avg300: pointer.Ptr(0.0),
+			Total:  pointer.Ptr(uint64(0)),
+		},
+	}, *stats))
+}

pkg/util/cgroups/cgroupv2_memory.go

@@ -24,10 +24,7 @@ func (c *cgroupV2) GetMemoryStats(stats *MemoryStats) error {
 
 	var kernelStack, slab *uint64
 
-	if err := parse2ColumnStats(c.fr, c.pathFor("memory.stat"), 0, 1, func(keyRaw, valueRaw []byte) error {
-		key := string(keyRaw)
-		value := string(valueRaw)
-
+	if err := parse2ColumnStats(c.fr, c.pathFor("memory.stat"), 0, 1, func(key, value string) error {
 		intVal, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
 			reportError(newValueError(value, err))
@@ -116,11 +113,7 @@ func (c *cgroupV2) GetMemoryStats(stats *MemoryStats) error {
 	}
 	nilIfZero(&stats.SwapLimit)
 
-	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)
-
+	if err := parse2ColumnStats(c.fr, c.pathFor("memory.events"), 0, 1, func(key, value string) error {
 		intVal, err := strconv.ParseUint(value, 10, 64)
 		if err != nil {
 			reportError(newValueError(value, err))

pkg/util/cgroups/errors.go

@@ -42,6 +42,13 @@ 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())
 }