From 7d5fe13d27126b96d81b0c654bd552a19db8a312 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 30 Jan 2025 08:46:21 +0000 Subject: [PATCH] types/views: make SliceEqualAnyOrder also do short slice optimization SliceEqualAnyOrderFunc had an optimization missing from SliceEqualAnyOrder. Now they share the same code and both have the optimization. Updates #14593 Change-Id: I550726e0964fc4006e77bb44addc67be989c131c Signed-off-by: Brad Fitzpatrick --- types/views/views.go | 150 ++++++++++++++++++++++---------------- types/views/views_test.go | 77 +++++++++++++++++++ 2 files changed, 165 insertions(+), 62 deletions(-) diff --git a/types/views/views.go b/types/views/views.go index ae776c3b2..3911f1112 100644 --- a/types/views/views.go +++ b/types/views/views.go @@ -330,6 +330,12 @@ func SliceEqual[T comparable](a, b Slice[T]) bool { return slices.Equal(a.ж, b.ж) } +// shortOOOLen (short Out-of-Order length) is the slice length at or +// under which we attempt to compare two slices quadratically rather +// than allocating memory for a map in SliceEqualAnyOrder and +// SliceEqualAnyOrderFunc. +const shortOOOLen = 5 + // SliceEqualAnyOrder reports whether a and b contain the same elements, regardless of order. // The underlying slices for a and b can be nil. func SliceEqualAnyOrder[T comparable](a, b Slice[T]) bool { @@ -347,18 +353,15 @@ func SliceEqualAnyOrder[T comparable](a, b Slice[T]) bool { return true } - // count the occurrences of remaining values and compare - valueCount := make(map[T]int) - for i, n := diffStart, a.Len(); i < n; i++ { - valueCount[a.At(i)]++ - valueCount[b.At(i)]-- + a, b = a.SliceFrom(diffStart), b.SliceFrom(diffStart) + cmp := func(v T) T { return v } + + // For a small number of items, avoid the allocation of a map and just + // do the quadratic thing. + if a.Len() <= shortOOOLen { + return unorderedSliceEqualAnyOrderSmall(a, b, cmp) } - for _, count := range valueCount { - if count != 0 { - return false - } - } - return true + return unorderedSliceEqualAnyOrder(a, b, cmp) } // SliceEqualAnyOrderFunc reports whether a and b contain the same elements, @@ -382,62 +385,31 @@ func SliceEqualAnyOrderFunc[T any, V comparable](a, b Slice[T], cmp func(T) V) b return true } + a, b = a.SliceFrom(diffStart), b.SliceFrom(diffStart) // For a small number of items, avoid the allocation of a map and just - // do the quadratic thing. We can also only check the items between - // diffStart and the end. - nRemain := a.Len() - diffStart - const shortOptLen = 5 - if nRemain <= shortOptLen { - // These track which elements in a and b have been matched, so - // that we don't treat arrays with differing number of - // duplicate elements as equal (e.g. [1, 1, 2] and [1, 2, 2]). - var aMatched, bMatched [shortOptLen]bool - - // Compare each element in a to each element in b - for i := range nRemain { - av := cmp(a.At(i + diffStart)) - found := false - for j := range nRemain { - // Skip elements in b that have already been - // used to match an item in a. - if bMatched[j] { - continue - } - - bv := cmp(b.At(j + diffStart)) - if av == bv { - // Mark these elements as already - // matched, so that a future loop - // iteration (of a duplicate element) - // doesn't match it again. - aMatched[i] = true - bMatched[j] = true - found = true - break - } - } - if !found { - return false - } - } - - // Verify all elements were matched exactly once. - for i := range nRemain { - if !aMatched[i] || !bMatched[i] { - return false - } - } + // do the quadratic thing. + if a.Len() <= shortOOOLen { + return unorderedSliceEqualAnyOrderSmall(a, b, cmp) + } + return unorderedSliceEqualAnyOrder(a, b, cmp) +} +// unorderedSliceEqualAnyOrder reports whether a and b contain the same elements +// using a map. The cmp function maps from a T slice element to a comparable +// value. +func unorderedSliceEqualAnyOrder[T any, V comparable](a, b Slice[T], cmp func(T) V) bool { + if a.Len() != b.Len() { + panic("internal error") + } + if a.Len() == 0 { return true } - - // count the occurrences of remaining values and compare - valueCount := make(map[V]int) - for i, n := diffStart, a.Len(); i < n; i++ { - valueCount[cmp(a.At(i))]++ - valueCount[cmp(b.At(i))]-- + m := make(map[V]int) + for i := range a.Len() { + m[cmp(a.At(i))]++ + m[cmp(b.At(i))]-- } - for _, count := range valueCount { + for _, count := range m { if count != 0 { return false } @@ -445,6 +417,60 @@ func SliceEqualAnyOrderFunc[T any, V comparable](a, b Slice[T], cmp func(T) V) b return true } +// unorderedSliceEqualAnyOrderSmall reports whether a and b (which must be the +// same length, and shortOOOLen or shorter) contain the same elements (using cmp +// to map from T to a comparable value) in some order. +// +// This is the quadratic-time implementation for small slices that doesn't +// allocate. +func unorderedSliceEqualAnyOrderSmall[T any, V comparable](a, b Slice[T], cmp func(T) V) bool { + if a.Len() != b.Len() || a.Len() > shortOOOLen { + panic("internal error") + } + + // These track which elements in a and b have been matched, so + // that we don't treat arrays with differing number of + // duplicate elements as equal (e.g. [1, 1, 2] and [1, 2, 2]). + var aMatched, bMatched [shortOOOLen]bool + + // Compare each element in a to each element in b + for i := range a.Len() { + av := cmp(a.At(i)) + found := false + for j := range a.Len() { + // Skip elements in b that have already been + // used to match an item in a. + if bMatched[j] { + continue + } + + bv := cmp(b.At(j)) + if av == bv { + // Mark these elements as already + // matched, so that a future loop + // iteration (of a duplicate element) + // doesn't match it again. + aMatched[i] = true + bMatched[j] = true + found = true + break + } + } + if !found { + return false + } + } + + // Verify all elements were matched exactly once. + for i := range a.Len() { + if !aMatched[i] || !bMatched[i] { + return false + } + } + + return true +} + // MapSlice is a view over a map whose values are slices. type MapSlice[K comparable, V any] struct { // ж is the underlying mutable value, named with a hard-to-type diff --git a/types/views/views_test.go b/types/views/views_test.go index 7837a89d6..2205cbc03 100644 --- a/types/views/views_test.go +++ b/types/views/views_test.go @@ -231,6 +231,83 @@ func TestSliceEqualAnyOrderFunc(t *testing.T) { } } +func TestSliceEqualAnyOrderAllocs(t *testing.T) { + ss := func(s ...string) Slice[string] { return SliceOf(s) } + cmp := func(s string) string { return s } + + t.Run("no-allocs-short-unordered", func(t *testing.T) { + // No allocations for short comparisons + short1 := ss("a", "b", "c") + short2 := ss("c", "b", "a") + if n := testing.AllocsPerRun(1000, func() { + if !SliceEqualAnyOrder(short1, short2) { + t.Fatal("not equal") + } + if !SliceEqualAnyOrderFunc(short1, short2, cmp) { + t.Fatal("not equal") + } + }); n > 0 { + t.Fatalf("allocs = %v; want 0", n) + } + }) + + t.Run("no-allocs-long-match", func(t *testing.T) { + long1 := ss("a", "b", "c", "d", "e", "f", "g", "h", "i", "j") + long2 := ss("a", "b", "c", "d", "e", "f", "g", "h", "i", "j") + + if n := testing.AllocsPerRun(1000, func() { + if !SliceEqualAnyOrder(long1, long2) { + t.Fatal("not equal") + } + if !SliceEqualAnyOrderFunc(long1, long2, cmp) { + t.Fatal("not equal") + } + }); n > 0 { + t.Fatalf("allocs = %v; want 0", n) + } + }) + + t.Run("allocs-long-unordered", func(t *testing.T) { + // We do unfortunately allocate for long comparisons. + long1 := ss("a", "b", "c", "d", "e", "f", "g", "h", "i", "j") + long2 := ss("c", "b", "a", "e", "d", "f", "g", "h", "i", "j") + + if n := testing.AllocsPerRun(1000, func() { + if !SliceEqualAnyOrder(long1, long2) { + t.Fatal("not equal") + } + if !SliceEqualAnyOrderFunc(long1, long2, cmp) { + t.Fatal("not equal") + } + }); n == 0 { + t.Fatalf("unexpectedly didn't allocate") + } + }) +} + +func BenchmarkSliceEqualAnyOrder(b *testing.B) { + b.Run("short", func(b *testing.B) { + b.ReportAllocs() + s1 := SliceOf([]string{"foo", "bar"}) + s2 := SliceOf([]string{"bar", "foo"}) + for range b.N { + if !SliceEqualAnyOrder(s1, s2) { + b.Fatal() + } + } + }) + b.Run("long", func(b *testing.B) { + b.ReportAllocs() + s1 := SliceOf([]string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"}) + s2 := SliceOf([]string{"c", "b", "a", "e", "d", "f", "g", "h", "i", "j"}) + for range b.N { + if !SliceEqualAnyOrder(s1, s2) { + b.Fatal() + } + } + }) +} + func TestSliceEqual(t *testing.T) { a := SliceOf([]string{"foo", "bar"}) b := SliceOf([]string{"foo", "bar"})