cpuset: Delete 'builder' methods

All usage of builder pattern is convertible to cpuset.New()
with the same or fewer lines of code.

Migrate Builder.Add to a private method of CPUSet, with a comment
that it is only intended for internal use to preserve immutable
propoerty of the exported interface.

This also removes 'require' library dependency, which avoids
non-standard library usage.
This commit is contained in:
Ian K. Coolidge 2022-11-08 14:57:03 +00:00
parent f3829c4be3
commit cbb985a310
6 changed files with 77 additions and 124 deletions

View File

@ -176,21 +176,21 @@ func TestStaticPolicyStart(t *testing.T) {
} }
func TestStaticPolicyAdd(t *testing.T) { func TestStaticPolicyAdd(t *testing.T) {
largeTopoBuilder := cpuset.NewBuilder() var largeTopoCPUids []int
largeTopoSock0Builder := cpuset.NewBuilder() var largeTopoSock0CPUids []int
largeTopoSock1Builder := cpuset.NewBuilder() var largeTopoSock1CPUids []int
largeTopo := *topoQuadSocketFourWayHT largeTopo := *topoQuadSocketFourWayHT
for cpuid, val := range largeTopo.CPUDetails { for cpuid, val := range largeTopo.CPUDetails {
largeTopoBuilder.Add(cpuid) largeTopoCPUids = append(largeTopoCPUids, cpuid)
if val.SocketID == 0 { if val.SocketID == 0 {
largeTopoSock0Builder.Add(cpuid) largeTopoSock0CPUids = append(largeTopoSock0CPUids, cpuid)
} else if val.SocketID == 1 { } else if val.SocketID == 1 {
largeTopoSock1Builder.Add(cpuid) largeTopoSock1CPUids = append(largeTopoSock1CPUids, cpuid)
} }
} }
largeTopoCPUSet := largeTopoBuilder.Result() largeTopoCPUSet := cpuset.New(largeTopoCPUids...)
largeTopoSock0CPUSet := largeTopoSock0Builder.Result() largeTopoSock0CPUSet := cpuset.New(largeTopoSock0CPUids...)
largeTopoSock1CPUSet := largeTopoSock1Builder.Result() largeTopoSock1CPUSet := cpuset.New(largeTopoSock1CPUids...)
// these are the cases which must behave the same regardless the policy options. // these are the cases which must behave the same regardless the policy options.
// So we will permutate the options to ensure this holds true. // So we will permutate the options to ensure this holds true.

View File

@ -83,138 +83,138 @@ func (d CPUDetails) KeepOnly(cpus cpuset.CPUSet) CPUDetails {
// NUMANodes returns all of the NUMANode IDs associated with the CPUs in this // NUMANodes returns all of the NUMANode IDs associated with the CPUs in this
// CPUDetails. // CPUDetails.
func (d CPUDetails) NUMANodes() cpuset.CPUSet { func (d CPUDetails) NUMANodes() cpuset.CPUSet {
b := cpuset.NewBuilder() var numaNodeIDs []int
for _, info := range d { for _, info := range d {
b.Add(info.NUMANodeID) numaNodeIDs = append(numaNodeIDs, info.NUMANodeID)
} }
return b.Result() return cpuset.New(numaNodeIDs...)
} }
// NUMANodesInSockets returns all of the logical NUMANode IDs associated with // NUMANodesInSockets returns all of the logical NUMANode IDs associated with
// the given socket IDs in this CPUDetails. // the given socket IDs in this CPUDetails.
func (d CPUDetails) NUMANodesInSockets(ids ...int) cpuset.CPUSet { func (d CPUDetails) NUMANodesInSockets(ids ...int) cpuset.CPUSet {
b := cpuset.NewBuilder() var numaNodeIDs []int
for _, id := range ids { for _, id := range ids {
for _, info := range d { for _, info := range d {
if info.SocketID == id { if info.SocketID == id {
b.Add(info.NUMANodeID) numaNodeIDs = append(numaNodeIDs, info.NUMANodeID)
} }
} }
} }
return b.Result() return cpuset.New(numaNodeIDs...)
} }
// Sockets returns all of the socket IDs associated with the CPUs in this // Sockets returns all of the socket IDs associated with the CPUs in this
// CPUDetails. // CPUDetails.
func (d CPUDetails) Sockets() cpuset.CPUSet { func (d CPUDetails) Sockets() cpuset.CPUSet {
b := cpuset.NewBuilder() var socketIDs []int
for _, info := range d { for _, info := range d {
b.Add(info.SocketID) socketIDs = append(socketIDs, info.SocketID)
} }
return b.Result() return cpuset.New(socketIDs...)
} }
// CPUsInSockets returns all of the logical CPU IDs associated with the given // CPUsInSockets returns all of the logical CPU IDs associated with the given
// socket IDs in this CPUDetails. // socket IDs in this CPUDetails.
func (d CPUDetails) CPUsInSockets(ids ...int) cpuset.CPUSet { func (d CPUDetails) CPUsInSockets(ids ...int) cpuset.CPUSet {
b := cpuset.NewBuilder() var cpuIDs []int
for _, id := range ids { for _, id := range ids {
for cpu, info := range d { for cpu, info := range d {
if info.SocketID == id { if info.SocketID == id {
b.Add(cpu) cpuIDs = append(cpuIDs, cpu)
} }
} }
} }
return b.Result() return cpuset.New(cpuIDs...)
} }
// SocketsInNUMANodes returns all of the logical Socket IDs associated with the // SocketsInNUMANodes returns all of the logical Socket IDs associated with the
// given NUMANode IDs in this CPUDetails. // given NUMANode IDs in this CPUDetails.
func (d CPUDetails) SocketsInNUMANodes(ids ...int) cpuset.CPUSet { func (d CPUDetails) SocketsInNUMANodes(ids ...int) cpuset.CPUSet {
b := cpuset.NewBuilder() var socketIDs []int
for _, id := range ids { for _, id := range ids {
for _, info := range d { for _, info := range d {
if info.NUMANodeID == id { if info.NUMANodeID == id {
b.Add(info.SocketID) socketIDs = append(socketIDs, info.SocketID)
} }
} }
} }
return b.Result() return cpuset.New(socketIDs...)
} }
// Cores returns all of the core IDs associated with the CPUs in this // Cores returns all of the core IDs associated with the CPUs in this
// CPUDetails. // CPUDetails.
func (d CPUDetails) Cores() cpuset.CPUSet { func (d CPUDetails) Cores() cpuset.CPUSet {
b := cpuset.NewBuilder() var coreIDs []int
for _, info := range d { for _, info := range d {
b.Add(info.CoreID) coreIDs = append(coreIDs, info.CoreID)
} }
return b.Result() return cpuset.New(coreIDs...)
} }
// CoresInNUMANodes returns all of the core IDs associated with the given // CoresInNUMANodes returns all of the core IDs associated with the given
// NUMANode IDs in this CPUDetails. // NUMANode IDs in this CPUDetails.
func (d CPUDetails) CoresInNUMANodes(ids ...int) cpuset.CPUSet { func (d CPUDetails) CoresInNUMANodes(ids ...int) cpuset.CPUSet {
b := cpuset.NewBuilder() var coreIDs []int
for _, id := range ids { for _, id := range ids {
for _, info := range d { for _, info := range d {
if info.NUMANodeID == id { if info.NUMANodeID == id {
b.Add(info.CoreID) coreIDs = append(coreIDs, info.CoreID)
} }
} }
} }
return b.Result() return cpuset.New(coreIDs...)
} }
// CoresInSockets returns all of the core IDs associated with the given socket // CoresInSockets returns all of the core IDs associated with the given socket
// IDs in this CPUDetails. // IDs in this CPUDetails.
func (d CPUDetails) CoresInSockets(ids ...int) cpuset.CPUSet { func (d CPUDetails) CoresInSockets(ids ...int) cpuset.CPUSet {
b := cpuset.NewBuilder() var coreIDs []int
for _, id := range ids { for _, id := range ids {
for _, info := range d { for _, info := range d {
if info.SocketID == id { if info.SocketID == id {
b.Add(info.CoreID) coreIDs = append(coreIDs, info.CoreID)
} }
} }
} }
return b.Result() return cpuset.New(coreIDs...)
} }
// CPUs returns all of the logical CPU IDs in this CPUDetails. // CPUs returns all of the logical CPU IDs in this CPUDetails.
func (d CPUDetails) CPUs() cpuset.CPUSet { func (d CPUDetails) CPUs() cpuset.CPUSet {
b := cpuset.NewBuilder() var cpuIDs []int
for cpuID := range d { for cpuID := range d {
b.Add(cpuID) cpuIDs = append(cpuIDs, cpuID)
} }
return b.Result() return cpuset.New(cpuIDs...)
} }
// CPUsInNUMANodes returns all of the logical CPU IDs associated with the given // CPUsInNUMANodes returns all of the logical CPU IDs associated with the given
// NUMANode IDs in this CPUDetails. // NUMANode IDs in this CPUDetails.
func (d CPUDetails) CPUsInNUMANodes(ids ...int) cpuset.CPUSet { func (d CPUDetails) CPUsInNUMANodes(ids ...int) cpuset.CPUSet {
b := cpuset.NewBuilder() var cpuIDs []int
for _, id := range ids { for _, id := range ids {
for cpu, info := range d { for cpu, info := range d {
if info.NUMANodeID == id { if info.NUMANodeID == id {
b.Add(cpu) cpuIDs = append(cpuIDs, cpu)
} }
} }
} }
return b.Result() return cpuset.New(cpuIDs...)
} }
// CPUsInCores returns all of the logical CPU IDs associated with the given // CPUsInCores returns all of the logical CPU IDs associated with the given
// core IDs in this CPUDetails. // core IDs in this CPUDetails.
func (d CPUDetails) CPUsInCores(ids ...int) cpuset.CPUSet { func (d CPUDetails) CPUsInCores(ids ...int) cpuset.CPUSet {
b := cpuset.NewBuilder() var cpuIDs []int
for _, id := range ids { for _, id := range ids {
for cpu, info := range d { for cpu, info := range d {
if info.CoreID == id { if info.CoreID == id {
b.Add(cpu) cpuIDs = append(cpuIDs, cpu)
} }
} }
} }
return b.Result() return cpuset.New(cpuIDs...)
} }
// Discover returns CPUTopology based on cadvisor node info // Discover returns CPUTopology based on cadvisor node info

View File

@ -25,40 +25,6 @@ import (
"strings" "strings"
) )
// Builder is a mutable builder for CPUSet. Functions that mutate instances
// of this type are not thread-safe.
type Builder struct {
result CPUSet
done bool
}
// NewBuilder returns a mutable CPUSet builder.
func NewBuilder() *Builder {
return &Builder{
result: CPUSet{
elems: map[int]struct{}{},
},
}
}
// Add adds the supplied elements to the result. Calling Add after calling
// Result has no effect.
func (b *Builder) Add(elems ...int) {
if b.done {
return
}
for _, elem := range elems {
b.result.elems[elem] = struct{}{}
}
}
// Result returns the result CPUSet containing all elements that were
// previously added to this builder. Subsequent calls to Add have no effect.
func (b *Builder) Result() CPUSet {
b.done = true
return b.result
}
// CPUSet is a thread-safe, immutable set-like data structure for CPU IDs. // CPUSet is a thread-safe, immutable set-like data structure for CPU IDs.
type CPUSet struct { type CPUSet struct {
elems map[int]struct{} elems map[int]struct{}
@ -66,11 +32,21 @@ type CPUSet struct {
// New returns a new CPUSet containing the supplied elements. // New returns a new CPUSet containing the supplied elements.
func New(cpus ...int) CPUSet { func New(cpus ...int) CPUSet {
b := NewBuilder() s := CPUSet{
for _, c := range cpus { elems: map[int]struct{}{},
b.Add(c) }
for _, c := range cpus {
s.add(c)
}
return s
}
// add adds the supplied elements to the CPUSet.
// It is intended for internal use only, since it mutates the CPUSet.
func (s CPUSet) add(elems ...int) {
for _, elem := range elems {
s.elems[elem] = struct{}{}
} }
return b.Result()
} }
// Size returns the number of elements in this set. // Size returns the number of elements in this set.
@ -98,13 +74,13 @@ func (s CPUSet) Equals(s2 CPUSet) bool {
// filter returns a new CPU set that contains all of the elements from this // filter returns a new CPU set that contains all of the elements from this
// set that match the supplied predicate, without mutating the source set. // set that match the supplied predicate, without mutating the source set.
func (s CPUSet) filter(predicate func(int) bool) CPUSet { func (s CPUSet) filter(predicate func(int) bool) CPUSet {
b := NewBuilder() r := New()
for cpu := range s.elems { for cpu := range s.elems {
if predicate(cpu) { if predicate(cpu) {
b.Add(cpu) r.add(cpu)
} }
} }
return b.Result() return r
} }
// IsSubsetOf returns true if the supplied set contains all the elements // IsSubsetOf returns true if the supplied set contains all the elements
@ -123,16 +99,16 @@ func (s CPUSet) IsSubsetOf(s2 CPUSet) bool {
// set and all of the elements from the supplied sets, without mutating // set and all of the elements from the supplied sets, without mutating
// either source set. // either source set.
func (s CPUSet) Union(s2 ...CPUSet) CPUSet { func (s CPUSet) Union(s2 ...CPUSet) CPUSet {
b := NewBuilder() r := New()
for cpu := range s.elems { for cpu := range s.elems {
b.Add(cpu) r.add(cpu)
} }
for _, cs := range s2 { for _, cs := range s2 {
for cpu := range cs.elems { for cpu := range cs.elems {
b.Add(cpu) r.add(cpu)
} }
} }
return b.Result() return r
} }
// Intersection returns a new CPU set that contains all of the elements // Intersection returns a new CPU set that contains all of the elements
@ -214,13 +190,13 @@ func (s CPUSet) String() string {
// //
// See: http://man7.org/linux/man-pages/man7/cpuset.7.html#FORMATS // See: http://man7.org/linux/man-pages/man7/cpuset.7.html#FORMATS
func Parse(s string) (CPUSet, error) { func Parse(s string) (CPUSet, error) {
b := NewBuilder()
// Handle empty string. // Handle empty string.
if s == "" { if s == "" {
return b.Result(), nil return New(), nil
} }
result := New()
// Split CPU list string: // Split CPU list string:
// "0-5,34,46-48" => ["0-5", "34", "46-48"] // "0-5,34,46-48" => ["0-5", "34", "46-48"]
ranges := strings.Split(s, ",") ranges := strings.Split(s, ",")
@ -233,7 +209,7 @@ func Parse(s string) (CPUSet, error) {
if err != nil { if err != nil {
return New(), err return New(), err
} }
b.Add(elem) result.add(elem)
} else if len(boundaries) == 2 { } else if len(boundaries) == 2 {
// Handle multi-element ranges like "0-5". // Handle multi-element ranges like "0-5".
start, err := strconv.Atoi(boundaries[0]) start, err := strconv.Atoi(boundaries[0])
@ -252,18 +228,18 @@ func Parse(s string) (CPUSet, error) {
// Add all elements to the result. // Add all elements to the result.
// e.g. "0-5", "46-48" => [0, 1, 2, 3, 4, 5, 46, 47, 48]. // e.g. "0-5", "46-48" => [0, 1, 2, 3, 4, 5, 46, 47, 48].
for e := start; e <= end; e++ { for e := start; e <= end; e++ {
b.Add(e) result.add(e)
} }
} }
} }
return b.Result(), nil return result, nil
} }
// Clone returns a copy of this CPU set. // Clone returns a copy of this CPU set.
func (s CPUSet) Clone() CPUSet { func (s CPUSet) Clone() CPUSet {
b := NewBuilder() r := New()
for elem := range s.elems { for elem := range s.elems {
b.Add(elem) r.add(elem)
} }
return b.Result() return r
} }

View File

@ -19,30 +19,8 @@ package cpuset
import ( import (
"reflect" "reflect"
"testing" "testing"
"github.com/stretchr/testify/require"
) )
func TestCPUSetBuilder(t *testing.T) {
b := NewBuilder()
elems := []int{1, 2, 3, 4, 5}
for _, elem := range elems {
b.Add(elem)
}
result := b.Result()
for _, elem := range elems {
if !result.Contains(elem) {
t.Fatalf("expected cpuset to contain element %d: [%v]", elem, result)
}
}
if len(elems) != result.Size() {
t.Fatalf("expected cpuset %s to have the same size as %v", result, elems)
}
b.Add(6)
require.False(t, result.Contains(6), "expected calls to Add after calling Result() to have no effect")
}
func TestCPUSetSize(t *testing.T) { func TestCPUSetSize(t *testing.T) {
testCases := []struct { testCases := []struct {
cpuset CPUSet cpuset CPUSet

View File

@ -733,13 +733,12 @@ func validateSMTAlignment(cpus cpuset.CPUSet, smtLevel int, pod *v1.Pod, cnt *v1
// now check all the given cpus are thread siblings. // now check all the given cpus are thread siblings.
// to do so the easiest way is to rebuild the expected set of siblings from all the cpus we got. // to do so the easiest way is to rebuild the expected set of siblings from all the cpus we got.
// if the expected set matches the given set, the given set was good. // if the expected set matches the given set, the given set was good.
b := cpuset.NewBuilder() siblingsCPUs := cpuset.New()
for _, cpuID := range cpus.UnsortedList() { for _, cpuID := range cpus.UnsortedList() {
threadSiblings, err := cpuset.Parse(strings.TrimSpace(getCPUSiblingList(int64(cpuID)))) threadSiblings, err := cpuset.Parse(strings.TrimSpace(getCPUSiblingList(int64(cpuID))))
framework.ExpectNoError(err, "parsing cpuset from logs for [%s] of pod [%s]", cnt.Name, pod.Name) framework.ExpectNoError(err, "parsing cpuset from logs for [%s] of pod [%s]", cnt.Name, pod.Name)
b.Add(threadSiblings.UnsortedList()...) siblingsCPUs = siblingsCPUs.Union(threadSiblings)
} }
siblingsCPUs := b.Result()
framework.Logf("siblings cpus: %v", siblingsCPUs) framework.Logf("siblings cpus: %v", siblingsCPUs)
if !siblingsCPUs.Equals(cpus) { if !siblingsCPUs.Equals(cpus) {

View File

@ -522,11 +522,11 @@ func podresourcesGetAllocatableResourcesTests(ctx context.Context, cli kubeletpo
resp, err := cli.GetAllocatableResources(ctx, &kubeletpodresourcesv1.AllocatableResourcesRequest{}) resp, err := cli.GetAllocatableResources(ctx, &kubeletpodresourcesv1.AllocatableResourcesRequest{})
framework.ExpectNoErrorWithOffset(1, err) framework.ExpectNoErrorWithOffset(1, err)
devs := resp.GetDevices() devs := resp.GetDevices()
b := cpuset.NewBuilder() var cpus []int
for _, cpuid := range resp.GetCpuIds() { for _, cpuid := range resp.GetCpuIds() {
b.Add(int(cpuid)) cpus = append(cpus, int(cpuid))
} }
allocatableCPUs := b.Result() allocatableCPUs := cpuset.New(cpus...)
if onlineCPUs.Size() == 0 { if onlineCPUs.Size() == 0 {
ginkgo.By("expecting no CPUs reported") ginkgo.By("expecting no CPUs reported")