From 504271a37476854da5a582ec67ad434de8f1ef6e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 5 Aug 2021 15:59:08 -0700 Subject: [PATCH 1/3] libct/cg: move GetAllPids out of utils.go This is just moving the code around to ease the code review, no other changes. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/getallpids.go | 30 ++++++++++++++++++++++++++++++ libcontainer/cgroups/utils.go | 22 ---------------------- 2 files changed, 30 insertions(+), 22 deletions(-) create mode 100644 libcontainer/cgroups/getallpids.go diff --git a/libcontainer/cgroups/getallpids.go b/libcontainer/cgroups/getallpids.go new file mode 100644 index 00000000000..58d00bc690a --- /dev/null +++ b/libcontainer/cgroups/getallpids.go @@ -0,0 +1,30 @@ +// +build linux + +package cgroups + +import ( + "os" + "path/filepath" +) + +// GetAllPids returns all pids, that were added to cgroup at path and to all its +// subcgroups. +func GetAllPids(path string) ([]int, error) { + var pids []int + // collect pids from all sub-cgroups + err := filepath.Walk(path, func(p string, info os.FileInfo, iErr error) error { + if iErr != nil { + return iErr + } + if info.IsDir() || info.Name() != CgroupProcesses { + return nil + } + cPids, err := readProcsFile(p) + if err != nil { + return err + } + pids = append(pids, cPids...) + return nil + }) + return pids, err +} diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go index 75015d6638c..9fc30db5b13 100644 --- a/libcontainer/cgroups/utils.go +++ b/libcontainer/cgroups/utils.go @@ -339,28 +339,6 @@ func GetPids(dir string) ([]int, error) { return readProcsFile(filepath.Join(dir, CgroupProcesses)) } -// GetAllPids returns all pids, that were added to cgroup at path and to all its -// subcgroups. -func GetAllPids(path string) ([]int, error) { - var pids []int - // collect pids from all sub-cgroups - err := filepath.Walk(path, func(p string, info os.FileInfo, iErr error) error { - if iErr != nil { - return iErr - } - if info.IsDir() || info.Name() != CgroupProcesses { - return nil - } - cPids, err := readProcsFile(p) - if err != nil { - return err - } - pids = append(pids, cPids...) - return nil - }) - return pids, err -} - // WriteCgroupProc writes the specified pid into the cgroup's cgroup.procs file func WriteCgroupProc(dir string, pid int) error { // Normally dir should not be empty, one case is that cgroup subsystem From 363468d0e4bb2365c5c16b88dbccb5ac5ff71f1f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 5 Aug 2021 16:04:29 -0700 Subject: [PATCH 2/3] libct/cg: improve GetAllPids and readProcsFile Since every cgroup directory is guaranteed to have cgroup.procs file, we don't have to do filename comparison in GetAllPids() and just read cgroup.procs in every directory. While at it, switch readProcsFile to use our own OpenFile. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/getallpids.go | 2 +- libcontainer/cgroups/utils.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libcontainer/cgroups/getallpids.go b/libcontainer/cgroups/getallpids.go index 58d00bc690a..9a23937202e 100644 --- a/libcontainer/cgroups/getallpids.go +++ b/libcontainer/cgroups/getallpids.go @@ -16,7 +16,7 @@ func GetAllPids(path string) ([]int, error) { if iErr != nil { return iErr } - if info.IsDir() || info.Name() != CgroupProcesses { + if !info.IsDir() { return nil } cPids, err := readProcsFile(p) diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go index 9fc30db5b13..b31705bed06 100644 --- a/libcontainer/cgroups/utils.go +++ b/libcontainer/cgroups/utils.go @@ -118,8 +118,8 @@ func GetAllSubsystems() ([]string, error) { return subsystems, nil } -func readProcsFile(file string) ([]int, error) { - f, err := os.Open(file) +func readProcsFile(dir string) ([]int, error) { + f, err := OpenFile(dir, CgroupProcesses, os.O_RDONLY) if err != nil { return nil, err } @@ -336,7 +336,7 @@ func getHugePageSizeFromFilenames(fileNames []string) ([]string, error) { // GetPids returns all pids, that were added to cgroup at path. func GetPids(dir string) ([]int, error) { - return readProcsFile(filepath.Join(dir, CgroupProcesses)) + return readProcsFile(dir) } // WriteCgroupProc writes the specified pid into the cgroup's cgroup.procs file From d0c3bc44e7b32e2ac4dd3a8fef7ca24678366484 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 5 Aug 2021 16:10:59 -0700 Subject: [PATCH 3/3] libct/cg: GetAllPids: optimize for go 1.16+ filepath.WalkDir function, introduced in Go 1.16, doesn't do stat(2) on every entry, and is therefore somewhat faster (see below). Since we have to support Go 1.15, keep the old version for backward compatibility. Add a quick benchmark, which shows approximately 3x improvement: $ go1.15.15 test -bench AllPid -run xxx . BenchmarkGetAllPids-4 48 23528839 ns/op $ go version go version go1.16.6 linux/amd64 $ go test -bench AllPid -run xxx . BenchmarkGetAllPids-4 147 7700170 ns/op (Unrelated but worth noting -- go 1.17rc2 is pushing it even further) $ go1.17rc2 test -bench AllPid -run xxx . BenchmarkGetAllPids-4 164 6820994 ns/op Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/getallpids.go | 13 +++++----- libcontainer/cgroups/getallpids_go115.go | 30 ++++++++++++++++++++++++ libcontainer/cgroups/getallpids_test.go | 19 +++++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 libcontainer/cgroups/getallpids_go115.go create mode 100644 libcontainer/cgroups/getallpids_test.go diff --git a/libcontainer/cgroups/getallpids.go b/libcontainer/cgroups/getallpids.go index 9a23937202e..b218744e42e 100644 --- a/libcontainer/cgroups/getallpids.go +++ b/libcontainer/cgroups/getallpids.go @@ -1,22 +1,21 @@ -// +build linux +// +build linux,go1.16 package cgroups import ( - "os" + "io/fs" "path/filepath" ) -// GetAllPids returns all pids, that were added to cgroup at path and to all its -// subcgroups. +// GetAllPids returns all pids from the cgroup identified by path, and all its +// sub-cgroups. func GetAllPids(path string) ([]int, error) { var pids []int - // collect pids from all sub-cgroups - err := filepath.Walk(path, func(p string, info os.FileInfo, iErr error) error { + err := filepath.WalkDir(path, func(p string, d fs.DirEntry, iErr error) error { if iErr != nil { return iErr } - if !info.IsDir() { + if !d.IsDir() { return nil } cPids, err := readProcsFile(p) diff --git a/libcontainer/cgroups/getallpids_go115.go b/libcontainer/cgroups/getallpids_go115.go new file mode 100644 index 00000000000..5d55a685dab --- /dev/null +++ b/libcontainer/cgroups/getallpids_go115.go @@ -0,0 +1,30 @@ +// +build linux,!go1.16 + +package cgroups + +import ( + "os" + "path/filepath" +) + +// GetAllPids returns all pids, that were added to cgroup at path and to all its +// subcgroups. +func GetAllPids(path string) ([]int, error) { + var pids []int + // collect pids from all sub-cgroups + err := filepath.Walk(path, func(p string, info os.FileInfo, iErr error) error { + if iErr != nil { + return iErr + } + if !info.IsDir() { + return nil + } + cPids, err := readProcsFile(p) + if err != nil { + return err + } + pids = append(pids, cPids...) + return nil + }) + return pids, err +} diff --git a/libcontainer/cgroups/getallpids_test.go b/libcontainer/cgroups/getallpids_test.go new file mode 100644 index 00000000000..b6100686ef4 --- /dev/null +++ b/libcontainer/cgroups/getallpids_test.go @@ -0,0 +1,19 @@ +// +build linux + +package cgroups + +import ( + "testing" +) + +func BenchmarkGetAllPids(b *testing.B) { + total := 0 + for i := 0; i < b.N; i++ { + i, err := GetAllPids("/sys/fs/cgroup") + if err != nil { + b.Fatal(err) + } + total += len(i) + } + b.Logf("iter: %d, total: %d", b.N, total) +}