diff --git a/attribute/set.go b/attribute/set.go index 65f0cb96526..50334cc0050 100644 --- a/attribute/set.go +++ b/attribute/set.go @@ -263,7 +263,7 @@ func NewSetWithSortableFiltered(kvs []KeyValue, tmp *Sortable, filter Filter) (S *tmp = nil - uniqPart := filterSliceFrom(kvs, -1, func() func(kv KeyValue) bool { + uniq := filterBack(kvs, func() func(kv KeyValue) bool { firstCall := true var prev Key return func(kv KeyValue) bool { @@ -278,36 +278,57 @@ func NewSetWithSortableFiltered(kvs []KeyValue, tmp *Sortable, filter Filter) (S return true } }()) + kvs = kvs[uniq:] + if filter != nil { // Unique prior to filtering so the returned dropped attributes will // not also include the deduplicated attributes. - fltrPart := filterSliceFrom(kvs, uniqPart, filter) - return Set{equivalent: computeDistinct(kvs[fltrPart:])}, kvs[uniqPart:fltrPart] + k := filterFront(kvs, filter) + if k == len(kvs) { + return Set{equivalent: computeDistinct(kvs[:k])}, nil + } + return Set{equivalent: computeDistinct(kvs[:k])}, kvs[k:] } - return Set{equivalent: computeDistinct(kvs[uniqPart:])}, nil + + return Set{equivalent: computeDistinct(kvs)}, nil } -// filterSliceFrom filters the slice in-place from the start index to its end -// using f. A partition index is returned defining the index of slice from -// which all values less than it are dropped and all values greater than or -// equal to it are kept (according to the filtering by f). The returned value -// may be equal to the length of slice, in which case all values are meant to -// be kept. -// -// If start is less than or equal to zero, the whole slice will be filtered. -func filterSliceFrom(slice []KeyValue, start int, f Filter) int { - if start < 0 { - start = 0 +// filterFront filters the slice in-place using f. All kept KeyValues are moved +// to the front of slice (in order), and all dropped KeyValues are moved the +// end. The index for the first dropped KeyValue is returned. +func filterFront(slice []KeyValue, f Filter) int { + n := len(slice) + if n == 0 { + return 0 + } + + j := -1 + for i := 0; i < n; i++ { + if f(slice[i]) { + j++ + slice[i], slice[j] = slice[j], slice[i] + } + } + return j + 1 +} + +// filterBack filters the slice in-place using f. All kept KeyValues are moved +// to the end of slice (in order), and all dropped KeyValues are moved the +// front. The index for the first kept KeyValue is returned. +func filterBack(slice []KeyValue, f Filter) int { + n := len(slice) + if n == 0 { + return 0 } - part := len(slice) - for j := len(slice) - 1; j >= start; j-- { - if f(slice[j]) { - part-- - slice[j], slice[part] = slice[part], slice[j] + j := n + for i := n - 1; i >= 0; i-- { + if f(slice[i]) { + j-- + slice[i], slice[j] = slice[j], slice[i] } } - return part + return j } // Filter returns a filtered copy of this Set. See the documentation for @@ -319,42 +340,33 @@ func (l *Set) Filter(re Filter) (Set, []KeyValue) { }, nil } - iter := l.Iter() - var filtered bool // Iterate to the first attribute that will be filtered out. - for iter.Next() { - if !re(iter.Attribute()) { - filtered = true + start, n := 0, l.Len() + for ; start < n; start++ { + kv, _ := l.Get(start) + if !re(kv) { break } } // No attributes will be dropped, return the immutable Set l and nil. - if !filtered { + if start >= n { return *l, nil } - // All values up to start are going to be kept based on iter loop above. - start, _ := iter.IndexedAttribute() - // Copy all attributes to a new slice now that we know we need to return a // modified set. // // Do not do this in-place on the underlying storage of *Set l. Sets are // immutable and filtering should not change this. - // - // Do this after the call to IndexedAttribute as it resets the iter index. - slice := iter.ToSlice() - - // Use start+1 given we have already run re on the start index and know it - // is filtered out. - part := filterSliceFrom(slice, start+1, re) + slice := l.ToSlice() - // Move slice[start] after others to keep order. - part-- - slice[start], slice[part] = slice[part], slice[start] + // Do not re-evaluate re(slice[i]). + end := len(slice) - 1 + slice[start], slice[end] = slice[end], slice[start] - return Set{equivalent: computeDistinct(slice[part:])}, slice[:part] + div := filterFront(slice[start:end], re) + start + return Set{equivalent: computeDistinct(slice[:div])}, slice[div:] } // computeDistinct returns a Distinct using either the fixed- or diff --git a/attribute/set_internal_test.go b/attribute/set_internal_test.go index c298b682620..97b727411af 100644 --- a/attribute/set_internal_test.go +++ b/attribute/set_internal_test.go @@ -18,127 +18,81 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestFilterSliceFrom(t *testing.T) { - type input struct { - slice []KeyValue - start int - f Filter - } - - type expect struct { - slice []KeyValue - part int - } - - var ( - a0 = Int("A", 0) - a1 = Int("A", 1) - a2 = Int("A", 2) - - b0 = Int("B", 0) - b1 = Int("B", 1) - b2 = Int("B", 2) - ) +func TestFilters(t *testing.T) { + a := String("A", "a") + b := String("B", "b") + c := String("C", "c") tests := []struct { name string - in input - want expect + in []KeyValue + fltr Filter + kept []KeyValue + drop []KeyValue }{ { - name: "FilterA", - in: input{ - f: func(kv KeyValue) bool { return kv.Key == "A" }, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - start: -1, - }, - want: expect{ - part: 3, - slice: []KeyValue{b0, b1, b2, a0, a1, a2}, - }, - }, - { - name: "FilterB", - in: input{ - f: func(kv KeyValue) bool { return kv.Key == "B" }, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - start: -1, - }, - want: expect{ - part: 3, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - }, + name: "A", + in: []KeyValue{a, b, c}, + fltr: func(kv KeyValue) bool { return kv.Key == "A" }, + kept: []KeyValue{a}, + drop: []KeyValue{b, c}, }, { - name: "NoMatch", - in: input{ - f: func(kv KeyValue) bool { return kv.Key == "C" }, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - start: -1, - }, - want: expect{ - part: 6, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - }, + name: "B", + in: []KeyValue{a, b, c}, + fltr: func(kv KeyValue) bool { return kv.Key == "B" }, + kept: []KeyValue{b}, + drop: []KeyValue{a, c}, }, { - name: "AllMatch", - in: input{ - f: func(kv KeyValue) bool { return true }, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - start: -1, - }, - want: expect{ - part: 0, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - }, + name: "C", + in: []KeyValue{a, b, c}, + fltr: func(kv KeyValue) bool { return kv.Key == "C" }, + kept: []KeyValue{c}, + drop: []KeyValue{a, b}, }, { - name: "StartHalfWay", - in: input{ - f: func(kv KeyValue) bool { return true }, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - start: 3, - }, - want: expect{ - part: 3, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - }, + name: "All", + in: []KeyValue{a, b, c}, + fltr: func(kv KeyValue) bool { return true }, + kept: []KeyValue{a, b, c}, + drop: []KeyValue{}, }, { - name: "StartBeyondEnd", - in: input{ - f: func(kv KeyValue) bool { return true }, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - start: 7, - }, - want: expect{ - part: 6, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - }, - }, - { - name: "StartGuard", - in: input{ - f: func(kv KeyValue) bool { return kv.Value.AsInt64() == 1 }, - slice: []KeyValue{a0, a1, a2, b0, b1, b2}, - start: 2, - }, - want: expect{ - part: 5, - // a1 is not moved. - slice: []KeyValue{a0, a1, a2, b0, b2, b1}, - }, + name: "None", + in: []KeyValue{a, b, c}, + fltr: func(kv KeyValue) bool { return false }, + kept: []KeyValue{}, + drop: []KeyValue{a, b, c}, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - part := filterSliceFrom(test.in.slice, test.in.start, test.in.f) - assert.Equal(t, test.want.part, part, "returned partition") - assert.Equal(t, test.want.slice, test.in.slice) + t.Run("filterFront", func(t *testing.T) { + in := make([]KeyValue, len(test.in)) + copy(in, test.in) + + k := filterFront(in, test.fltr) + require.Equal(t, len(test.kept), k, "partition index") + + assert.Equal(t, test.kept, in[:k], "kept") + assert.ElementsMatch(t, test.drop, in[k:], "dropped") + }) + + t.Run("filterBack", func(t *testing.T) { + in := make([]KeyValue, len(test.in)) + copy(in, test.in) + + k := filterBack(in, test.fltr) + require.Equal(t, len(test.drop), k, "partition index") + + assert.ElementsMatch(t, test.drop, in[:k], "dropped") + assert.Equal(t, test.kept, in[k:], "kept") + }) }) } } diff --git a/attribute/set_test.go b/attribute/set_test.go index 90bd688c959..66d80b7781c 100644 --- a/attribute/set_test.go +++ b/attribute/set_test.go @@ -130,6 +130,75 @@ func TestSetDedup(t *testing.T) { } } +func TestFiltering(t *testing.T) { + var ( + a = attribute.String("A", "a") + b = attribute.String("B", "b") + c = attribute.String("C", "c") + ) + + tests := []struct { + name string + in []attribute.KeyValue + filter attribute.Filter + filtered []attribute.KeyValue + dropped []attribute.KeyValue + }{ + { + name: "A", + in: []attribute.KeyValue{a, b, c}, + filter: func(kv attribute.KeyValue) bool { return kv.Key == "A" }, + filtered: []attribute.KeyValue{a}, + dropped: []attribute.KeyValue{b, c}, + }, + { + name: "B", + in: []attribute.KeyValue{a, b, c}, + filter: func(kv attribute.KeyValue) bool { return kv.Key == "B" }, + filtered: []attribute.KeyValue{b}, + dropped: []attribute.KeyValue{a, c}, + }, + { + name: "C", + in: []attribute.KeyValue{a, b, c}, + filter: func(kv attribute.KeyValue) bool { return kv.Key == "C" }, + filtered: []attribute.KeyValue{c}, + dropped: []attribute.KeyValue{a, b}, + }, + { + name: "None", + in: []attribute.KeyValue{a, b, c}, + filter: func(kv attribute.KeyValue) bool { return false }, + filtered: nil, + dropped: []attribute.KeyValue{a, b, c}, + }, + { + name: "All", + in: []attribute.KeyValue{a, b, c}, + filter: func(kv attribute.KeyValue) bool { return true }, + filtered: []attribute.KeyValue{a, b, c}, + dropped: nil, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Run("NewSetWithFiltered", func(t *testing.T) { + fltr, drop := attribute.NewSetWithFiltered(test.in, test.filter) + assert.Equal(t, test.filtered, fltr.ToSlice(), "filtered") + assert.ElementsMatch(t, test.dropped, drop, "dropped") + }) + + t.Run("Set.Filter", func(t *testing.T) { + s := attribute.NewSet(test.in...) + fltr, drop := s.Filter(test.filter) + assert.Equal(t, test.filtered, fltr.ToSlice(), "filtered") + assert.ElementsMatch(t, test.dropped, drop, "dropped") + }) + }) + } +} + func TestUniqueness(t *testing.T) { short := []attribute.KeyValue{ attribute.String("A", "0"), @@ -167,7 +236,7 @@ func TestUniqueness(t *testing.T) { full := attribute.NewSet(uniq...) - require.Equal(t, tc.encoding, distinct.Encoded(enc)) + require.Equal(t, tc.encoding, distinct.Encoded(enc), tc.keyRe) require.Equal(t, tc.fullEnc, full.Encoded(enc)) } }