From defb1cc718c3c7ebdc62345a54b7eafe2850cff0 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Thu, 27 Jan 2022 19:23:18 -0800
Subject: [PATCH] libct/cg/dev: optimize and test findDeviceGroup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

1. Use strings.TrimPrefix instead of fmt.Sscanf and simplify the code.

2. Add a test case and a benchmark.

The benchmark shows some improvement, compared to the old
implementation:

name               old time/op    new time/op    delta
FindDeviceGroup-4    39.7µs ± 2%    26.8µs ± 2%  -32.63%  (p=0.008 n=5+5)

name               old alloc/op   new alloc/op   delta
FindDeviceGroup-4    6.08kB ± 0%    4.23kB ± 0%  -30.39%  (p=0.008 n=5+5)

name               old allocs/op  new allocs/op  delta
FindDeviceGroup-4       117 ± 0%         6 ± 0%  -94.87%  (p=0.008 n=5+5)

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/devices/systemd.go      | 20 ++++-----------
 libcontainer/cgroups/devices/systemd_test.go | 27 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/libcontainer/cgroups/devices/systemd.go b/libcontainer/cgroups/devices/systemd.go
index b78d90eaf..70768f9db 100644
--- a/libcontainer/cgroups/devices/systemd.go
+++ b/libcontainer/cgroups/devices/systemd.go
@@ -2,9 +2,9 @@ package devices
 
 import (
 	"bufio"
-	"errors"
 	"fmt"
 	"os"
+	"strconv"
 	"strings"
 
 	systemdDbus "github.com/coreos/go-systemd/v22/dbus"
@@ -181,6 +181,7 @@ func findDeviceGroup(ruleType devices.Type, ruleMajor int64) (string, error) {
 	if err != nil {
 		return "", err
 	}
+	ruleMajorStr := strconv.FormatInt(ruleMajor, 10) + " "
 
 	scanner := bufio.NewScanner(fh)
 	var currentType devices.Type
@@ -205,20 +206,9 @@ func findDeviceGroup(ruleType devices.Type, ruleMajor int64) (string, error) {
 			continue
 		}
 
-		// Parse out the (major, name).
-		var (
-			currMajor int64
-			currName  string
-		)
-		if n, err := fmt.Sscanf(line, "%d %s", &currMajor, &currName); err != nil || n != 2 {
-			if err == nil {
-				err = errors.New("wrong number of fields")
-			}
-			return "", fmt.Errorf("scan /proc/devices line %q: %w", line, err)
-		}
-
-		if currMajor == ruleMajor {
-			return prefix + currName, nil
+		group := strings.TrimPrefix(line, ruleMajorStr)
+		if len(group) < len(line) { // got it
+			return prefix + group, nil
 		}
 	}
 	if err := scanner.Err(); err != nil {
diff --git a/libcontainer/cgroups/devices/systemd_test.go b/libcontainer/cgroups/devices/systemd_test.go
index 050fdd2d2..b6e81dcd8 100644
--- a/libcontainer/cgroups/devices/systemd_test.go
+++ b/libcontainer/cgroups/devices/systemd_test.go
@@ -2,6 +2,7 @@ package devices
 
 import (
 	"bytes"
+	"fmt"
 	"os"
 	"os/exec"
 	"strings"
@@ -240,6 +241,32 @@ func TestSkipDevicesFalse(t *testing.T) {
 	})
 }
 
+func testFindDeviceGroup() error {
+	const (
+		major = 136
+		group = "char-pts"
+	)
+	res, err := findDeviceGroup(devices.CharDevice, major)
+	if res != group || err != nil {
+		return fmt.Errorf("expected %v, nil, got %v, %w", group, res, err)
+	}
+	return nil
+}
+
+func TestFindDeviceGroup(t *testing.T) {
+	if err := testFindDeviceGroup(); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func BenchmarkFindDeviceGroup(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		if err := testFindDeviceGroup(); err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
 func newManager(t *testing.T, config *configs.Cgroup) (m cgroups.Manager) {
 	t.Helper()
 	var err error