From f10e76962f4d08cdde9d3991f1bab3f320f85441 Mon Sep 17 00:00:00 2001 From: nolancon Date: Tue, 15 Jan 2019 10:56:56 +0000 Subject: [PATCH 1/7] Add Socket Mask for Topology Manager --- .../topologymanager/socketmask/socketmask.go | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 pkg/kubelet/cm/topologymanager/socketmask/socketmask.go diff --git a/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go b/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go new file mode 100644 index 00000000000..02b580fd194 --- /dev/null +++ b/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go @@ -0,0 +1,154 @@ +package socketmask + +import ( + "strconv" + "strings" + "bytes" + "math" + "github.com/golang/glog" +) + + +type SocketMask []int64 + +func NewSocketMask(Mask []int64) SocketMask { + sm := Mask + return sm +} + +func (sm SocketMask) GetSocketMask(socketMask []SocketMask, maskHolder []string, count int) (SocketMask, []string) { + var socketAffinityInt64 [][]int64 + for r := range socketMask { + socketAffinityVals := []int64(socketMask[r]) + socketAffinityInt64 = append(socketAffinityInt64,socketAffinityVals) + } + if count == 0 { + maskHolder = buildMaskHolder(socketAffinityInt64) + } + glog.Infof("[socketmask] MaskHolder : %v", maskHolder) + glog.Infof("[socketmask] %v is passed into arrange function", socketMask) + arrangedMask := arrangeMask(socketAffinityInt64) + newMask := getTopologyAffinity(arrangedMask, maskHolder) + glog.Infof("[socketmask] New Mask after getTopologyAffinity (new mask) : %v ",newMask) + finalMaskValue := parseMask(newMask) + glog.Infof("[socketmask] Mask []Int64 (finalMaskValue): %v", finalMaskValue) + maskHolder = newMask + glog.Infof("[socketmask] New MaskHolder: %v", maskHolder) + finalSocketMask := SocketMask(finalMaskValue) + return finalSocketMask, maskHolder +} + +func buildMaskHolder(mask [][]int64) []string { + var maskHolder []string + outerLen := len(mask) + var innerLen int = 0 + for i := 0; i < outerLen; i++ { + if innerLen < len(mask[i]) { + innerLen = len(mask[i]) + } + } + var buffer bytes.Buffer + var i, j int = 0, 0 + for i = 0; i < outerLen; i++ { + for j = 0; j < innerLen; j++ { + buffer.WriteString("1") + } + maskHolder = append(maskHolder, buffer.String()) + buffer.Reset() + } + return maskHolder +} + +func getTopologyAffinity(arrangedMask, maskHolder []string) []string { + var topologyTemp []string + for i:= 0; i < (len(maskHolder)); i++ { + for j:= 0; j < (len(arrangedMask)); j++ { + tempStr := andOperation(maskHolder[i],arrangedMask[j]) + if strings.Contains(tempStr, "1") { + topologyTemp = append(topologyTemp, tempStr ) + } + } + } + duplicates := map[string]bool{} + for v:= range topologyTemp { + duplicates[topologyTemp[v]] = true + } + // Place all keys from the map into a slice. + topologyResult := []string{} + for key, _ := range duplicates { + topologyResult = append(topologyResult, key) + } + + return topologyResult +} + +func parseMask(mask []string) []int64 { + var maskStr string + min := strings.Count(mask[0], "1") + var num, index int + + for i := 0; i < len(mask); i++ { + num = strings.Count(mask[i], "1") + if num < min { + min = num + index = i + } + maskStr = mask[index] + } + var maskInt []int64 + for _, char := range maskStr { + convertedStr, err := strconv.Atoi(string(char)) + if err != nil { + glog.Errorf("Could not convert string to int. Err: %v", err) + return maskInt + } + maskInt = append(maskInt, int64(convertedStr)) + } + glog.Infof("[socketmask] Mask Int in Parse Mask: %v", maskInt) + return maskInt +} + +func arrangeMask(mask [][]int64) []string { + var socketStr []string + var bufferNew bytes.Buffer + outerLen := len(mask) + innerLen := len(mask[0]) + for i := 0; i < outerLen; i++ { + for j := 0; j < innerLen; j++ { + if mask[i][j] == 1 { + bufferNew.WriteString("1") + } else if mask[i][j] == 0 { + bufferNew.WriteString("0") + } + } + socketStr = append(socketStr, bufferNew.String()) + bufferNew.Reset() + } + return socketStr +} + +func andOperation(val1, val2 string) (string) { + l1, l2 := len(val1), len(val2) + //compare lengths of strings - pad shortest with trailing zeros + if l1 != l2 { + // Get the bit difference + var num int + diff := math.Abs(float64(l1) - float64(l2)) + num = int(diff) + if l1 < l2 { + val1 = val1 + strings.Repeat("0", num) + } else { + val2 = val2 + strings.Repeat("0", num) + } + } + length := len(val1) + byteArr := make([]byte, length) + for i := 0; i < length ; i++ { + byteArr[i] = (val1[i] & val2[i]) + } + var finalStr string + finalStr = string(byteArr[:]) + + return finalStr +} + From a273333f1ff4993c11ae3477c4bde02909684ffb Mon Sep 17 00:00:00 2001 From: nolancon Date: Wed, 16 Jan 2019 14:05:42 +0000 Subject: [PATCH 2/7] Add BUILD files and Boilerplates Updates based on comments * Export comments added * glog changed to klog * Other small edits --- pkg/kubelet/cm/BUILD | 1 + pkg/kubelet/cm/topologymanager/BUILD | 23 ++ .../cm/topologymanager/socketmask/BUILD | 23 ++ .../topologymanager/socketmask/socketmask.go | 257 +++++++++--------- .../cm/topologymanager/socketmask_test.go | 104 +++++++ 5 files changed, 286 insertions(+), 122 deletions(-) create mode 100644 pkg/kubelet/cm/topologymanager/BUILD create mode 100644 pkg/kubelet/cm/topologymanager/socketmask/BUILD create mode 100644 pkg/kubelet/cm/topologymanager/socketmask_test.go diff --git a/pkg/kubelet/cm/BUILD b/pkg/kubelet/cm/BUILD index ee61d44d630..016820c19b3 100644 --- a/pkg/kubelet/cm/BUILD +++ b/pkg/kubelet/cm/BUILD @@ -166,6 +166,7 @@ filegroup( "//pkg/kubelet/cm/cpumanager:all-srcs", "//pkg/kubelet/cm/cpuset:all-srcs", "//pkg/kubelet/cm/devicemanager:all-srcs", + "//pkg/kubelet/cm/topologymanager:all-srcs", "//pkg/kubelet/cm/util:all-srcs", ], tags = ["automanaged"], diff --git a/pkg/kubelet/cm/topologymanager/BUILD b/pkg/kubelet/cm/topologymanager/BUILD new file mode 100644 index 00000000000..cfb40449b3b --- /dev/null +++ b/pkg/kubelet/cm/topologymanager/BUILD @@ -0,0 +1,23 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_test") + +go_test( + name = "go_default_test", + srcs = ["socketmask_test.go"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [ + ":package-srcs", + "//pkg/kubelet/cm/topologymanager/socketmask:all-srcs", + ], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/pkg/kubelet/cm/topologymanager/socketmask/BUILD b/pkg/kubelet/cm/topologymanager/socketmask/BUILD new file mode 100644 index 00000000000..33791a89bb3 --- /dev/null +++ b/pkg/kubelet/cm/topologymanager/socketmask/BUILD @@ -0,0 +1,23 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["socketmask.go"], + importpath = "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/socketmask", + visibility = ["//visibility:public"], + deps = ["//vendor/k8s.io/klog:go_default_library"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go b/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go index 02b580fd194..2e5056ccaf9 100644 --- a/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go +++ b/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go @@ -1,154 +1,167 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package socketmask import ( + "bytes" + "k8s.io/klog" + "math" "strconv" - "strings" - "bytes" - "math" - "github.com/golang/glog" + "strings" ) - +// SocketMask represents the NUMA affinity of a socket. type SocketMask []int64 +// NewSocketMask creates a new socket mask. func NewSocketMask(Mask []int64) SocketMask { - sm := Mask - return sm + return Mask } +// GetSocketMask calculates the socket mask. func (sm SocketMask) GetSocketMask(socketMask []SocketMask, maskHolder []string, count int) (SocketMask, []string) { var socketAffinityInt64 [][]int64 - for r := range socketMask { - socketAffinityVals := []int64(socketMask[r]) - socketAffinityInt64 = append(socketAffinityInt64,socketAffinityVals) - } - if count == 0 { - maskHolder = buildMaskHolder(socketAffinityInt64) + for r := range socketMask { + socketAffinityVals := []int64(socketMask[r]) + socketAffinityInt64 = append(socketAffinityInt64, socketAffinityVals) } - glog.Infof("[socketmask] MaskHolder : %v", maskHolder) - glog.Infof("[socketmask] %v is passed into arrange function", socketMask) - arrangedMask := arrangeMask(socketAffinityInt64) - newMask := getTopologyAffinity(arrangedMask, maskHolder) - glog.Infof("[socketmask] New Mask after getTopologyAffinity (new mask) : %v ",newMask) - finalMaskValue := parseMask(newMask) - glog.Infof("[socketmask] Mask []Int64 (finalMaskValue): %v", finalMaskValue) - maskHolder = newMask - glog.Infof("[socketmask] New MaskHolder: %v", maskHolder) - finalSocketMask := SocketMask(finalMaskValue) - return finalSocketMask, maskHolder + if count == 0 { + maskHolder = buildMaskHolder(socketAffinityInt64) + } + klog.V(4).Infof("[socketmask] MaskHolder : %v", maskHolder) + klog.V(4).Infof("[socketmask] %v is passed into arrange function", socketMask) + arrangedMask := arrangeMask(socketAffinityInt64) + newMask := getTopologyAffinity(arrangedMask, maskHolder) + klog.V(4).Infof("[socketmask] New Mask after getTopologyAffinity (new mask) : %v ", newMask) + finalMaskValue := parseMask(newMask) + klog.V(4).Infof("[socketmask] Mask []Int64 (finalMaskValue): %v", finalMaskValue) + maskHolder = newMask + klog.V(4).Infof("[socketmask] New MaskHolder: %v", maskHolder) + return SocketMask(finalMaskValue), maskHolder } func buildMaskHolder(mask [][]int64) []string { - var maskHolder []string - outerLen := len(mask) - var innerLen int = 0 - for i := 0; i < outerLen; i++ { - if innerLen < len(mask[i]) { - innerLen = len(mask[i]) - } - } - var buffer bytes.Buffer - var i, j int = 0, 0 - for i = 0; i < outerLen; i++ { - for j = 0; j < innerLen; j++ { - buffer.WriteString("1") - } - maskHolder = append(maskHolder, buffer.String()) - buffer.Reset() - } - return maskHolder + outerLen := len(mask) + var innerLen int + for i := 0; i < outerLen; i++ { + if innerLen < len(mask[i]) { + innerLen = len(mask[i]) + } + } + var maskHolder []string + var buffer bytes.Buffer + var i, j int = 0, 0 + for i = 0; i < outerLen; i++ { + for j = 0; j < innerLen; j++ { + buffer.WriteString("1") + } + maskHolder = append(maskHolder, buffer.String()) + buffer.Reset() + } + return maskHolder } func getTopologyAffinity(arrangedMask, maskHolder []string) []string { - var topologyTemp []string - for i:= 0; i < (len(maskHolder)); i++ { - for j:= 0; j < (len(arrangedMask)); j++ { - tempStr := andOperation(maskHolder[i],arrangedMask[j]) - if strings.Contains(tempStr, "1") { - topologyTemp = append(topologyTemp, tempStr ) - } - } - } - duplicates := map[string]bool{} - for v:= range topologyTemp { - duplicates[topologyTemp[v]] = true - } - // Place all keys from the map into a slice. - topologyResult := []string{} - for key, _ := range duplicates { - topologyResult = append(topologyResult, key) - } + var topologyTemp []string + for i := 0; i < len(maskHolder); i++ { + for j := 0; j < len(arrangedMask); j++ { + tempStr := andOperation(maskHolder[i], arrangedMask[j]) + if strings.Contains(tempStr, "1") { + topologyTemp = append(topologyTemp, tempStr) + } + } + } + duplicates := map[string]bool{} + for v := range topologyTemp { + duplicates[topologyTemp[v]] = true + } + // Place all keys from the map into a slice. + topologyResult := []string{} + for key := range duplicates { + topologyResult = append(topologyResult, key) + } - return topologyResult + return topologyResult } func parseMask(mask []string) []int64 { - var maskStr string - min := strings.Count(mask[0], "1") - var num, index int + var maskStr string + min := strings.Count(mask[0], "1") + var num, index int - for i := 0; i < len(mask); i++ { - num = strings.Count(mask[i], "1") - if num < min { - min = num - index = i - } - maskStr = mask[index] - } - var maskInt []int64 - for _, char := range maskStr { - convertedStr, err := strconv.Atoi(string(char)) - if err != nil { - glog.Errorf("Could not convert string to int. Err: %v", err) - return maskInt - } - maskInt = append(maskInt, int64(convertedStr)) - } - glog.Infof("[socketmask] Mask Int in Parse Mask: %v", maskInt) - return maskInt + for i := 0; i < len(mask); i++ { + num = strings.Count(mask[i], "1") + if num < min { + min = num + index = i + } + maskStr = mask[index] + } + var maskInt []int64 + for _, char := range maskStr { + convertedStr, err := strconv.Atoi(string(char)) + if err != nil { + klog.V(4).Infof("could not convert mask character: %v", err) + return maskInt + } + maskInt = append(maskInt, int64(convertedStr)) + } + klog.V(4).Infof("[socketmask] Mask Int in Parse Mask: %v", maskInt) + return maskInt } func arrangeMask(mask [][]int64) []string { - var socketStr []string - var bufferNew bytes.Buffer - outerLen := len(mask) - innerLen := len(mask[0]) - for i := 0; i < outerLen; i++ { - for j := 0; j < innerLen; j++ { - if mask[i][j] == 1 { - bufferNew.WriteString("1") - } else if mask[i][j] == 0 { - bufferNew.WriteString("0") - } - } - socketStr = append(socketStr, bufferNew.String()) - bufferNew.Reset() - } - return socketStr + var socketStr []string + var bufferNew bytes.Buffer + outerLen := len(mask) + innerLen := len(mask[0]) + for i := 0; i < outerLen; i++ { + for j := 0; j < innerLen; j++ { + if mask[i][j] == 1 { + bufferNew.WriteString("1") + } else if mask[i][j] == 0 { + bufferNew.WriteString("0") + } + } + socketStr = append(socketStr, bufferNew.String()) + bufferNew.Reset() + } + return socketStr } -func andOperation(val1, val2 string) (string) { - l1, l2 := len(val1), len(val2) - //compare lengths of strings - pad shortest with trailing zeros - if l1 != l2 { - // Get the bit difference - var num int - diff := math.Abs(float64(l1) - float64(l2)) - num = int(diff) - if l1 < l2 { - val1 = val1 + strings.Repeat("0", num) - } else { - val2 = val2 + strings.Repeat("0", num) - } - } - length := len(val1) - byteArr := make([]byte, length) - for i := 0; i < length ; i++ { - byteArr[i] = (val1[i] & val2[i]) - } - var finalStr string - finalStr = string(byteArr[:]) +func andOperation(val1, val2 string) string { + l1, l2 := len(val1), len(val2) + //compare lengths of strings - pad shortest with trailing zeros + if l1 != l2 { + // Get the bit difference + var num int + diff := math.Abs(float64(l1) - float64(l2)) + num = int(diff) + if l1 < l2 { + val1 = val1 + strings.Repeat("0", num) + } else { + val2 = val2 + strings.Repeat("0", num) + } + } + length := len(val1) + byteArr := make([]byte, length) + for i := 0; i < length; i++ { + byteArr[i] = val1[i] & val2[i] + } - return finalStr + return string(byteArr[:]) } - diff --git a/pkg/kubelet/cm/topologymanager/socketmask_test.go b/pkg/kubelet/cm/topologymanager/socketmask_test.go new file mode 100644 index 00000000000..69a1efedf9e --- /dev/null +++ b/pkg/kubelet/cm/topologymanager/socketmask_test.go @@ -0,0 +1,104 @@ +/* +Copyright 2015 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package socketmask + +import ( + "reflect" + "testing" +) + +func TestGetSocketmask(t *testing.T) { + tcases := []struct { + name string + socketmask []SocketMask + maskholder []string + count int + expectedMaskHolder []string + expectedFinalSocketMask SocketMask + }{ + { + name: "Empty MaskHolder and count set as 0", + socketmask: []SocketMask{{1, 0}, {0, 1}, {1, 1}}, + maskholder: []string{""}, + count: 0, + expectedMaskHolder: []string{"10", "01", "11"}, + expectedFinalSocketMask: SocketMask{1, 0}, + }, + { + name: "MaskHolder non zero, count set as 1", + socketmask: []SocketMask{{1, 0}, {0, 1}, {1, 1}}, + maskholder: []string{"10", "01", "11"}, + count: 1, + expectedMaskHolder: []string{"10", "01", "11"}, + expectedFinalSocketMask: SocketMask{1, 0}, + }, + + { + name: "Empty MaskHolder and count set as 0", + socketmask: []SocketMask{{0, 1}, {1, 1}}, + maskholder: []string{""}, + count: 0, + expectedMaskHolder: []string{"01", "11"}, + expectedFinalSocketMask: SocketMask{0, 1}, + }, + { + name: "MaskHolder non zero, count set as 1", + socketmask: []SocketMask{{0, 1}, {1, 1}}, + maskholder: []string{"01", "11"}, + count: 1, + expectedMaskHolder: []string{"01", "11"}, + expectedFinalSocketMask: SocketMask{0, 1}, + }, + } + + for _, tc := range tcases { + + sm := NewSocketMask(nil) + actualSocketMask, actualMaskHolder := sm.GetSocketMask(tc.socketmask, tc.maskholder, tc.count) + if !reflect.DeepEqual(actualSocketMask, tc.expectedFinalSocketMask) { + t.Errorf("Expected final socketmask to be %v, got %v", tc.expectedFinalSocketMask, actualSocketMask) + } + + if !reflect.DeepEqual(actualMaskHolder, tc.expectedMaskHolder) { + t.Errorf("Expected maskholder to be %v, got %v", tc.expectedMaskHolder, actualMaskHolder) + } + + } +} + +func TestNewSocketMask(t *testing.T) { + tcases := []struct { + name string + Mask []int64 + expectedMask []int64 + }{ + { + name: "Mask as an int64 binary array", + Mask: []int64{1, 0}, + expectedMask: []int64{1, 0}, + }, + } + + for _, tc := range tcases { + + sm := NewSocketMask(nil) + + if reflect.DeepEqual(sm, tc.expectedMask) { + t.Errorf("Expected socket mask to be %v, got %v", tc.expectedMask, sm) + } + } +} From ce56956409b19645daadcf422e0bf1157340fa85 Mon Sep 17 00:00:00 2001 From: Sreemanti-Ghosh <48021426+Sreemanti-Ghosh@users.noreply.github.com> Date: Tue, 5 Mar 2019 15:59:36 +0000 Subject: [PATCH 3/7] Socket mask unit test (#4) --- pkg/kubelet/cm/BUILD | 2 +- pkg/kubelet/cm/topologymanager/BUILD | 23 ------------------- .../cm/topologymanager/socketmask/BUILD | 8 ++++++- .../{ => socketmask}/socketmask_test.go | 0 4 files changed, 8 insertions(+), 25 deletions(-) delete mode 100644 pkg/kubelet/cm/topologymanager/BUILD rename pkg/kubelet/cm/topologymanager/{ => socketmask}/socketmask_test.go (100%) diff --git a/pkg/kubelet/cm/BUILD b/pkg/kubelet/cm/BUILD index 016820c19b3..7cb81e90690 100644 --- a/pkg/kubelet/cm/BUILD +++ b/pkg/kubelet/cm/BUILD @@ -166,7 +166,7 @@ filegroup( "//pkg/kubelet/cm/cpumanager:all-srcs", "//pkg/kubelet/cm/cpuset:all-srcs", "//pkg/kubelet/cm/devicemanager:all-srcs", - "//pkg/kubelet/cm/topologymanager:all-srcs", + "//pkg/kubelet/cm/topologymanager/socketmask:all-srcs", "//pkg/kubelet/cm/util:all-srcs", ], tags = ["automanaged"], diff --git a/pkg/kubelet/cm/topologymanager/BUILD b/pkg/kubelet/cm/topologymanager/BUILD deleted file mode 100644 index cfb40449b3b..00000000000 --- a/pkg/kubelet/cm/topologymanager/BUILD +++ /dev/null @@ -1,23 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_test") - -go_test( - name = "go_default_test", - srcs = ["socketmask_test.go"], -) - -filegroup( - name = "package-srcs", - srcs = glob(["**"]), - tags = ["automanaged"], - visibility = ["//visibility:private"], -) - -filegroup( - name = "all-srcs", - srcs = [ - ":package-srcs", - "//pkg/kubelet/cm/topologymanager/socketmask:all-srcs", - ], - tags = ["automanaged"], - visibility = ["//visibility:public"], -) diff --git a/pkg/kubelet/cm/topologymanager/socketmask/BUILD b/pkg/kubelet/cm/topologymanager/socketmask/BUILD index 33791a89bb3..0e49d414b8c 100644 --- a/pkg/kubelet/cm/topologymanager/socketmask/BUILD +++ b/pkg/kubelet/cm/topologymanager/socketmask/BUILD @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -21,3 +21,9 @@ filegroup( tags = ["automanaged"], visibility = ["//visibility:public"], ) + +go_test( + name = "go_default_test", + srcs = ["socketmask_test.go"], + embed = [":go_default_library"], +) diff --git a/pkg/kubelet/cm/topologymanager/socketmask_test.go b/pkg/kubelet/cm/topologymanager/socketmask/socketmask_test.go similarity index 100% rename from pkg/kubelet/cm/topologymanager/socketmask_test.go rename to pkg/kubelet/cm/topologymanager/socketmask/socketmask_test.go From 7c525ffaa8c4222d9a882052667bb451f3d5a3e8 Mon Sep 17 00:00:00 2001 From: nolancon Date: Wed, 8 May 2019 04:22:39 +0100 Subject: [PATCH 4/7] More intuitive TopologyHints - socketmask.go --- .../cm/topologymanager/socketmask/socketmask.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go b/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go index 2e5056ccaf9..0c0a213d65b 100644 --- a/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go +++ b/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go @@ -33,18 +33,13 @@ func NewSocketMask(Mask []int64) SocketMask { } // GetSocketMask calculates the socket mask. -func (sm SocketMask) GetSocketMask(socketMask []SocketMask, maskHolder []string, count int) (SocketMask, []string) { - var socketAffinityInt64 [][]int64 - for r := range socketMask { - socketAffinityVals := []int64(socketMask[r]) - socketAffinityInt64 = append(socketAffinityInt64, socketAffinityVals) - } +func (sm SocketMask) GetSocketMask(socketMaskInt64 [][]int64, maskHolder []string, count int) (SocketMask, []string) { if count == 0 { - maskHolder = buildMaskHolder(socketAffinityInt64) + maskHolder = buildMaskHolder(socketMaskInt64) } klog.V(4).Infof("[socketmask] MaskHolder : %v", maskHolder) klog.V(4).Infof("[socketmask] %v is passed into arrange function", socketMask) - arrangedMask := arrangeMask(socketAffinityInt64) + arrangedMask := arrangeMask(socketMaskInt64) newMask := getTopologyAffinity(arrangedMask, maskHolder) klog.V(4).Infof("[socketmask] New Mask after getTopologyAffinity (new mask) : %v ", newMask) finalMaskValue := parseMask(newMask) From e8566caa3fc7a9fe073f5c805a81161c43c3cb99 Mon Sep 17 00:00:00 2001 From: nolancon Date: Fri, 10 May 2019 03:53:46 +0100 Subject: [PATCH 5/7] Update to unit test and comment bug fixed --- .../cm/topologymanager/socketmask/socketmask.go | 2 +- .../cm/topologymanager/socketmask/socketmask_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go b/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go index 0c0a213d65b..c6f483932df 100644 --- a/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go +++ b/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go @@ -38,7 +38,7 @@ func (sm SocketMask) GetSocketMask(socketMaskInt64 [][]int64, maskHolder []strin maskHolder = buildMaskHolder(socketMaskInt64) } klog.V(4).Infof("[socketmask] MaskHolder : %v", maskHolder) - klog.V(4).Infof("[socketmask] %v is passed into arrange function", socketMask) + klog.V(4).Infof("[socketmask] %v is passed into arrange function", socketMaskInt64) arrangedMask := arrangeMask(socketMaskInt64) newMask := getTopologyAffinity(arrangedMask, maskHolder) klog.V(4).Infof("[socketmask] New Mask after getTopologyAffinity (new mask) : %v ", newMask) diff --git a/pkg/kubelet/cm/topologymanager/socketmask/socketmask_test.go b/pkg/kubelet/cm/topologymanager/socketmask/socketmask_test.go index 69a1efedf9e..624221a931a 100644 --- a/pkg/kubelet/cm/topologymanager/socketmask/socketmask_test.go +++ b/pkg/kubelet/cm/topologymanager/socketmask/socketmask_test.go @@ -24,7 +24,7 @@ import ( func TestGetSocketmask(t *testing.T) { tcases := []struct { name string - socketmask []SocketMask + socketmask [][]int64 maskholder []string count int expectedMaskHolder []string @@ -32,7 +32,7 @@ func TestGetSocketmask(t *testing.T) { }{ { name: "Empty MaskHolder and count set as 0", - socketmask: []SocketMask{{1, 0}, {0, 1}, {1, 1}}, + socketmask: [][]int64{{1, 0}, {0, 1}, {1, 1}}, maskholder: []string{""}, count: 0, expectedMaskHolder: []string{"10", "01", "11"}, @@ -40,7 +40,7 @@ func TestGetSocketmask(t *testing.T) { }, { name: "MaskHolder non zero, count set as 1", - socketmask: []SocketMask{{1, 0}, {0, 1}, {1, 1}}, + socketmask: [][]int64{{1, 0}, {0, 1}, {1, 1}}, maskholder: []string{"10", "01", "11"}, count: 1, expectedMaskHolder: []string{"10", "01", "11"}, @@ -49,7 +49,7 @@ func TestGetSocketmask(t *testing.T) { { name: "Empty MaskHolder and count set as 0", - socketmask: []SocketMask{{0, 1}, {1, 1}}, + socketmask: [][]int64{{0, 1}, {1, 1}}, maskholder: []string{""}, count: 0, expectedMaskHolder: []string{"01", "11"}, @@ -57,7 +57,7 @@ func TestGetSocketmask(t *testing.T) { }, { name: "MaskHolder non zero, count set as 1", - socketmask: []SocketMask{{0, 1}, {1, 1}}, + socketmask: [][]int64{{0, 1}, {1, 1}}, maskholder: []string{"01", "11"}, count: 1, expectedMaskHolder: []string{"01", "11"}, From 283dff9335011fdc30aef0051af346123d9abf54 Mon Sep 17 00:00:00 2001 From: nolancon Date: Mon, 27 May 2019 04:23:13 +0100 Subject: [PATCH 6/7] Update SocketMask based on feedback TODO: Unit tests to be updated --- .../cm/topologymanager/socketmask/BUILD | 1 - .../topologymanager/socketmask/socketmask.go | 226 +++++++++--------- 2 files changed, 108 insertions(+), 119 deletions(-) diff --git a/pkg/kubelet/cm/topologymanager/socketmask/BUILD b/pkg/kubelet/cm/topologymanager/socketmask/BUILD index 0e49d414b8c..25bd381e862 100644 --- a/pkg/kubelet/cm/topologymanager/socketmask/BUILD +++ b/pkg/kubelet/cm/topologymanager/socketmask/BUILD @@ -5,7 +5,6 @@ go_library( srcs = ["socketmask.go"], importpath = "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/socketmask", visibility = ["//visibility:public"], - deps = ["//vendor/k8s.io/klog:go_default_library"], ) filegroup( diff --git a/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go b/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go index c6f483932df..8e74ce0291a 100644 --- a/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go +++ b/pkg/kubelet/cm/topologymanager/socketmask/socketmask.go @@ -17,146 +17,136 @@ limitations under the License. package socketmask import ( - "bytes" - "k8s.io/klog" - "math" - "strconv" - "strings" + "fmt" ) -// SocketMask represents the NUMA affinity of a socket. -type SocketMask []int64 - -// NewSocketMask creates a new socket mask. -func NewSocketMask(Mask []int64) SocketMask { - return Mask +//SocketMask interface allows hint providers to create SocketMasks for TopologyHints +type SocketMask interface { + Add(sockets ...int) error + Remove(sockets ...int) error + And(masks ...SocketMask) + Or(masks ...SocketMask) + Clear() + Fill() + IsEqual(mask SocketMask) bool + IsEmpty() bool + IsSet(socket int) bool + String() string + Count() int + GetSockets() []int } -// GetSocketMask calculates the socket mask. -func (sm SocketMask) GetSocketMask(socketMaskInt64 [][]int64, maskHolder []string, count int) (SocketMask, []string) { - if count == 0 { - maskHolder = buildMaskHolder(socketMaskInt64) +type socketMask uint64 + +//NewSocketMask creates a new SocketMask +func NewSocketMask(sockets ...int) (SocketMask, error) { + s := socketMask(0) + err := (&s).Add(sockets...) + if err != nil { + return nil, err } - klog.V(4).Infof("[socketmask] MaskHolder : %v", maskHolder) - klog.V(4).Infof("[socketmask] %v is passed into arrange function", socketMaskInt64) - arrangedMask := arrangeMask(socketMaskInt64) - newMask := getTopologyAffinity(arrangedMask, maskHolder) - klog.V(4).Infof("[socketmask] New Mask after getTopologyAffinity (new mask) : %v ", newMask) - finalMaskValue := parseMask(newMask) - klog.V(4).Infof("[socketmask] Mask []Int64 (finalMaskValue): %v", finalMaskValue) - maskHolder = newMask - klog.V(4).Infof("[socketmask] New MaskHolder: %v", maskHolder) - return SocketMask(finalMaskValue), maskHolder + return &s, nil } -func buildMaskHolder(mask [][]int64) []string { - outerLen := len(mask) - var innerLen int - for i := 0; i < outerLen; i++ { - if innerLen < len(mask[i]) { - innerLen = len(mask[i]) +//Add adds the sockets with topology affinity to the SocketMask +func (s *socketMask) Add(sockets ...int) error { + mask := *s + for _, i := range sockets { + if i < 0 || i >= 64 { + return fmt.Errorf("socket number must be in range 0-63") } + mask |= 1 << uint64(i) } - var maskHolder []string - var buffer bytes.Buffer - var i, j int = 0, 0 - for i = 0; i < outerLen; i++ { - for j = 0; j < innerLen; j++ { - buffer.WriteString("1") - } - maskHolder = append(maskHolder, buffer.String()) - buffer.Reset() - } - return maskHolder + *s = mask + return nil } -func getTopologyAffinity(arrangedMask, maskHolder []string) []string { - var topologyTemp []string - for i := 0; i < len(maskHolder); i++ { - for j := 0; j < len(arrangedMask); j++ { - tempStr := andOperation(maskHolder[i], arrangedMask[j]) - if strings.Contains(tempStr, "1") { - topologyTemp = append(topologyTemp, tempStr) - } +//Remove removes specified sockets from SocketMask +func (s *socketMask) Remove(sockets ...int) error { + mask := *s + for _, i := range sockets { + if i < 0 || i >= 64 { + return fmt.Errorf("socket number must be in range 0-63") } + mask &^= 1 << uint64(i) } - duplicates := map[string]bool{} - for v := range topologyTemp { - duplicates[topologyTemp[v]] = true - } - // Place all keys from the map into a slice. - topologyResult := []string{} - for key := range duplicates { - topologyResult = append(topologyResult, key) - } - - return topologyResult + *s = mask + return nil } -func parseMask(mask []string) []int64 { - var maskStr string - min := strings.Count(mask[0], "1") - var num, index int - - for i := 0; i < len(mask); i++ { - num = strings.Count(mask[i], "1") - if num < min { - min = num - index = i - } - maskStr = mask[index] +//And performs and operation on all bits in masks +func (s *socketMask) And(masks ...SocketMask) { + for _, m := range masks { + *s &= *m.(*socketMask) } - var maskInt []int64 - for _, char := range maskStr { - convertedStr, err := strconv.Atoi(string(char)) - if err != nil { - klog.V(4).Infof("could not convert mask character: %v", err) - return maskInt - } - maskInt = append(maskInt, int64(convertedStr)) - } - klog.V(4).Infof("[socketmask] Mask Int in Parse Mask: %v", maskInt) - return maskInt } -func arrangeMask(mask [][]int64) []string { - var socketStr []string - var bufferNew bytes.Buffer - outerLen := len(mask) - innerLen := len(mask[0]) - for i := 0; i < outerLen; i++ { - for j := 0; j < innerLen; j++ { - if mask[i][j] == 1 { - bufferNew.WriteString("1") - } else if mask[i][j] == 0 { - bufferNew.WriteString("0") - } - } - socketStr = append(socketStr, bufferNew.String()) - bufferNew.Reset() +//Or performs or operation on all bits in masks +func (s *socketMask) Or(masks ...SocketMask) { + for _, m := range masks { + *s |= *m.(*socketMask) } - return socketStr } -func andOperation(val1, val2 string) string { - l1, l2 := len(val1), len(val2) - //compare lengths of strings - pad shortest with trailing zeros - if l1 != l2 { - // Get the bit difference - var num int - diff := math.Abs(float64(l1) - float64(l2)) - num = int(diff) - if l1 < l2 { - val1 = val1 + strings.Repeat("0", num) +//Clear resets all bits in mask to zero +func (s *socketMask) Clear() { + *s = 0 +} + +//Fill sets all bits in mask to one +func (s *socketMask) Fill() { + *s = socketMask(^uint64(0)) +} + +//IsEmpty checks mask to see if all bits are zero +func (s *socketMask) IsEmpty() bool { + return *s == 0 +} + +//IsSet checks socket in mask to see if bit is set to one +func (s *socketMask) IsSet(socket int) bool { + if socket < 0 || socket >= 64 { + return false + } + return (*s & (1 << uint64(socket))) > 0 +} + +//IsEqual checks if masks are equal +func (s *socketMask) IsEqual(mask SocketMask) bool { + return *s == *mask.(*socketMask) +} + +//String converts mask to string +func (s *socketMask) String() string { + str := "" + for i := uint64(0); i < 64; i++ { + if (*s & (1 << i)) > 0 { + str += "1" } else { - val2 = val2 + strings.Repeat("0", num) + str += "0" } } - length := len(val1) - byteArr := make([]byte, length) - for i := 0; i < length; i++ { - byteArr[i] = val1[i] & val2[i] - } - - return string(byteArr[:]) + return str +} + +//Count counts number of bits in mask set to one +func (s *socketMask) Count() int { + count := 0 + for i := uint64(0); i < 64; i++ { + if (*s & (1 << i)) > 0 { + count++ + } + } + return count +} + +//GetSockets returns each socket number with bits set to one +func (s *socketMask) GetSockets() []int { + var sockets []int + for i := uint64(0); i < 64; i++ { + if (*s & (1 << i)) > 0 { + sockets = append(sockets, int(i)) + } + } + return sockets } From b7f6b8f8f1868309a06f307914e58e02f074f03e Mon Sep 17 00:00:00 2001 From: nolancon Date: Tue, 28 May 2019 05:00:04 +0100 Subject: [PATCH 7/7] Updated unit test for socketmask --- .../socketmask/socketmask_test.go | 328 ++++++++++++++---- 1 file changed, 257 insertions(+), 71 deletions(-) diff --git a/pkg/kubelet/cm/topologymanager/socketmask/socketmask_test.go b/pkg/kubelet/cm/topologymanager/socketmask/socketmask_test.go index 624221a931a..01df68999c0 100644 --- a/pkg/kubelet/cm/topologymanager/socketmask/socketmask_test.go +++ b/pkg/kubelet/cm/topologymanager/socketmask/socketmask_test.go @@ -1,5 +1,5 @@ /* -Copyright 2015 The Kubernetes Authors. +Copyright 2019 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -21,84 +21,270 @@ import ( "testing" ) -func TestGetSocketmask(t *testing.T) { - tcases := []struct { - name string - socketmask [][]int64 - maskholder []string - count int - expectedMaskHolder []string - expectedFinalSocketMask SocketMask - }{ - { - name: "Empty MaskHolder and count set as 0", - socketmask: [][]int64{{1, 0}, {0, 1}, {1, 1}}, - maskholder: []string{""}, - count: 0, - expectedMaskHolder: []string{"10", "01", "11"}, - expectedFinalSocketMask: SocketMask{1, 0}, - }, - { - name: "MaskHolder non zero, count set as 1", - socketmask: [][]int64{{1, 0}, {0, 1}, {1, 1}}, - maskholder: []string{"10", "01", "11"}, - count: 1, - expectedMaskHolder: []string{"10", "01", "11"}, - expectedFinalSocketMask: SocketMask{1, 0}, - }, - - { - name: "Empty MaskHolder and count set as 0", - socketmask: [][]int64{{0, 1}, {1, 1}}, - maskholder: []string{""}, - count: 0, - expectedMaskHolder: []string{"01", "11"}, - expectedFinalSocketMask: SocketMask{0, 1}, - }, - { - name: "MaskHolder non zero, count set as 1", - socketmask: [][]int64{{0, 1}, {1, 1}}, - maskholder: []string{"01", "11"}, - count: 1, - expectedMaskHolder: []string{"01", "11"}, - expectedFinalSocketMask: SocketMask{0, 1}, - }, - } - - for _, tc := range tcases { - - sm := NewSocketMask(nil) - actualSocketMask, actualMaskHolder := sm.GetSocketMask(tc.socketmask, tc.maskholder, tc.count) - if !reflect.DeepEqual(actualSocketMask, tc.expectedFinalSocketMask) { - t.Errorf("Expected final socketmask to be %v, got %v", tc.expectedFinalSocketMask, actualSocketMask) - } - - if !reflect.DeepEqual(actualMaskHolder, tc.expectedMaskHolder) { - t.Errorf("Expected maskholder to be %v, got %v", tc.expectedMaskHolder, actualMaskHolder) - } - - } -} - func TestNewSocketMask(t *testing.T) { tcases := []struct { name string - Mask []int64 - expectedMask []int64 + socket int + expectedMask string }{ { - name: "Mask as an int64 binary array", - Mask: []int64{1, 0}, - expectedMask: []int64{1, 0}, + name: "New SocketMask with socket 0 set", + socket: int(0), + expectedMask: "1000000000000000000000000000000000000000000000000000000000000000", }, } - for _, tc := range tcases { - - sm := NewSocketMask(nil) - - if reflect.DeepEqual(sm, tc.expectedMask) { - t.Errorf("Expected socket mask to be %v, got %v", tc.expectedMask, sm) + sm, _ := NewSocketMask(0) + if sm.String() != tc.expectedMask { + t.Errorf("Expected mask to be %v, got %v", tc.expectedMask, sm) + } + } +} + +func TestAdd(t *testing.T) { + tcases := []struct { + name string + firstSocket int + secondSocket int + expectedMask string + }{ + { + name: "Reset bit 1 SocketMask to 0", + firstSocket: 0, + secondSocket: 1, + expectedMask: "1100000000000000000000000000000000000000000000000000000000000000", + }, + } + for _, tc := range tcases { + mask, _ := NewSocketMask() + mask.Add(tc.firstSocket, tc.secondSocket) + if mask.String() != tc.expectedMask { + t.Errorf("Expected mask to be %v, got %v", tc.expectedMask, mask) + } + } +} + +func TestRemove(t *testing.T) { + tcases := []struct { + name string + firstSocketSet int + secondSocketSet int + firstSocketRemove int + expectedMask string + }{ + { + name: "Reset bit 1 SocketMask to 0", + firstSocketSet: 0, + secondSocketSet: 1, + firstSocketRemove: 0, + expectedMask: "0100000000000000000000000000000000000000000000000000000000000000", + }, + } + for _, tc := range tcases { + mask, _ := NewSocketMask(tc.firstSocketSet, tc.secondSocketSet) + mask.Remove(tc.firstSocketRemove) + if mask.String() != tc.expectedMask { + t.Errorf("Expected mask to be %v, got %v", tc.expectedMask, mask) + } + } +} + +func TestAnd(t *testing.T) { + tcases := []struct { + name string + firstMaskBit int + secondMaskBit int + andMask string + }{ + { + name: "And socket masks", + firstMaskBit: 0, + secondMaskBit: 0, + andMask: "1000000000000000000000000000000000000000000000000000000000000000", + }, + } + for _, tc := range tcases { + firstMask, _ := NewSocketMask(tc.firstMaskBit) + secondMask, _ := NewSocketMask(tc.secondMaskBit) + firstMask.And(secondMask) + if firstMask.String() != string(tc.andMask) { + t.Errorf("Expected mask to be %v, got %v", tc.andMask, firstMask) + } + } +} + +func TestOr(t *testing.T) { + tcases := []struct { + name string + firstMaskBit int + secondMaskBit int + orMask string + }{ + { + name: "Or socket masks", + firstMaskBit: int(0), + secondMaskBit: int(1), + orMask: "1100000000000000000000000000000000000000000000000000000000000000", + }, + } + for _, tc := range tcases { + firstMask, _ := NewSocketMask(tc.firstMaskBit) + secondMask, _ := NewSocketMask(tc.secondMaskBit) + firstMask.Or(secondMask) + if firstMask.String() != string(tc.orMask) { + t.Errorf("Expected mask to be %v, got %v", tc.orMask, firstMask) + } + } +} + +func TestClear(t *testing.T) { + tcases := []struct { + name string + firstBit int + secondBit int + clearedMask string + }{ + { + name: "Clear socket masks", + firstBit: int(0), + secondBit: int(1), + clearedMask: "0000000000000000000000000000000000000000000000000000000000000000", + }, + } + for _, tc := range tcases { + mask, _ := NewSocketMask(tc.firstBit, tc.secondBit) + mask.Clear() + if mask.String() != string(tc.clearedMask) { + t.Errorf("Expected mask to be %v, got %v", tc.clearedMask, mask) + } + } +} + +func TestFill(t *testing.T) { + tcases := []struct { + name string + filledMask string + }{ + { + name: "Fill socket masks", + filledMask: "1111111111111111111111111111111111111111111111111111111111111111", + }, + } + for _, tc := range tcases { + mask, _ := NewSocketMask() + mask.Fill() + if mask.String() != string(tc.filledMask) { + t.Errorf("Expected mask to be %v, got %v", tc.filledMask, mask) + } + } +} + +func TestIsEmpty(t *testing.T) { + tcases := []struct { + name string + maskBit int + expectedEmpty bool + }{ + { + name: "Check if mask is empty", + maskBit: int(0), + expectedEmpty: false, + }, + } + for _, tc := range tcases { + mask, _ := NewSocketMask(tc.maskBit) + empty := mask.IsEmpty() + if empty { + t.Errorf("Expected value to be %v, got %v", tc.expectedEmpty, empty) + } + } +} + +func TestIsSet(t *testing.T) { + tcases := []struct { + name string + maskBit int + expectedSet bool + }{ + { + name: "Check if mask bit is set", + maskBit: int(0), + expectedSet: true, + }, + } + for _, tc := range tcases { + mask, _ := NewSocketMask(tc.maskBit) + set := mask.IsSet(tc.maskBit) + if !set { + t.Errorf("Expected value to be %v, got %v", tc.expectedSet, set) + } + } +} + +func TestIsEqual(t *testing.T) { + tcases := []struct { + name string + firstMaskBit int + secondMaskBit int + isEqual bool + }{ + { + name: "And socket masks", + firstMaskBit: int(0), + secondMaskBit: int(0), + isEqual: true, + }, + } + for _, tc := range tcases { + firstMask, _ := NewSocketMask(tc.firstMaskBit) + secondMask, _ := NewSocketMask(tc.secondMaskBit) + isEqual := firstMask.IsEqual(secondMask) + if !isEqual { + t.Errorf("Expected mask to be %v, got %v", tc.isEqual, isEqual) + } + } +} + +func TestCount(t *testing.T) { + tcases := []struct { + name string + maskBit int + expectedCount int + }{ + { + name: "Count number of bits set in full mask", + maskBit: 0, + expectedCount: 1, + }, + } + for _, tc := range tcases { + mask, _ := NewSocketMask(tc.maskBit) + count := mask.Count() + if count != tc.expectedCount { + t.Errorf("Expected value to be %v, got %v", tc.expectedCount, count) + } + } +} + +func TestGetSockets(t *testing.T) { + tcases := []struct { + name string + firstSocket int + secondSocket int + expectedSockets []int + }{ + { + name: "Get number of each socket which has been set", + firstSocket: 0, + secondSocket: 1, + expectedSockets: []int{0, 1}, + }, + } + for _, tc := range tcases { + mask, _ := NewSocketMask(tc.firstSocket, tc.secondSocket) + sockets := mask.GetSockets() + if !reflect.DeepEqual(sockets, tc.expectedSockets) { + t.Errorf("Expected value to be %v, got %v", tc.expectedSockets, sockets) } } }