Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

internal: remove unused TrimDuplicates #326

Merged
merged 1 commit into from May 17, 2022
Merged

internal: remove unused TrimDuplicates #326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 0 additions & 49 deletions internal/serialize/keyvalues.go
View file
Open in desktop
Expand Up @@ -44,55 +44,6 @@ func WithValues(oldKV, newKV []interface{}) []interface{} {
return kv
}

// TrimDuplicates deduplicates elements provided in multiple key/value tuple
// slices, whilst maintaining the distinction between where the items are
// contained.
func TrimDuplicates(kvLists ...[]interface{}) [][]interface{} {
// maintain a map of all seen keys
seenKeys := map[interface{}]struct{}{}
// build the same number of output slices as inputs
outs := make([][]interface{}, len(kvLists))
// iterate over the input slices backwards, as 'later' kv specifications
// of the same key will take precedence over earlier ones
for i := len(kvLists) - 1; i >= 0; i-- {
// initialise this output slice
outs[i] = []interface{}{}
// obtain a reference to the kvList we are processing
// and make sure it has an even number of entries
kvList := kvLists[i]
if len(kvList)%2 != 0 {
kvList = append(kvList, missingValue)
}

// start iterating at len(kvList) - 2 (i.e. the 2nd last item) for
// slices that have an even number of elements.
// We add (len(kvList) % 2) here to handle the case where there is an
// odd number of elements in a kvList.
// If there is an odd number, then the last element in the slice will
// have the value 'null'.
for i2 := len(kvList) - 2 + (len(kvList) % 2); i2 >= 0; i2 -= 2 {
k := kvList[i2]
// if we have already seen this key, do not include it again
if _, ok := seenKeys[k]; ok {
continue
}
// make a note that we've observed a new key
seenKeys[k] = struct{}{}
// attempt to obtain the value of the key
var v interface{}
// i2+1 should only ever be out of bounds if we handling the first
// iteration over a slice with an odd number of elements
if i2+1 < len(kvList) {
v = kvList[i2+1]
}
// add this KV tuple to the *start* of the output list to maintain
// the original order as we are iterating over the slice backwards
outs[i] = append([]interface{}{k, v}, outs[i]...)
}
}
return outs
}

// MergeKVs deduplicates elements provided in two key/value slices.
//
// Keys in each slice are expected to be unique, so duplicates can only occur
Expand Down
106 changes: 33 additions & 73 deletions internal/serialize/keyvalues_test.go
View file
Open in desktop
Expand Up @@ -153,106 +153,66 @@ No whitespace.`,

func TestDuplicates(t *testing.T) {
for name, test := range map[string]struct {
first, second []interface{}
expectedTrimmed [][]interface{}
expectedMerged []interface{}
first, second []interface{}
expected []interface{}
}{
"empty": {
expectedTrimmed: [][]interface{}{{}, {}},
},
"empty": {},
"no-duplicates": {
first: makeKV("a", 3),
second: makeKV("b", 3),
expectedTrimmed: [][]interface{}{
makeKV("a", 3),
makeKV("b", 3),
},
expectedMerged: append(makeKV("a", 3), makeKV("b", 3)...),
first: makeKV("a", 3),
second: makeKV("b", 3),
expected: append(makeKV("a", 3), makeKV("b", 3)...),
},
"all-duplicates": {
first: makeKV("a", 3),
second: makeKV("a", 3),
expectedTrimmed: [][]interface{}{
{},
makeKV("a", 3),
},
expectedMerged: makeKV("a", 3),
first: makeKV("a", 3),
second: makeKV("a", 3),
expected: makeKV("a", 3),
},
"start-duplicate": {
first: append([]interface{}{"x", 1}, makeKV("a", 3)...),
second: append([]interface{}{"x", 2}, makeKV("b", 3)...),
expectedTrimmed: [][]interface{}{
makeKV("a", 3),
append([]interface{}{"x", 2}, makeKV("b", 3)...),
},
expectedMerged: append(append(makeKV("a", 3), "x", 2), makeKV("b", 3)...),
first: append([]interface{}{"x", 1}, makeKV("a", 3)...),
second: append([]interface{}{"x", 2}, makeKV("b", 3)...),
expected: append(append(makeKV("a", 3), "x", 2), makeKV("b", 3)...),
},
"subset-first": {
first: append([]interface{}{"x", 1}, makeKV("a", 3)...),
second: append([]interface{}{"x", 2}, makeKV("a", 3)...),
expectedTrimmed: [][]interface{}{
{},
append([]interface{}{"x", 2}, makeKV("a", 3)...),
},
expectedMerged: append([]interface{}{"x", 2}, makeKV("a", 3)...),
first: append([]interface{}{"x", 1}, makeKV("a", 3)...),
second: append([]interface{}{"x", 2}, makeKV("a", 3)...),
expected: append([]interface{}{"x", 2}, makeKV("a", 3)...),
},
"subset-second": {
first: append([]interface{}{"x", 1}, makeKV("a", 1)...),
second: append([]interface{}{"x", 2}, makeKV("b", 2)...),
expectedTrimmed: [][]interface{}{
makeKV("a", 1),
append([]interface{}{"x", 2}, makeKV("b", 2)...),
},
expectedMerged: append(append(makeKV("a", 1), "x", 2), makeKV("b", 2)...),
first: append([]interface{}{"x", 1}, makeKV("a", 1)...),
second: append([]interface{}{"x", 2}, makeKV("b", 2)...),
expected: append(append(makeKV("a", 1), "x", 2), makeKV("b", 2)...),
},
"end-duplicate": {
first: append(makeKV("a", 3), "x", 1),
second: append(makeKV("b", 3), "x", 2),
expectedTrimmed: [][]interface{}{
makeKV("a", 3),
append(makeKV("b", 3), "x", 2),
},
expectedMerged: append(makeKV("a", 3), append(makeKV("b", 3), "x", 2)...),
first: append(makeKV("a", 3), "x", 1),
second: append(makeKV("b", 3), "x", 2),
expected: append(makeKV("a", 3), append(makeKV("b", 3), "x", 2)...),
},
"middle-duplicate": {
first: []interface{}{"a-0", 0, "x", 1, "a-1", 2},
second: []interface{}{"b-0", 0, "x", 2, "b-1", 2},
expectedTrimmed: [][]interface{}{
{"a-0", 0, "a-1", 2},
{"b-0", 0, "x", 2, "b-1", 2},
},
expectedMerged: []interface{}{"a-0", 0, "a-1", 2, "b-0", 0, "x", 2, "b-1", 2},
first: []interface{}{"a-0", 0, "x", 1, "a-1", 2},
second: []interface{}{"b-0", 0, "x", 2, "b-1", 2},
expected: []interface{}{"a-0", 0, "a-1", 2, "b-0", 0, "x", 2, "b-1", 2},
},
"internal-duplicates": {
first: []interface{}{"a", 0, "x", 1, "a", 2},
second: []interface{}{"b", 0, "x", 2, "b", 2},
expectedTrimmed: [][]interface{}{
{"a", 2},
{"x", 2, "b", 2},
},
// This is the case where Merged keeps key/value pairs
// that were already duplicated inside the slices, for
// performance.
expectedMerged: []interface{}{"a", 0, "a", 2, "b", 0, "x", 2, "b", 2},
expected: []interface{}{"a", 0, "a", 2, "b", 0, "x", 2, "b", 2},
},
} {
t.Run(name, func(t *testing.T) {
t.Run("TrimDuplicates", func(t *testing.T) {
actual := serialize.TrimDuplicates(test.first, test.second)
expectEqual(t, "trimmed key/value pairs", test.expectedTrimmed, actual)
})

t.Run("Merged", func(t *testing.T) {
actual := serialize.MergeKVs(test.first, test.second)
expectEqual(t, "merged key/value pairs", test.expectedMerged, actual)
expectEqual(t, "merged key/value pairs", test.expected, actual)
})
})
}
}

// BenchmarkTrimDuplicates checks performance when TrimDuplicates is called with two slices.
// BenchmarkMergeKVs checks performance when MergeKVs is called with two slices.
// In practice that is how the function is used.
func BenchmarkTrimDuplicates(b *testing.B) {
func BenchmarkMergeKVs(b *testing.B) {
for firstLength := 0; firstLength < 10; firstLength++ {
firstA := makeKV("a", firstLength)
for secondLength := 0; secondLength < 10; secondLength++ {
Expand All @@ -262,7 +222,7 @@ func BenchmarkTrimDuplicates(b *testing.B) {
// This is the most common case: all key/value pairs are kept.
b.Run("no-duplicates", func(b *testing.B) {
expected := append(firstA, secondB...)
benchTrimDuplicates(b, expected, firstA, secondB)
benchMergeKVs(b, expected, firstA, secondB)
})

// Fairly unlikely...
Expand All @@ -272,7 +232,7 @@ func BenchmarkTrimDuplicates(b *testing.B) {
expected = firstA[secondLength*2:]
}
expected = append(expected, secondA...)
benchTrimDuplicates(b, expected, firstA, secondA)
benchMergeKVs(b, expected, firstA, secondA)
})

// First entry is the same.
Expand All @@ -282,7 +242,7 @@ func BenchmarkTrimDuplicates(b *testing.B) {
second := []interface{}{"x", 1}
second = append(second, secondB...)
expected := append(firstA, second...)
benchTrimDuplicates(b, expected, first, second)
benchMergeKVs(b, expected, first, second)
})

// Last entry is the same.
Expand All @@ -292,7 +252,7 @@ func BenchmarkTrimDuplicates(b *testing.B) {
second := secondB[:]
second = append(second, "x", 1)
expected := append(firstA, second...)
benchTrimDuplicates(b, expected, first, second)
benchMergeKVs(b, expected, first, second)
})
})
}
Expand All @@ -310,7 +270,7 @@ func makeKV(prefix string, length int) []interface{} {
return kv
}

func benchTrimDuplicates(b *testing.B, expected []interface{}, first, second []interface{}) {
func benchMergeKVs(b *testing.B, expected []interface{}, first, second []interface{}) {
if len(expected) == 0 {
expected = nil
}
Expand Down