Merge pull request #291 from manugupt1/improve-perf

Improve performance by for pid stats (cgroups1) re-using readuint

Author: kzys

cgroup1/pids.go

@@ -20,7 +20,6 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
-	"strings"
 
 	v1 "github.com/containerd/cgroups/v3/cgroup1/stats"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
@@ -67,16 +66,10 @@ func (p *pidsController) Stat(path string, stats *v1.Metrics) error {
 	if err != nil {
 		return err
 	}
-	var max uint64
-	maxData, err := os.ReadFile(filepath.Join(p.Path(path), "pids.max"))
+	max, err := readUint(filepath.Join(p.Path(path), "pids.max"))
 	if err != nil {
 		return err
 	}
-	if maxS := strings.TrimSpace(string(maxData)); maxS != "max" {
-		if max, err = parseUint(maxS, 10, 64); err != nil {
-			return err
-		}
-	}
 	stats.Pids = &v1.PidsStat{
 		Current: current,
 		Limit:   max,

cgroup1/pids_test.go

@@ -186,3 +186,50 @@ func TestPidsOverflowMax(t *testing.T) {
 		t.Fatal("expected not nil err")
 	}
 }
+
+func BenchmarkTestPids(b *testing.B) {
+
+	mock, err := newMock(b)
+	if err != nil {
+		b.Fatal(err)
+	}
+	defer func() {
+		if err := mock.delete(); err != nil {
+			b.Errorf("failed delete: %v", err)
+		}
+	}()
+
+	pids := NewPids(mock.root)
+	if pids == nil {
+		b.Fatal("pids is nil")
+	}
+	resources := specs.LinuxResources{
+		Pids: &specs.LinuxPids{
+			Limit: 10,
+		},
+	}
+
+	err = pids.Create("test", &resources)
+	if err != nil {
+		b.Fatal(err)
+	}
+
+	current := filepath.Join(mock.root, "pids", "test", "pids.current")
+	if err = os.WriteFile(
+		current,
+		[]byte(strconv.Itoa(5)),
+		defaultFilePerm,
+	); err != nil {
+		b.Fatal(err)
+	}
+
+	b.ReportAllocs()
+
+	for i := 0; i < b.N; i++ {
+		metrics := v1.Metrics{}
+		err = pids.Stat("test", &metrics)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}

cgroup1/utils.go

@@ -18,6 +18,7 @@ package cgroup1
 
 import (
 	"bufio"
+	"bytes"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -137,13 +138,19 @@ func readUint(path string) (uint64, error) {
 	}
 	defer f.Close()
 
-	b := make([]byte, 128) // Chose 128 as some files have leading/trailing whitespaces and alignment
+	// We should only need 20 bytes for the max uint64, but for a nice power of 2
+	// lets use 32.
+	b := make([]byte, 32)
 	n, err := f.Read(b)
 	if err != nil {
 		return 0, err
 	}
-
-	return parseUint(strings.TrimSpace(string(b[:n])), 10, 64)
+	s := string(bytes.TrimSpace(b[:n]))
+	if s == "max" {
+		// Return 0 for the max value to maintain backward compatibility.
+		return 0, nil
+	}
+	return parseUint(s, 10, 64)
 }
 
 func parseUint(s string, base, bitSize int) (uint64, error) {

cgroup1/utils_test.go

@@ -17,6 +17,8 @@
 package cgroup1
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 )
 
@@ -30,3 +32,20 @@ func BenchmarkReaduint64(b *testing.B) {
 		}
 	}
 }
+
+func TestReadUint(t *testing.T) {
+	tDir := t.TempDir()
+	pidsmax := filepath.Join(tDir, "pids.max")
+	err := os.WriteFile(pidsmax, []byte("max"), 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+	max, err := readUint(pidsmax)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// test for backwards compatibility
+	if max != 0 {
+		t.Fail()
+	}
+}