Skip to content

Commit

Permalink
Merge pull request moby#49221 from thaJeztah/internalize_pkg_parsers_…
Browse files Browse the repository at this point in the history
…step2

pkg/sysinfo: parse cpuset.cpus/mems once and memoize
  • Loading branch information
thaJeztah authored Jan 6, 2025
2 parents 0342576 + 1359772 commit 3ca5ca4
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 38 deletions.
13 changes: 4 additions & 9 deletions integration-cli/docker_cli_run_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/docker/docker/client"
"github.com/docker/docker/integration-cli/cli"
"github.com/docker/docker/integration-cli/cli/build"
"github.com/docker/docker/pkg/parsers"
"github.com/docker/docker/pkg/sysinfo"
"github.com/docker/docker/testutil"
"github.com/moby/sys/mount"
Expand Down Expand Up @@ -694,12 +693,10 @@ func (s *DockerCLIRunSuite) TestRunSwapLessThanMemoryLimit(c *testing.T) {
func (s *DockerCLIRunSuite) TestRunInvalidCpusetCpusFlagValue(c *testing.T) {
testRequires(c, cgroupCpuset, testEnv.IsLocalDaemon)

sysInfo := sysinfo.New()
cpus, err := parsers.ParseUintList(sysInfo.Cpus)
assert.NilError(c, err)
cpus := sysinfo.New().CPUSets
var invalid int
for i := 0; i <= len(cpus)+1; i++ {
if !cpus[i] {
if _, ok := cpus[i]; !ok {
invalid = i
break
}
Expand All @@ -713,12 +710,10 @@ func (s *DockerCLIRunSuite) TestRunInvalidCpusetCpusFlagValue(c *testing.T) {
func (s *DockerCLIRunSuite) TestRunInvalidCpusetMemsFlagValue(c *testing.T) {
testRequires(c, cgroupCpuset)

sysInfo := sysinfo.New()
mems, err := parsers.ParseUintList(sysInfo.Mems)
assert.NilError(c, err)
mems := sysinfo.New().MemSets
var invalid int
for i := 0; i <= len(mems)+1; i++ {
if !mems[i] {
if _, ok := mems[i]; !ok {
invalid = i
break
}
Expand Down
14 changes: 14 additions & 0 deletions pkg/sysinfo/cgroup2_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,25 @@ func applyCPUSetCgroupInfoV2(info *SysInfo) {
}
info.Cpus = strings.TrimSpace(string(cpus))

cpuSets, err := parseUintList(info.Cpus, 0)
if err != nil {
info.Warnings = append(info.Warnings, "Unable to parse cpuset cpus: "+err.Error())
return
}
info.CPUSets = cpuSets

mems, err := os.ReadFile(path.Join("/sys/fs/cgroup", info.cg2GroupPath, "cpuset.mems.effective"))
if err != nil {
return
}
info.Mems = strings.TrimSpace(string(mems))

memSets, err := parseUintList(info.Cpus, 0)
if err != nil {
info.Warnings = append(info.Warnings, "Unable to parse cpuset mems: "+err.Error())
return
}
info.MemSets = memSets
}

func applyPIDSCgroupInfoV2(info *SysInfo) {
Expand Down
12 changes: 10 additions & 2 deletions pkg/sysinfo/sysinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,17 @@ type cgroupCpusetInfo struct {
// or "cpuset.cpus" (cgroups v1).
Cpus string

// CPUSets holds the list of available cpusets parsed from "cpuset.cpus.effective" (cgroups v2)
// or "cpuset.cpus" (cgroups v1).
CPUSets map[int]struct{}

// Available Cpuset's memory nodes as read from "cpuset.mems.effective" (cgroups v2)
// or "cpuset.mems" (cgroups v1).
Mems string

// MemSets holds the list of available cpusets parsed from "cpuset.mems.effective" (cgroups v2)
// or "cpuset.mems" (cgroups v1).
MemSets map[int]struct{}
}

type cgroupPids struct {
Expand All @@ -138,12 +146,12 @@ type cgroupPids struct {
// in cgroup's cpuset.cpus set, `false` otherwise.
// If error is not nil a parsing error occurred.
func (c cgroupCpusetInfo) IsCpusetCpusAvailable(requested string) (bool, error) {
return isCpusetListAvailable(requested, c.Cpus)
return isCpusetListAvailable(requested, c.CPUSets)
}

// IsCpusetMemsAvailable returns `true` if the provided string set is contained
// in cgroup's cpuset.mems set, `false` otherwise.
// If error is not nil a parsing error occurred.
func (c cgroupCpusetInfo) IsCpusetMemsAvailable(requested string) (bool, error) {
return isCpusetListAvailable(requested, c.Mems)
return isCpusetListAvailable(requested, c.MemSets)
}
37 changes: 22 additions & 15 deletions pkg/sysinfo/sysinfo_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,24 @@ func applyCPUSetCgroupInfo(info *SysInfo) {
return
}
info.Cpus = strings.TrimSpace(string(cpus))
cpuSets, err := parseUintList(info.Cpus, 0)
if err != nil {
info.Warnings = append(info.Warnings, "Unable to parse cpuset cpus: "+err.Error())
return
}
info.CPUSets = cpuSets

mems, err := os.ReadFile(path.Join(mountPoint, "cpuset.mems"))
if err != nil {
return
}
info.Mems = strings.TrimSpace(string(mems))
memSets, err := parseUintList(info.Cpus, 0)
if err != nil {
info.Warnings = append(info.Warnings, "Unable to parse cpuset mems: "+err.Error())
return
}
info.MemSets = memSets
}

// applyPIDSCgroupInfo adds whether the pids cgroup controller is available to the info.
Expand Down Expand Up @@ -319,11 +331,7 @@ func readProcBool(path string) bool {
// defaultMaxCPUs is the normal maximum number of CPUs on Linux.
const defaultMaxCPUs = 8192

func isCpusetListAvailable(requested, available string) (bool, error) {
parsedAvailable, err := parseUintList(available, 0)
if err != nil {
return false, err
}
func isCpusetListAvailable(requested string, available map[int]struct{}) (bool, error) {
// Start with the normal maximum number of CPUs on Linux, but accept
// more if we actually have more CPUs available.
//
Expand All @@ -336,7 +344,7 @@ func isCpusetListAvailable(requested, available string) (bool, error) {
//
// More details in https://github.com/docker-archive/engine/pull/70#issuecomment-458458288
maxCPUs := defaultMaxCPUs
for m := range parsedAvailable {
for m := range available {
if m > maxCPUs {
maxCPUs = m
}
Expand All @@ -346,7 +354,7 @@ func isCpusetListAvailable(requested, available string) (bool, error) {
return false, err
}
for k := range parsedRequested {
if !parsedAvailable[k] {
if _, ok := available[k]; !ok {
return false, nil
}
}
Expand All @@ -357,7 +365,7 @@ func isCpusetListAvailable(requested, available string) (bool, error) {
// found in some cgroup file (e.g. `cpuset.cpus`, `cpuset.mems`), which could be
// one of the formats below. Note that duplicates are actually allowed in the
// input string. It returns a `map[int]bool` with available elements from `val`
// set to `true`. Values larger than `maximum` cause an error if max is non zero,
// set to `true`. Values larger than `maximum` cause an error if max is non-zero,
// in order to stop the map becoming excessively large.
// Supported formats:
//
Expand All @@ -368,16 +376,15 @@ func isCpusetListAvailable(requested, available string) (bool, error) {
// 03,1-3 <- this is gonna get parsed as [1,2,3]
// 3,2,1
// 0-2,3,1
func parseUintList(val string, maximum int) (map[int]bool, error) {
func parseUintList(val string, maximum int) (map[int]struct{}, error) {
if val == "" {
return map[int]bool{}, nil
return map[int]struct{}{}, nil
}

availableInts := make(map[int]bool)
split := strings.Split(val, ",")
availableInts := make(map[int]struct{})
errInvalidFormat := fmt.Errorf("invalid format: %s", val)

for _, r := range split {
for _, r := range strings.Split(val, ",") {
if !strings.Contains(r, "-") {
v, err := strconv.Atoi(r)
if err != nil {
Expand All @@ -386,7 +393,7 @@ func parseUintList(val string, maximum int) (map[int]bool, error) {
if maximum != 0 && v > maximum {
return nil, fmt.Errorf("value of out range, maximum is %d", maximum)
}
availableInts[v] = true
availableInts[v] = struct{}{}
} else {
minS, maxS, _ := strings.Cut(r, "-")
minAvailable, err := strconv.Atoi(minS)
Expand All @@ -404,7 +411,7 @@ func parseUintList(val string, maximum int) (map[int]bool, error) {
return nil, fmt.Errorf("value of out range, maximum is %d", maximum)
}
for i := minAvailable; i <= maxAvailable; i++ {
availableInts[i] = true
availableInts[i] = struct{}{}
}
}
}
Expand Down
27 changes: 16 additions & 11 deletions pkg/sysinfo/sysinfo_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,29 +83,34 @@ func TestIsCpusetListAvailable(t *testing.T) {
{"01,3", "0-4", true, false},
{"", "0-7", true, false},
{"1--42", "0-7", false, true},
{"1-42", "00-1,8,,9", false, true},
{"1-42", "00-1,8,9", false, true},
{"1,41-42", "43,45", false, false},
{"0-3", "", false, false},
}
for _, c := range cases {
r, err := isCpusetListAvailable(c.provided, c.available)
available, err := parseUintList(c.available, 0)
if err != nil {
t.Fatal(err)
}
r, err := isCpusetListAvailable(c.provided, available)
if (c.err && err == nil) && r != c.res {
t.Fatalf("Expected pair: %v, %v for %s, %s. Got %v, %v instead", c.res, c.err, c.provided, c.available, (c.err && err == nil), r)
}
}
}

func TestParseUintList(t *testing.T) {
valids := map[string]map[int]bool{
yes := struct{}{}
valids := map[string]map[int]struct{}{
"": {},
"7": {7: true},
"1-6": {1: true, 2: true, 3: true, 4: true, 5: true, 6: true},
"0-7": {0: true, 1: true, 2: true, 3: true, 4: true, 5: true, 6: true, 7: true},
"0,3-4,7,8-10": {0: true, 3: true, 4: true, 7: true, 8: true, 9: true, 10: true},
"0-0,0,1-4": {0: true, 1: true, 2: true, 3: true, 4: true},
"03,1-3": {1: true, 2: true, 3: true},
"3,2,1": {1: true, 2: true, 3: true},
"0-2,3,1": {0: true, 1: true, 2: true, 3: true},
"7": {7: yes},
"1-6": {1: yes, 2: yes, 3: yes, 4: yes, 5: yes, 6: yes},
"0-7": {0: yes, 1: yes, 2: yes, 3: yes, 4: yes, 5: yes, 6: yes, 7: yes},
"0,3-4,7,8-10": {0: yes, 3: yes, 4: yes, 7: yes, 8: yes, 9: yes, 10: yes},
"0-0,0,1-4": {0: yes, 1: yes, 2: yes, 3: yes, 4: yes},
"03,1-3": {1: yes, 2: yes, 3: yes},
"3,2,1": {1: yes, 2: yes, 3: yes},
"0-2,3,1": {0: yes, 1: yes, 2: yes, 3: yes},
}
for k, v := range valids {
out, err := parseUintList(k, 0)
Expand Down
2 changes: 1 addition & 1 deletion pkg/sysinfo/sysinfo_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ func New(options ...Opt) *SysInfo {
return &SysInfo{}
}

func isCpusetListAvailable(string, string) (bool, error) {
func isCpusetListAvailable(string, map[int]struct{}) (bool, error) {
return false, nil
}

0 comments on commit 3ca5ca4

Please sign in to comment.