From 14423bf5d858312c0e433b825f4e1939431a49f3 Mon Sep 17 00:00:00 2001 From: RainbowMango Date: Tue, 13 Dec 2022 16:57:37 +0800 Subject: [PATCH] Introduce generic hashset to simplify code Signed-off-by: RainbowMango --- .../dependencies_distributor.go | 17 ---- .../configurableinterpreter/configurable.go | 23 ++++- .../configurableinterpreter/dependencyset.go | 36 ------- .../dependencyset_test.go | 69 ------------- pkg/util/hashset/hashset.go | 29 ++++++ pkg/util/hashset/hashset_test.go | 99 +++++++++++++++++++ 6 files changed, 148 insertions(+), 125 deletions(-) delete mode 100644 pkg/resourceinterpreter/configurableinterpreter/dependencyset.go delete mode 100644 pkg/resourceinterpreter/configurableinterpreter/dependencyset_test.go create mode 100644 pkg/util/hashset/hashset.go create mode 100644 pkg/util/hashset/hashset_test.go diff --git a/pkg/dependenciesdistributor/dependencies_distributor.go b/pkg/dependenciesdistributor/dependencies_distributor.go index c69cc62e3..682cf957d 100644 --- a/pkg/dependenciesdistributor/dependencies_distributor.go +++ b/pkg/dependenciesdistributor/dependencies_distributor.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "sort" "time" corev1 "k8s.io/api/core/v1" @@ -415,22 +414,6 @@ func (d *DependenciesDistributor) syncScheduleResultToAttachedBindings(binding * } func (d *DependenciesDistributor) recordDependenciesForIndependentBinding(binding *workv1alpha2.ResourceBinding, dependencies []configv1alpha1.DependentObjectReference) error { - sort.Slice(dependencies, func(i, j int) bool { - if dependencies[i].APIVersion != dependencies[j].APIVersion { - return dependencies[i].APIVersion < dependencies[j].APIVersion - } - if dependencies[i].Kind != dependencies[j].Kind { - return dependencies[i].Kind < dependencies[j].Kind - } - if dependencies[i].Namespace != dependencies[j].Namespace { - return dependencies[i].Namespace < dependencies[j].Namespace - } - if dependencies[i].Name != dependencies[j].Name { - return dependencies[i].Name < dependencies[j].Name - } - return false - }) - dependenciesBytes, err := json.Marshal(dependencies) if err != nil { klog.Errorf("failed to marshal dependencies of binding(%s/%s): %v", binding.Namespace, binding.Name, err) diff --git a/pkg/resourceinterpreter/configurableinterpreter/configurable.go b/pkg/resourceinterpreter/configurableinterpreter/configurable.go index 882e167e3..032fe7f6b 100644 --- a/pkg/resourceinterpreter/configurableinterpreter/configurable.go +++ b/pkg/resourceinterpreter/configurableinterpreter/configurable.go @@ -1,6 +1,8 @@ package configurableinterpreter import ( + "sort" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -11,6 +13,7 @@ import ( "github.com/karmada-io/karmada/pkg/resourceinterpreter/configurableinterpreter/configmanager" "github.com/karmada-io/karmada/pkg/resourceinterpreter/configurableinterpreter/luavm" "github.com/karmada-io/karmada/pkg/util/fedinformer/genericmanager" + "github.com/karmada-io/karmada/pkg/util/hashset" ) // ConfigurableInterpreter interprets resources with resource interpreter customizations. @@ -150,7 +153,7 @@ func (c *ConfigurableInterpreter) GetDependencies(object *unstructured.Unstructu return } - referenceSet := newDependencySet() + refs := hashset.Make[configv1alpha1.DependentObjectReference]() for _, luaScript := range scripts { var references []configv1alpha1.DependentObjectReference references, err = c.luaVM.GetDependencies(object, luaScript) @@ -159,9 +162,23 @@ func (c *ConfigurableInterpreter) GetDependencies(object *unstructured.Unstructu object.GroupVersionKind(), object.GetNamespace(), object.GetName(), err) return } - referenceSet.insert(references...) + refs.Insert(references...) } - dependencies = referenceSet.list() + dependencies = refs.List() + + // keep returned items in the same order between each call. + sort.Slice(dependencies, func(i, j int) bool { + if dependencies[i].APIVersion != dependencies[j].APIVersion { + return dependencies[i].APIVersion < dependencies[j].APIVersion + } + if dependencies[i].Kind != dependencies[j].Kind { + return dependencies[i].Kind < dependencies[j].Kind + } + if dependencies[i].Namespace != dependencies[j].Namespace { + return dependencies[i].Namespace < dependencies[j].Namespace + } + return dependencies[i].Name < dependencies[j].Name + }) return } diff --git a/pkg/resourceinterpreter/configurableinterpreter/dependencyset.go b/pkg/resourceinterpreter/configurableinterpreter/dependencyset.go deleted file mode 100644 index 4d91522d9..000000000 --- a/pkg/resourceinterpreter/configurableinterpreter/dependencyset.go +++ /dev/null @@ -1,36 +0,0 @@ -package configurableinterpreter - -import ( - "fmt" - "sort" - - configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1" -) - -type dependencySet map[configv1alpha1.DependentObjectReference]struct{} - -func newDependencySet(items ...configv1alpha1.DependentObjectReference) dependencySet { - s := make(dependencySet, len(items)) - s.insert(items...) - return s -} - -func (s dependencySet) insert(items ...configv1alpha1.DependentObjectReference) dependencySet { - for _, item := range items { - s[item] = struct{}{} - } - return s -} - -func (s dependencySet) list() []configv1alpha1.DependentObjectReference { - keys := make([]configv1alpha1.DependentObjectReference, len(s)) - index := 0 - for key := range s { - keys[index] = key - index++ - } - sort.Slice(keys, func(i, j int) bool { - return fmt.Sprintf("%s", keys[i]) < fmt.Sprintf("%s", keys[j]) - }) - return keys -} diff --git a/pkg/resourceinterpreter/configurableinterpreter/dependencyset_test.go b/pkg/resourceinterpreter/configurableinterpreter/dependencyset_test.go deleted file mode 100644 index b5f65a863..000000000 --- a/pkg/resourceinterpreter/configurableinterpreter/dependencyset_test.go +++ /dev/null @@ -1,69 +0,0 @@ -package configurableinterpreter - -import ( - "reflect" - "testing" - - configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1" -) - -func Test_dependencySet_list(t *testing.T) { - tests := []struct { - name string - s dependencySet - want []configv1alpha1.DependentObjectReference - }{ - { - name: "empty set", - s: newDependencySet(), - want: []configv1alpha1.DependentObjectReference{}, - }, - { - name: "new with items", - s: newDependencySet([]configv1alpha1.DependentObjectReference{ - {APIVersion: "v1", Kind: "Secret", Namespace: "foo", Name: "foo"}, - }...), - want: []configv1alpha1.DependentObjectReference{ - {APIVersion: "v1", Kind: "Secret", Namespace: "foo", Name: "foo"}, - }, - }, - { - name: "insert with different items", - s: newDependencySet([]configv1alpha1.DependentObjectReference{ - {APIVersion: "v1", Kind: "Secret", Namespace: "foo", Name: "foo"}, - }...).insert( - []configv1alpha1.DependentObjectReference{ - {APIVersion: "v1", Kind: "Configmap", Namespace: "foo", Name: "foo"}, - }...), - want: []configv1alpha1.DependentObjectReference{ - {APIVersion: "v1", Kind: "Configmap", Namespace: "foo", Name: "foo"}, - {APIVersion: "v1", Kind: "Secret", Namespace: "foo", Name: "foo"}, - }, - }, - { - name: "insert with same items", - s: newDependencySet([]configv1alpha1.DependentObjectReference{ - {APIVersion: "v1", Kind: "Secret", Namespace: "foo", Name: "foo"}, - }...).insert( - []configv1alpha1.DependentObjectReference{ - {APIVersion: "v1", Kind: "Configmap", Namespace: "foo", Name: "foo"}, - }...).insert( - []configv1alpha1.DependentObjectReference{ - {APIVersion: "v1", Kind: "Secret", Namespace: "foo", Name: "foo"}, - {APIVersion: "apps/v1", Kind: "Deployment", Namespace: "foo", Name: "foo"}, - }...), - want: []configv1alpha1.DependentObjectReference{ - {APIVersion: "apps/v1", Kind: "Deployment", Namespace: "foo", Name: "foo"}, - {APIVersion: "v1", Kind: "Configmap", Namespace: "foo", Name: "foo"}, - {APIVersion: "v1", Kind: "Secret", Namespace: "foo", Name: "foo"}, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := tt.s.list(); !reflect.DeepEqual(got, tt.want) { - t.Errorf("dependencySet.list() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/pkg/util/hashset/hashset.go b/pkg/util/hashset/hashset.go new file mode 100644 index 000000000..4309e3018 --- /dev/null +++ b/pkg/util/hashset/hashset.go @@ -0,0 +1,29 @@ +package hashset + +// Set is a data structure that can store elements and has no repeated values. +// A set is backed by a hash table(Go's map), and is not safe for concurrent use +// by multiple goroutines without additional locking or coordination. +type Set[T comparable] map[T]struct{} + +// Make builds a hash set. +func Make[T comparable]() Set[T] { + return make(Set[T]) +} + +// Insert adds items to the set. +func (s Set[T]) Insert(items ...T) { + for _, item := range items { + s[item] = struct{}{} + } +} + +// List returns the contents as a slice. +func (s Set[T]) List() []T { + res := make([]T, 0, len(s)) + + for key := range s { + res = append(res, key) + } + + return res +} diff --git a/pkg/util/hashset/hashset_test.go b/pkg/util/hashset/hashset_test.go new file mode 100644 index 000000000..9c28949e8 --- /dev/null +++ b/pkg/util/hashset/hashset_test.go @@ -0,0 +1,99 @@ +package hashset + +import ( + "fmt" + "reflect" + "sort" + "testing" +) + +func ExampleSet_List() { + s := Make[int]() // make a set for int + s.Insert(1, 3, 2) + s.Insert(4) + l := s.List() + sort.Slice(l, func(i, j int) bool { + return l[i] < l[j] + }) + fmt.Println(l) + // Output: + // [1 2 3 4] +} + +func TestSet_List_BuiltinType(t *testing.T) { + tests := []struct { + name string + input []string + expected []string + }{ + { + name: "input nothing should list nothing", + input: []string{}, + expected: []string{}, + }, + { + name: "should guarantee no repeated items", + input: []string{"Kevin", "Jim", "Kevin", "Kevin"}, // repeat "Kevin" + expected: []string{"Jim", "Kevin"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + set := Make[string]() + set.Insert(test.input...) + list := set.List() + sort.Slice(list, func(i, j int) bool { + return list[i] < list[j] + }) + if !reflect.DeepEqual(list, test.expected) { + t.Fatalf("expected: %v, but got: %v", test.expected, list) + } + }) + } +} + +func TestSet_List_CustomType(t *testing.T) { + type CustomType struct { + Kind string + Name string + } + + tests := []struct { + name string + input []CustomType + expected []CustomType + }{ + { + name: "input nothing should list nothing", + input: []CustomType{}, + expected: []CustomType{}, + }, + { + name: "should guarantee no repeated items", + input: []CustomType{ // repeat {Kind: "k1", Name: "n1"} + {Kind: "k1", Name: "n1"}, + {Kind: "k2", Name: "n2"}, + {Kind: "k1", Name: "n1"}, + }, + expected: []CustomType{{Kind: "k1", Name: "n1"}, {Kind: "k2", Name: "n2"}}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + set := Make[CustomType]() + set.Insert(test.input...) + list := set.List() + sort.Slice(list, func(i, j int) bool { + if list[i].Kind != list[j].Kind { + return list[i].Kind < list[j].Kind + } + return list[i].Name < list[j].Name + }) + if !reflect.DeepEqual(list, test.expected) { + t.Fatalf("expected: %v, but got: %v", test.expected, list) + } + }) + } +}