diff --git a/changelog/v0.39.1/efficient-union.yaml b/changelog/v0.39.1/efficient-union.yaml new file mode 100644 index 000000000..1ef92df86 --- /dev/null +++ b/changelog/v0.39.1/efficient-union.yaml @@ -0,0 +1,5 @@ +changelog: + - type: NON_USER_FACING + description: > + "Makes the `Union` method in v2 sets more efficient. If the input set is sorted by resource ID, we can take advantage of this to + do a more efficient Union while keeping the Unioned set in sorted order. diff --git a/contrib/pkg/sets/v2/sets.go b/contrib/pkg/sets/v2/sets.go index 1c8702586..b68d7279e 100644 --- a/contrib/pkg/sets/v2/sets.go +++ b/contrib/pkg/sets/v2/sets.go @@ -229,13 +229,48 @@ func (s *resourceSet[T]) Delete(resource ezkube.ResourceId) { } func (s *resourceSet[T]) Union(set ResourceSet[T]) ResourceSet[T] { - list := []T{} - for _, resource := range s.Generic().Union(set.Generic()).List() { - list = append(list, resource.(T)) + + if s == nil && set == nil { + return NewResourceSet[T]() } - return NewResourceSet[T]( - list..., - ) + if s == nil { + return set.ShallowCopy() + } + if set == nil { + return s.ShallowCopy() + } + + return s.unionSortedSet(set) +} + +// Assuming that the argument set is sorted by resource id, +// this method will efficiently union the two sets together and return the unioned set. +func (s *resourceSet[T]) unionSortedSet(set ResourceSet[T]) *resourceSet[T] { + merged := make([]T, 0, len(s.set)+set.Len()) + idx := 0 + + // Iterate through the second set + set.Iter(func(_ int, resource T) bool { + // Ensure all elements from s.set are added in sorted order + for idx < len(s.set) && ezkube.ResourceIdsCompare(s.set[idx], resource) < 0 { + merged = append(merged, s.set[idx]) + idx++ + } + // If elements are equal, skip the element from s.set and use the element from the argument set + if idx < len(s.set) && ezkube.ResourceIdsCompare(s.set[idx], resource) == 0 { + idx++ // Increment to skip the element in s.set since it's equal and we use the one from set + } + // Add the current element from the second set + merged = append(merged, resource) + return true + }) + + // Append any remaining elements from s.set that were not added + if idx < len(s.set) { + merged = append(merged, s.set[idx:]...) + } + + return &resourceSet[T]{set: merged} } func (s *resourceSet[T]) Difference(set ResourceSet[T]) ResourceSet[T] { diff --git a/contrib/tests/set_v2_test.go b/contrib/tests/set_v2_test.go index 801519aef..f49d3ee1e 100644 --- a/contrib/tests/set_v2_test.go +++ b/contrib/tests/set_v2_test.go @@ -301,4 +301,123 @@ var _ = Describe("PaintSetV2", func() { Expect(paintList2[i]).To(Equal(paint)) } }) + Context("Union", func() { + var ( + setA, setB, setC sets_v2.ResourceSet[*v1.Paint] + paintA *v1.Paint + paintB *v1.Paint + ) + BeforeEach(func() { + setA = sets_v2.NewResourceSet[*v1.Paint]() + setB = sets_v2.NewResourceSet[*v1.Paint]() + setC = sets_v2.NewResourceSet[*v1.Paint]() + paintA = &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{Name: "nameA", Namespace: "nsA"}, + } + paintB = &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{Name: "nameB", Namespace: "nsB"}, + } + }) + It("should correctly perform union of a set with itself", func() { + setA.Insert(paintA) + unionSet := setA.Union(setA) + Expect(unionSet.Len()).To(Equal(1)) + Expect(unionSet.Has(paintA)).To(BeTrue()) + }) + + It("should correctly perform union of two distinct sets", func() { + setA.Insert(paintA) + setB.Insert(paintB) + unionSet := setA.Union(setB) + Expect(unionSet.Len()).To(Equal(2)) + Expect(unionSet.Has(paintA)).To(BeTrue()) + Expect(unionSet.Has(paintB)).To(BeTrue()) + }) + + It("should handle union with an empty set", func() { + setA.Insert(paintA) + unionSet := setA.Union(setC) // setC is empty + Expect(unionSet.Len()).To(Equal(1)) + Expect(unionSet.Has(paintA)).To(BeTrue()) + }) + + It("should return an empty set when unioning two empty sets", func() { + unionSet := setC.Union(setB) // both setC and setB are empty + Expect(unionSet.Len()).To(Equal(0)) + }) + + It("should maintain distinct elements when unioning sets with overlap", func() { + setA.Insert(paintA) + setB.Insert(paintA, paintB) + unionSet := setA.Union(setB) + Expect(unionSet.Len()).To(Equal(2)) + Expect(unionSet.Has(paintA)).To(BeTrue()) + Expect(unionSet.Has(paintB)).To(BeTrue()) + }) + + It("should be commutative (A union B = B union A)", func() { + setA.Insert(paintA) + setB.Insert(paintB) + unionSetAB := setA.Union(setB) + unionSetBA := setB.Union(setA) + Expect(unionSetAB.Len()).To(Equal(unionSetBA.Len())) + Expect(unionSetAB.Has(paintA) && unionSetAB.Has(paintB)).To(BeTrue()) + Expect(unionSetBA.Has(paintA) && unionSetBA.Has(paintB)).To(BeTrue()) + }) + + Context("Sorted Order Preservation after Union", func() { + var ( + setA, setB, unionSet sets_v2.ResourceSet[*v1.Paint] + paint1, paint2, paint3 *v1.Paint + ) + + BeforeEach(func() { + setA = sets_v2.NewResourceSet[*v1.Paint]() + setB = sets_v2.NewResourceSet[*v1.Paint]() + paint1 = &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{Name: "C", Namespace: "1"}, + } + paint2 = &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{Name: "A", Namespace: "3"}, + } + paint3 = &v1.Paint{ + ObjectMeta: metav1.ObjectMeta{Name: "B", Namespace: "2"}, + } + setA.Insert(paint1) + setB.Insert(paint2, paint3) + unionSet = setA.Union(setB) + }) + + It("should maintain sorted order in Iter after union", func() { + expectedOrder := []*v1.Paint{paint2, paint3, paint1} // Expected sorted by namespace + var actualOrder []*v1.Paint + unionSet.Iter(func(_ int, p *v1.Paint) bool { + actualOrder = append(actualOrder, p) + return true + }) + Expect(actualOrder).To(Equal(expectedOrder)) + }) + + It("should maintain sorted order in Filter after union", func() { + var filteredOrder []*v1.Paint + filterFunc := func(p *v1.Paint) bool { + return true // Select all + } + unionSet.Filter(filterFunc)(func(_ int, p *v1.Paint) bool { + filteredOrder = append(filteredOrder, p) + return true + }) + Expect(filteredOrder).To(Equal([]*v1.Paint{paint2, paint3, paint1})) // Should match expected sorted order + }) + + It("should maintain sorted order in FilterOutAndCreateList after union", func() { + filterOutFunc := func(p *v1.Paint) bool { + return false // Do not filter out any items + } + filteredList := unionSet.FilterOutAndCreateList(filterOutFunc) + Expect(filteredList).To(Equal([]*v1.Paint{paint2, paint3, paint1})) // Should match expected sorted order + }) + }) + + }) }) diff --git a/pkg/ezkube/resource_id.go b/pkg/ezkube/resource_id.go index 3b1d8207d..60bdfd963 100644 --- a/pkg/ezkube/resource_id.go +++ b/pkg/ezkube/resource_id.go @@ -174,9 +174,10 @@ func ResourceIdFromKeyWithSeparator(key string, separator string) (ResourceId, e } } -// ResourceIdsCompare returns an integer comparing two ResourceIds lexicographically. -// The comparison is based on name, then namespace, then cluster name. -// The result will be 0 if a == b, -1 if a < b, and +1 if a > b. +// ResourceIdsCompare compares two ResourceId instances, first by name, then by namespace, and finally by cluster name. +// If the names or namespaces differ, the comparison returns the result of strings.Compare on those values. +// If the names and namespaces are the same, it attempts to cast the ResourceId instances to ClusterResourceId +// and compares the cluster names. If the cast fails, it falls back to using the deprecated cluster name retrieval. func ResourceIdsCompare(a, b ResourceId) int { // compare names if cmp := strings.Compare(a.GetName(), b.GetName()); cmp != 0 {