Cgroups2: Reduce allocations for manager.Stat

This change works towards bringing down allocations for manager.Stat(). There's
quite a few things we can do:

1. Swap to opening a file and issuing a single read for uint64/"max" values.
Previously we were doing os.ReadFile's which returns a byte slice, so it needs
to allocate.
2. Sizing the map we store {controller}.stat values in. We store 40+ stats in this
map and the default bucket size for Go seems to be smaller than this, so we'd incur
a couple allocs at runtime when adding these.
3. Rework the type of map we stored these values in. We were using map[string]interface{}
as some of the files can have a string of "max" inside, but we can be a little smarter
about when to try converting to a uint64 so we don't need to store values as an interface{}
which cut down on a few allocs.
4. Finally, and funnily enough the biggest win, was just not calling f.Readdir(-1) and
swapping this for f.Readdirnames as it doesn't return a slice of interfaces.

All in all on Go 1.19.4 this drops allocations in Stat from 324 to 230, or 33.94%.

Signed-off-by: Danny Canter <danny@dcantah.dev>

cgroup2/manager.go

@@ -21,7 +21,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"io"
 	"math"
 	"os"
 	"path/filepath"
@@ -529,7 +528,10 @@ func (c *Manager) Stat() (*stats.Metrics, error) {
 	if err != nil {
 		return nil, err
 	}
-	out := make(map[string]interface{})
+	// Sizing this avoids an allocation to increase the map at runtime;
+	// currently the default bucket size is 8 and we put 40+ elements
+	// in it so we'd always end up allocating.
+	out := make(map[string]string, 50)
 	for _, controller := range controllers {
 		switch controller {
 		case "cpu", "memory":
@@ -549,14 +551,14 @@ func (c *Manager) Stat() (*stats.Metrics, error) {
 			return nil, err
 		}
 	}
-	memoryEvents := make(map[string]interface{})
+	memoryEvents := make(map[string]string)
 	if err := readKVStatsFile(c.path, "memory.events", memoryEvents); err != nil {
 		if !os.IsNotExist(err) {
 			return nil, err
 		}
 	}
-	var metrics stats.Metrics
 
+	var metrics stats.Metrics
 	metrics.Pids = &stats.PidsStat{
 		Current: getPidValue("pids.current", out),
 		Limit:   getPidValue("pids.max", out),
@@ -625,56 +627,53 @@ func (c *Manager) Stat() (*stats.Metrics, error) {
 	return &metrics, nil
 }
 
-func getUint64Value(key string, out map[string]interface{}) uint64 {
+func getUint64Value(key string, out map[string]string) uint64 {
 	v, ok := out[key]
 	if !ok {
 		return 0
 	}
-	switch t := v.(type) {
-	case uint64:
-		return t
+	val, err := parseUint(v, 10, 64)
+	if err != nil {
+		return 0
 	}
-	return 0
+	return val
 }
 
-func getPidValue(key string, out map[string]interface{}) uint64 {
+func getPidValue(key string, out map[string]string) uint64 {
 	v, ok := out[key]
 	if !ok {
 		return 0
 	}
-	switch t := v.(type) {
-	case uint64:
-		return t
-	case string:
-		if t == "max" {
-			return math.MaxUint64
-		}
+	if v == "max" {
+		return math.MaxUint64
+	}
+	val, err := parseUint(v, 10, 64)
+	if err != nil {
+		return 0
 	}
-	return 0
+	return val
 }
 
-func readSingleFile(path string, file string, out map[string]interface{}) error {
+func readSingleFile(path string, file string, out map[string]string) error {
 	f, err := os.Open(filepath.Join(path, file))
 	if err != nil {
 		return err
 	}
 	defer f.Close()
-	data, err := io.ReadAll(f)
+
+	// We expect an unsigned 64 bit integer, or a "max" string
+	// in some cases.
+	buf := make([]byte, 32)
+	n, err := f.Read(buf)
 	if err != nil {
 		return err
 	}
-	s := strings.TrimSpace(string(data))
-	v, err := parseUint(s, 10, 64)
-	if err != nil {
-		// if we cannot parse as a uint, parse as a string
-		out[file] = s
-		return nil
-	}
-	out[file] = v
+
+	out[file] = strings.TrimSpace(string(buf[:n]))
 	return nil
 }
 
-func readKVStatsFile(path string, file string, out map[string]interface{}) error {
+func readKVStatsFile(path string, file string, out map[string]string) error {
 	f, err := os.Open(filepath.Join(path, file))
 	if err != nil {
 		return err
@@ -719,18 +718,11 @@ func (c *Manager) freeze(path string, state State) error {
 
 func (c *Manager) isCgroupEmpty() bool {
 	// In case of any error we return true so that we exit and don't leak resources
-	out := make(map[string]interface{})
+	out := make(map[string]string)
 	if err := readKVStatsFile(c.path, "cgroup.events", out); err != nil {
 		return true
 	}
-	if v, ok := out["populated"]; ok {
-		populated, ok := v.(uint64)
-		if !ok {
-			return true
-		}
-		return populated == 0
-	}
-	return true
+	return out["populated"] == "0"
 }
 
 // MemoryEventFD returns inotify file descriptor and 'memory.events' inotify watch descriptor
@@ -763,37 +755,42 @@ func (c *Manager) EventChan() (<-chan Event, <-chan error) {
 	return ec, errCh
 }
 
-func parseMemoryEvents(out map[string]interface{}) (Event, error) {
+func parseMemoryEvents(out map[string]string) (Event, error) {
 	e := Event{}
 	if v, ok := out["high"]; ok {
-		e.High, ok = v.(uint64)
-		if !ok {
+		val, err := parseUint(v, 10, 64)
+		if err != nil {
 			return Event{}, fmt.Errorf("cannot convert high to uint64: %+v", v)
 		}
+		e.High = val
 	}
 	if v, ok := out["low"]; ok {
-		e.Low, ok = v.(uint64)
-		if !ok {
+		val, err := parseUint(v, 10, 64)
+		if err != nil {
 			return Event{}, fmt.Errorf("cannot convert low to uint64: %+v", v)
 		}
+		e.Low = val
 	}
 	if v, ok := out["max"]; ok {
-		e.Max, ok = v.(uint64)
-		if !ok {
+		val, err := parseUint(v, 10, 64)
+		if err != nil {
 			return Event{}, fmt.Errorf("cannot convert max to uint64: %+v", v)
 		}
+		e.Max = val
 	}
 	if v, ok := out["oom"]; ok {
-		e.OOM, ok = v.(uint64)
-		if !ok {
+		val, err := parseUint(v, 10, 64)
+		if err != nil {
 			return Event{}, fmt.Errorf("cannot convert oom to uint64: %+v", v)
 		}
+		e.OOM = val
 	}
 	if v, ok := out["oom_kill"]; ok {
-		e.OOMKill, ok = v.(uint64)
-		if !ok {
+		val, err := parseUint(v, 10, 64)
+		if err != nil {
 			return Event{}, fmt.Errorf("cannot convert oom_kill to uint64: %+v", v)
 		}
+		e.OOMKill = val
 	}
 	return e, nil
 }
@@ -816,7 +813,7 @@ func (c *Manager) waitForEvents(ec chan<- Event, errCh chan<- error) {
 			return
 		}
 		if bytesRead >= unix.SizeofInotifyEvent {
-			out := make(map[string]interface{})
+			out := make(map[string]string)
 			if err := readKVStatsFile(c.path, "memory.events", out); err != nil {
 				// When cgroup is deleted read may return -ENODEV instead of -ENOENT from open.
 				if _, statErr := os.Lstat(filepath.Join(c.path, "memory.events")); !os.IsNotExist(statErr) {

cgroup2/manager_test.go

@@ -244,3 +244,22 @@ func TestCgroupType(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, cgType, Threaded)
 }
+
+func BenchmarkStat(b *testing.B) {
+	checkCgroupMode(b)
+	group := "/stat-test-cg"
+	groupPath := fmt.Sprintf("%s-%d", group, os.Getpid())
+	c, err := NewManager(defaultCgroup2Path, groupPath, &Resources{})
+	if err != nil {
+		b.Fatal("failed to init new cgroup manager: ", err)
+	}
+	defer os.Remove(c.path)
+
+	b.ReportAllocs()
+	for i := 0; i < b.N; i++ {
+		_, err := c.Stat()
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}

cgroup2/testutils_test.go

@@ -26,14 +26,14 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-func checkCgroupMode(t *testing.T) {
+func checkCgroupMode(tb testing.TB) {
 	var st unix.Statfs_t
 	if err := unix.Statfs(defaultCgroup2Path, &st); err != nil {
-		t.Fatal("cannot statfs cgroup root")
+		tb.Fatal("cannot statfs cgroup root")
 	}
 	isUnified := st.Type == unix.CGROUP2_SUPER_MAGIC
 	if !isUnified {
-		t.Skip("System running in hybrid or cgroupv1 mode")
+		tb.Skip("System running in hybrid or cgroupv1 mode")
 	}
 }
 

cgroup2/utils.go

@@ -92,18 +92,13 @@ func parseCgroupProcsFile(path string) ([]uint64, error) {
 	return out, nil
 }
 
-func parseKV(raw string) (string, interface{}, error) {
+func parseKV(raw string) (string, string, error) {
 	parts := strings.Fields(raw)
 	switch len(parts) {
 	case 2:
-		v, err := parseUint(parts[1], 10, 64)
-		if err != nil {
-			// if we cannot parse as a uint, parse as a string
-			return parts[0], parts[1], nil
-		}
-		return parts[0], v, nil
+		return parts[0], parts[1], nil
 	default:
-		return "", 0, ErrInvalidFormat
+		return "", "", ErrInvalidFormat
 	}
 }
 
@@ -242,21 +237,30 @@ func ToResources(spec *specs.LinuxResources) *Resources {
 
 // Gets uint64 parsed content of single value cgroup stat file
 func getStatFileContentUint64(filePath string) uint64 {
-	contents, err := os.ReadFile(filePath)
+	f, err := os.Open(filePath)
 	if err != nil {
 		return 0
 	}
-	trimmed := strings.TrimSpace(string(contents))
+	defer f.Close()
+
+	// We expect an unsigned 64 bit integer, or a "max" string
+	// in some cases.
+	buf := make([]byte, 32)
+	n, err := f.Read(buf)
+	if err != nil {
+		return 0
+	}
+
+	trimmed := strings.TrimSpace(string(buf[:n]))
 	if trimmed == "max" {
 		return math.MaxUint64
 	}
 
 	res, err := parseUint(trimmed, 10, 64)
 	if err != nil {
-		logrus.Errorf("unable to parse %q as a uint from Cgroup file %q", string(contents), filePath)
+		logrus.Errorf("unable to parse %q as a uint from Cgroup file %q", trimmed, filePath)
 		return res
 	}
-
 	return res
 }
 
@@ -385,41 +389,55 @@ func systemdUnitFromPath(path string) string {
 func readHugeTlbStats(path string) []*stats.HugeTlbStat {
 	usage := []*stats.HugeTlbStat{}
 	keyUsage := make(map[string]*stats.HugeTlbStat)
+
 	f, err := os.Open(path)
 	if err != nil {
 		return usage
 	}
-	files, err := f.Readdir(-1)
+
+	files, err := f.Readdirnames(-1)
 	f.Close()
 	if err != nil {
 		return usage
 	}
 
+	buf := make([]byte, 32)
 	for _, file := range files {
-		if strings.Contains(file.Name(), "hugetlb") &&
-			(strings.HasSuffix(file.Name(), "max") || strings.HasSuffix(file.Name(), "current")) {
-			var hugeTlb *stats.HugeTlbStat
-			var ok bool
-			fileName := strings.Split(file.Name(), ".")
+		if strings.Contains(file, "hugetlb") &&
+			(strings.HasSuffix(file, "max") || strings.HasSuffix(file, "current")) {
+			var (
+				hugeTlb *stats.HugeTlbStat
+				ok      bool
+			)
+			fileName := strings.Split(file, ".")
 			pageSize := fileName[1]
 			if hugeTlb, ok = keyUsage[pageSize]; !ok {
 				hugeTlb = &stats.HugeTlbStat{}
 			}
 			hugeTlb.Pagesize = pageSize
-			out, err := os.ReadFile(filepath.Join(path, file.Name()))
+
+			f, err := os.Open(filepath.Join(path, file))
+			if err != nil {
+				continue
+			}
+			defer f.Close()
+
+			n, err := f.Read(buf)
 			if err != nil {
 				continue
 			}
+
 			var value uint64
-			stringVal := strings.TrimSpace(string(out))
+			stringVal := strings.TrimSpace(string(buf[:n]))
 			if stringVal == "max" {
 				value = math.MaxUint64
 			} else {
 				value, err = strconv.ParseUint(stringVal, 10, 64)
+				if err != nil {
+					continue
+				}
 			}
-			if err != nil {
-				continue
-			}
+
 			switch fileName[2] {
 			case "max":
 				hugeTlb.Max = value

cgroup2/utils_test.go

@@ -64,3 +64,22 @@ func TestToResources(t *testing.T) {
 	v2resources2 := ToResources(&res2)
 	assert.Equal(t, CPUMax("max 10000"), v2resources2.CPU.Max)
 }
+
+func BenchmarkReaduint64(b *testing.B) {
+	b.ReportAllocs()
+
+	for i := 0; i < b.N; i++ {
+		_ = getStatFileContentUint64("/proc/self/loginuid")
+	}
+}
+
+func BenchmarkReadSingleFile(b *testing.B) {
+	b.ReportAllocs()
+
+	out := make(map[string]string)
+	for i := 0; i < b.N; i++ {
+		if err := readSingleFile("/proc/self/loginuid", "", out); err != nil {
+			b.Fatal(err)
+		}
+	}
+}