Minimize allocations in strutil (#19)

strutil/go.mod

@@ -2,7 +2,4 @@ module github.com/hashicorp/go-secure-stdlib/strutil
 
 go 1.16
 
-require (
-	github.com/ryanuber/go-glob v1.0.0
-	github.com/stretchr/testify v1.7.0
-)
+require github.com/ryanuber/go-glob v1.0.0

strutil/go.sum

@@ -1,13 +1,2 @@
-github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
-github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
-github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk=
 github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc=
-github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
-github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
-gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
-gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

strutil/strutil.go

@@ -230,16 +230,16 @@ func TrimStrings(items []string) []string {
 // strings. This also may convert the items in the slice to lower case and
 // returns a sorted slice.
 func RemoveDuplicates(items []string, lowercase bool) []string {
-	itemsMap := map[string]bool{}
+	itemsMap := make(map[string]struct{}, len(items))
 	for _, item := range items {
 		item = strings.TrimSpace(item)
-		if lowercase {
-			item = strings.ToLower(item)
-		}
 		if item == "" {
 			continue
 		}
-		itemsMap[item] = true
+		if lowercase {
+			item = strings.ToLower(item)
+		}
+		itemsMap[item] = struct{}{}
 	}
 	items = make([]string, 0, len(itemsMap))
 	for item := range itemsMap {
@@ -254,18 +254,21 @@ func RemoveDuplicates(items []string, lowercase bool) []string {
 // In all cases, strings are compared after trimming whitespace
 // If caseInsensitive, strings will be compared after ToLower()
 func RemoveDuplicatesStable(items []string, caseInsensitive bool) []string {
-	itemsMap := make(map[string]bool, len(items))
+	itemsMap := make(map[string]struct{}, len(items))
 	deduplicated := make([]string, 0, len(items))
 
 	for _, item := range items {
 		key := strings.TrimSpace(item)
+		if _, ok := itemsMap[key]; ok || key == "" {
+			continue
+		}
 		if caseInsensitive {
 			key = strings.ToLower(key)
 		}
-		if key == "" || itemsMap[key] {
+		if _, ok := itemsMap[key]; ok {
 			continue
 		}
-		itemsMap[key] = true
+		itemsMap[key] = struct{}{}
 		deduplicated = append(deduplicated, item)
 	}
 	return deduplicated
@@ -299,17 +302,18 @@ func EquivalentSlices(a, b []string) bool {
 	}
 
 	// First we'll build maps to ensure unique values
-	mapA := map[string]bool{}
-	mapB := map[string]bool{}
+	mapA := make(map[string]struct{}, len(a))
+	mapB := make(map[string]struct{}, len(b))
 	for _, keyA := range a {
-		mapA[keyA] = true
+		mapA[keyA] = struct{}{}
 	}
 	for _, keyB := range b {
-		mapB[keyB] = true
+		mapB[keyB] = struct{}{}
 	}
 
 	// Now we'll build our checking slices
-	var sortedA, sortedB []string
+	sortedA := make([]string, 0, len(mapA))
+	sortedB := make([]string, 0, len(mapB))
 	for keyA := range mapA {
 		sortedA = append(sortedA, keyA)
 	}
@@ -434,23 +438,21 @@ func Difference(a, b []string, lowercase bool) []string {
 	a = RemoveDuplicates(a, lowercase)
 	b = RemoveDuplicates(b, lowercase)
 
-	itemsMap := map[string]bool{}
+	itemsMap := map[string]struct{}{}
 	for _, aVal := range a {
-		itemsMap[aVal] = true
+		itemsMap[aVal] = struct{}{}
 	}
 
 	// Perform difference calculation
 	for _, bVal := range b {
 		if _, ok := itemsMap[bVal]; ok {
-			itemsMap[bVal] = false
+			delete(itemsMap, bVal)
 		}
 	}
 
 	items := []string{}
-	for item, exists := range itemsMap {
-		if exists {
-			items = append(items, item)
-		}
+	for item := range itemsMap {
+		items = append(items, item)
 	}
 	sort.Strings(items)
 	return items

strutil/strutil_benchmark_test.go

@@ -0,0 +1,44 @@
+package strutil
+
+import (
+	"fmt"
+	"testing"
+)
+
+func BenchmarkRemoveDuplicates(b *testing.B) {
+	a := make([]string, 1_000_000)
+	for i := 0; i < len(a); i++ {
+		a[i] = fmt.Sprintf("test.%d", i)
+	}
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		RemoveDuplicates(a, true)
+	}
+}
+
+func BenchmarkRemoveDuplicatesStable(b *testing.B) {
+	a := make([]string, 1_000_000)
+	for i := 0; i < len(a); i++ {
+		a[i] = fmt.Sprintf("test.%d", i)
+	}
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		RemoveDuplicatesStable(a, true)
+	}
+}
+
+func BenchmarkEquivalentSlices(b *testing.B) {
+	x := make([]string, 1_000_000)
+	y := make([]string, len(x))
+	for i := 0; i < len(x); i++ {
+		x[i] = fmt.Sprintf("test.%d", i)
+		y[i] = fmt.Sprintf("test.%d", i)
+	}
+	b.ResetTimer()
+
+	for i := 0; i < b.N; i++ {
+		EquivalentSlices(x, y)
+	}
+}

strutil/strutil_test.go

@@ -6,11 +6,9 @@ import (
 	"fmt"
 	"reflect"
 	"testing"
-
-	"github.com/stretchr/testify/require"
 )
 
-func TestStrUtil_StrListDelete(t *testing.T) {
+func TestStrListDelete(t *testing.T) {
 	output := StrListDelete([]string{"item1", "item2", "item3"}, "item1")
 	if StrListContains(output, "item1") {
 		t.Fatal("bad: 'item1' should not have been present")
@@ -47,7 +45,7 @@ func TestStrUtil_StrListDelete(t *testing.T) {
 	}
 }
 
-func TestStrutil_EquivalentSlices(t *testing.T) {
+func TestEquivalentSlices(t *testing.T) {
 	slice1 := []string{"test2", "test1", "test3"}
 	slice2 := []string{"test3", "test2", "test1"}
 	if !EquivalentSlices(slice1, slice2) {
@@ -60,7 +58,7 @@ func TestStrutil_EquivalentSlices(t *testing.T) {
 	}
 }
 
-func TestStrutil_ListContainsGlob(t *testing.T) {
+func TestListContainsGlob(t *testing.T) {
 	haystack := []string{
 		"dev",
 		"ops*",
@@ -91,7 +89,7 @@ func TestStrutil_ListContainsGlob(t *testing.T) {
 	}
 }
 
-func TestStrutil_ListContains(t *testing.T) {
+func TestListContains(t *testing.T) {
 	haystack := []string{
 		"dev",
 		"ops",
@@ -106,7 +104,7 @@ func TestStrutil_ListContains(t *testing.T) {
 	}
 }
 
-func TestStrutil_ListSubset(t *testing.T) {
+func TestListSubset(t *testing.T) {
 	parent := []string{
 		"dev",
 		"ops",
@@ -137,7 +135,7 @@ func TestStrutil_ListSubset(t *testing.T) {
 	}
 }
 
-func TestStrutil_ParseKeyValues(t *testing.T) {
+func TestParseKeyValues(t *testing.T) {
 	actual := make(map[string]string)
 	expected := map[string]string{
 		"key1": "value1",
@@ -195,7 +193,7 @@ func TestStrutil_ParseKeyValues(t *testing.T) {
 	}
 }
 
-func TestStrutil_ParseArbitraryKeyValues(t *testing.T) {
+func TestParseArbitraryKeyValues(t *testing.T) {
 	actual := make(map[string]string)
 	expected := map[string]string{
 		"key1": "value1",
@@ -257,7 +255,7 @@ func TestStrutil_ParseArbitraryKeyValues(t *testing.T) {
 	}
 }
 
-func TestStrutil_ParseArbitraryStringSlice(t *testing.T) {
+func TestParseArbitraryStringSlice(t *testing.T) {
 	input := `CREATE ROLE "{{name}}" WITH LOGIN PASSWORD '{{password}}' VALID UNTIL '{{expiration}}';GRANT "foo-role" TO "{{name}}";ALTER ROLE "{{name}}" SET search_path = foo;GRANT CONNECT ON DATABASE "postgres" TO "{{name}}";`
 
 	jsonExpected := []string{
@@ -381,7 +379,7 @@ func TestRemoveEmpty(t *testing.T) {
 	}
 }
 
-func TestStrutil_AppendIfMissing(t *testing.T) {
+func TestAppendIfMissing(t *testing.T) {
 	keys := []string{}
 
 	keys = AppendIfMissing(keys, "foo")
@@ -418,7 +416,7 @@ func TestStrutil_AppendIfMissing(t *testing.T) {
 	}
 }
 
-func TestStrUtil_RemoveDuplicates(t *testing.T) {
+func TestRemoveDuplicates(t *testing.T) {
 	type tCase struct {
 		input     []string
 		expect    []string
@@ -442,7 +440,7 @@ func TestStrUtil_RemoveDuplicates(t *testing.T) {
 	}
 }
 
-func TestStrUtil_RemoveDuplicatesStable(t *testing.T) {
+func TestRemoveDuplicatesStable(t *testing.T) {
 	type tCase struct {
 		input           []string
 		expect          []string
@@ -469,7 +467,7 @@ func TestStrUtil_RemoveDuplicatesStable(t *testing.T) {
 	}
 }
 
-func TestStrUtil_ParseStringSlice(t *testing.T) {
+func TestParseStringSlice(t *testing.T) {
 	type tCase struct {
 		input  string
 		sep    string
@@ -495,7 +493,7 @@ func TestStrUtil_ParseStringSlice(t *testing.T) {
 	}
 }
 
-func TestStrUtil_MergeSlices(t *testing.T) {
+func TestMergeSlices(t *testing.T) {
 	res := MergeSlices([]string{"a", "c", "d"}, []string{}, []string{"c", "f", "a"}, nil, []string{"foo"})
 
 	expect := []string{"a", "c", "d", "f", "foo"}
@@ -561,7 +559,7 @@ func TestDifference(t *testing.T) {
 	}
 }
 
-func TestStrUtil_EqualStringMaps(t *testing.T) {
+func TestEqualStringMaps(t *testing.T) {
 	m1 := map[string]string{
 		"foo": "a",
 	}
@@ -671,31 +669,35 @@ func TestGetString(t *testing.T) {
 func TestPrintable(t *testing.T) {
 	cases := []struct {
 		input string
-		err   bool
+		exp   bool
 	}{
 		{
 			input: "/valid",
+			exp:   true,
 		},
 		{
 			input: "foobarvalid",
+			exp:   true,
 		},
 		{
 			input: "/invalid\u000A",
-			err:   true,
+			exp:   false,
 		},
 		{
 			input: "/invalid\u000D",
-			err:   true,
+			exp:   false,
 		},
 		{
 			input: "/invalid\u0000",
-			err:   true,
+			exp:   false,
 		},
 	}
 
 	for i, tc := range cases {
 		t.Run(fmt.Sprint(i), func(t *testing.T) {
-			require.True(t, Printable(tc.input) != tc.err)
+			if got, want := Printable(tc.input), tc.exp; got != want {
+				t.Errorf("expected %q printable to be %t, got %t", tc.input, want, got)
+			}
 		})
 	}
 }
@@ -717,7 +719,9 @@ func TestReverse(t *testing.T) {
 
 	for i, tc := range cases {
 		t.Run(fmt.Sprint(i), func(t *testing.T) {
-			require.Equal(t, Reverse(tc.in), tc.out)
+			if got, want := Reverse(tc.in), tc.out; got != want {
+				t.Errorf("expected %q to be %q", got, want)
+			}
 		})
 	}
 }