feat: reduce memory allocation for common slice and map types (#30)

dyolcekaj · web-flow · commit ab9387cbe802 · 2024-10-21T11:12:59.000-07:00
This commit reduces memory allocations for some collection types by avoiding the slow and expensive JSON marshal->unmarshal path. It extends the list of type assertions to slices and maps of some primitive types. **Additional context** While memory profiling a production application we (@dyolcekaj) noticed that 50+% of our memory allocations were happening during `ldvalue.CopyArbitraryValue`. We use feature flags heavily, and for any HTTP request it is typical to evaluate 2-8 feature flags. We found that the reason for this high memory usage was due to a string slice with several hundred items being passed through `ldvalue.CopyArbitraryValue` on most `ldcontext.Builder` instantiations. This was occurring in a general use function like ``` func IsEnabled(ctx context.Context, flag string, attributes map[string]any) bool { client := ctx.Value("ld-client).(*ldclient.LDClient) builder := ldcontextBuilderFromCtx(ctx) // get some HTTP request type info, user info, etc. // note ldvalue.CopyArbitraryValueMap has the same issue for key, value := range attributes { builder.SetValue(key, ldvalue.CopyArbitraryValue(value)) } // ignore errors for ex. enabled, _ := client.BoolVariation(flag, builder.Build(), false) return enabled } ``` If any of the `map[string]any` values are themselves a `[]string` or similar we hit the `FromJSONMarshal` slow path. To partially solve the problem we have written a wrapper around this SDK with more or less the same code as I am submitting here. The reduction in memory allocation is especially noticeable when copying `[]string`, but in all cases there is a fairly significant improvement in latency and total memory usage. Although this is verbose, alternatives that use generic functions like ``` func copyArbitraryType[T comparable](data []T) Value { ... } ``` don't save on allocations, and using reflection to determine the slice or map element types does have some benefit especially for slices but still has a high number of allocations for large maps. You can see an implementation of this change using reflection [here](https://github.com/dyolcekaj/launchdarkly-go-sdk-common/tree/arbitrary-collection-copies) Here are benchmarks from my machine showing the improvement for these specific use cases. Command for all is `go test -benchmem '-run=^$$' -bench="CollectionCopy*" ./ldvalue` Before: ``` BenchmarkCollectionCopyMapStringSmall-16 197137 5597 ns/op 4730 B/op 47 allocs/op BenchmarkCollectionCopyMapStringLarge-16 1776 632766 ns/op 676664 B/op 4043 allocs/op BenchmarkCollectionCopySliceStringSmall-16 415045 2731 ns/op 3931 B/op 19 allocs/op BenchmarkCollectionCopySliceStringMedium-16 57158 20780 ns/op 35357 B/op 112 allocs/op BenchmarkCollectionCopySliceStringLarge-16 6002 186840 ns/op 267965 B/op 1016 allocs/op BenchmarkCollectionCopySliceIntSmall-16 428552 2633 ns/op 3835 B/op 9 allocs/op BenchmarkCollectionCopySliceIntMedium-16 58148 20332 ns/op 34397 B/op 12 allocs/op BenchmarkCollectionCopySliceIntLarge-16 6182 180290 ns/op 260010 B/op 15 allocs/op ``` After: ``` BenchmarkCollectionCopyMapStringSmall-16 1568584 769.4 ns/op 2117 B/op 2 allocs/op BenchmarkCollectionCopyMapStringLarge-16 15642 76328 ns/op 254033 B/op 3 allocs/op BenchmarkCollectionCopySliceStringSmall-16 5143736 226.0 ns/op 1048 B/op 2 allocs/op BenchmarkCollectionCopySliceStringMedium-16 825129 1439 ns/op 9752 B/op 2 allocs/op BenchmarkCollectionCopySliceStringLarge-16 78724 15250 ns/op 98328 B/op 2 allocs/op BenchmarkCollectionCopySliceIntSmall-16 5327779 224.1 ns/op 1048 B/op 2 allocs/op BenchmarkCollectionCopySliceIntMedium-16 832033 1419 ns/op 9752 B/op 2 allocs/op BenchmarkCollectionCopySliceIntLarge-16 86389 13898 ns/op 98328 B/op 2 allocs/op ``` Reflection based benchmark ``` BenchmarkReflectCopyMapStringSmall-16 516852 2229 ns/op 2678 B/op 23 allocs/op BenchmarkReflectCopyMapStringLarge-16 5341 197526 ns/op 310622 B/op 2004 allocs/op BenchmarkReflectCopySliceStringSmall-16 2635124 447.3 ns/op 1048 B/op 2 allocs/op BenchmarkReflectCopySliceStringMedium-16 346424 3233 ns/op 9752 B/op 2 allocs/op BenchmarkReflectCopySliceStringLarge-16 38649 31042 ns/op 98329 B/op 2 allocs/op BenchmarkReflectCopySliceIntSmall-16 2634298 448.7 ns/op 1048 B/op 2 allocs/op BenchmarkReflectCopySliceIntMedium-16 340862 3293 ns/op 9752 B/op 2 allocs/op BenchmarkReflectCopySliceIntLarge-16 39921 30468 ns/op 98328 B/op 2 allocs/op ``` Benchstat ``` goos: darwin goarch: amd64 pkg: github.com/launchdarkly/go-sdk-common/v3/ldvalue cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz │ original.txt │ updated.txt │ │ sec/op │ sec/op vs base │ CollectionCopyMapStringSmall-16 5.534µ ± ∞ ¹ 2.103µ ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopyMapStringLarge-16 680.4µ ± ∞ ¹ 193.5µ ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceStringSmall-16 2818.0n ± ∞ ¹ 431.1n ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceStringMedium-16 20.874µ ± ∞ ¹ 3.299µ ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceStringLarge-16 188.00µ ± ∞ ¹ 30.97µ ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceIntSmall-16 2672.0n ± ∞ ¹ 441.5n ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceIntMedium-16 20.542µ ± ∞ ¹ 3.196µ ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceIntLarge-16 181.99µ ± ∞ ¹ 30.73µ ± ∞ ¹ ~ (p=1.000 n=1) ² geomean 28.34µ 5.449µ -80.77% ¹ need >= 6 samples for confidence interval at level 0.95 ² need >= 4 samples to detect a difference at alpha level 0.05 │ original.txt │ updated.txt │ │ B/op │ B/op vs base │ CollectionCopyMapStringSmall-16 4.618Ki ± ∞ ¹ 2.615Ki ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopyMapStringLarge-16 661.2Ki ± ∞ ¹ 303.3Ki ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceStringSmall-16 3.839Ki ± ∞ ¹ 1.023Ki ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceStringMedium-16 34.526Ki ± ∞ ¹ 9.523Ki ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceStringLarge-16 262.07Ki ± ∞ ¹ 96.02Ki ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceIntSmall-16 3.745Ki ± ∞ ¹ 1.023Ki ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceIntMedium-16 33.590Ki ± ∞ ¹ 9.523Ki ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceIntLarge-16 253.86Ki ± ∞ ¹ 96.02Ki ± ∞ ¹ ~ (p=1.000 n=1) ² geomean 36.83Ki 12.74Ki -65.41% ¹ need >= 6 samples for confidence interval at level 0.95 ² need >= 4 samples to detect a difference at alpha level 0.05 │ original.txt │ updated.txt │ │ allocs/op │ allocs/op vs base │ CollectionCopyMapStringSmall-16 47.00 ± ∞ ¹ 23.00 ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopyMapStringLarge-16 4.043k ± ∞ ¹ 2.004k ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceStringSmall-16 19.000 ± ∞ ¹ 2.000 ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceStringMedium-16 112.000 ± ∞ ¹ 2.000 ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceStringLarge-16 1016.000 ± ∞ ¹ 2.000 ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceIntSmall-16 9.000 ± ∞ ¹ 2.000 ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceIntMedium-16 12.000 ± ∞ ¹ 2.000 ± ∞ ¹ ~ (p=1.000 n=1) ² CollectionCopySliceIntLarge-16 15.000 ± ∞ ¹ 2.000 ± ∞ ¹ ~ (p=1.000 n=1) ² geomean 71.27 6.438 -90.97% ¹ need >= 6 samples for confidence interval at level 0.95 ² need >= 4 samples to detect a difference at alpha level 0.05 ```
diff --git a/ldvalue/value_array.go b/ldvalue/value_array.go
@@ -248,3 +248,129 @@ func (a ValueArray) Transform(fn func(index int, value Value) (Value, bool)) Val
 func (a ValueArray) String() string {
 	return a.JSONString()
 }
+
+func copyArbitraryArrayString(data []string) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = String(v)
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayBool(data []bool) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Bool(v)
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayInt(data []int) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayInt8(data []int8) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayInt16(data []int16) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayInt32(data []int32) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayInt64(data []int64) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayUint(data []uint) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayUint8(data []uint8) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayUint16(data []uint16) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayUint32(data []uint32) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayUint64(data []uint64) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayFloat32(data []float32) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(float64(v))
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
+
+func copyArbitraryArrayFloat64(data []float64) Value {
+	a := make([]Value, len(data))
+	for i, v := range data {
+		a[i] = Float64(v)
+	}
+
+	return Value{valueType: ArrayType, arrayValue: ValueArray{data: a}}
+}
diff --git a/ldvalue/value_array_test.go b/ldvalue/value_array_test.go
@@ -255,6 +255,87 @@ func TestValueArrayTransform(t *testing.T) {
 	assert.Equal(t, ValueArrayOf(), ValueArrayOf().Transform(shouldNotCallThis))
 }
 
+func TestCopyArbitrarySliceOfType(t *testing.T) {
+	var sNil []string
+	vm := CopyArbitraryValue(sNil)
+	assert.Equal(t, ArrayType, vm.Type())
+
+	sEmpty := []string{}
+	vm = CopyArbitraryValue(sEmpty)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{}, vm.arrayValue.data)
+
+	sStr := []string{"a", "b", "c"}
+	vm = CopyArbitraryValue(sStr)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{String("a"), String("b"), String("c")}, vm.arrayValue.data)
+
+	sBool := []bool{true, false}
+	vm = CopyArbitraryValue(sBool)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Bool(true), Bool(false)}, vm.arrayValue.data)
+
+	sInt := []int{1, 2, 3}
+	vm = CopyArbitraryValue(sInt)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Int(1), Int(2), Int(3)}, vm.arrayValue.data)
+
+	sInt8 := []int8{1, 2, 3}
+	vm = CopyArbitraryValue(sInt8)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Int(1), Int(2), Int(3)}, vm.arrayValue.data)
+
+	sInt16 := []int16{1, 2, 3}
+	vm = CopyArbitraryValue(sInt16)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Int(1), Int(2), Int(3)}, vm.arrayValue.data)
+
+	sInt32 := []int32{1, 2, 3}
+	vm = CopyArbitraryValue(sInt32)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Int(1), Int(2), Int(3)}, vm.arrayValue.data)
+
+	sInt64 := []int64{1, 2, 3}
+	vm = CopyArbitraryValue(sInt64)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Int(1), Int(2), Int(3)}, vm.arrayValue.data)
+
+	sUint := []uint{1, 2, 3}
+	vm = CopyArbitraryValue(sUint)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Int(1), Int(2), Int(3)}, vm.arrayValue.data)
+
+	sUint8 := []uint8{1, 2, 3}
+	vm = CopyArbitraryValue(sUint8)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Int(1), Int(2), Int(3)}, vm.arrayValue.data)
+
+	sUint16 := []uint16{1, 2, 3}
+	vm = CopyArbitraryValue(sUint16)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Int(1), Int(2), Int(3)}, vm.arrayValue.data)
+
+	sUint32 := []uint32{1, 2, 3}
+	vm = CopyArbitraryValue(sUint32)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Int(1), Int(2), Int(3)}, vm.arrayValue.data)
+
+	sUint64 := []uint64{1, 2, 3}
+	vm = CopyArbitraryValue(sUint64)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Int(1), Int(2), Int(3)}, vm.arrayValue.data)
+
+	sFloat32 := []float32{1.0, 2.0, 3.0}
+	vm = CopyArbitraryValue(sFloat32)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Float64(1.0), Float64(2.0), Float64(3.0)}, vm.arrayValue.data)
+
+	sFloat64 := []float64{1.1, 2.2, 3.3}
+	vm = CopyArbitraryValue(sFloat64)
+	assert.Equal(t, ArrayType, vm.Type())
+	assert.Equal(t, []Value{Float64(1.1), Float64(2.2), Float64(3.3)}, vm.arrayValue.data)
+}
+
 func shouldBeSameSlice(t *testing.T, s0 []Value, s1 []Value) {
 	old := s0[0]
 	s0[0] = String("temp-value")
diff --git a/ldvalue/value_base.go b/ldvalue/value_base.go
@@ -296,6 +296,34 @@ func CopyArbitraryValue(anyValue any) Value { //nolint:gocyclo // yes, we know i
 			return Null()
 		}
 		return ArrayOf((*o)...)
+	case []string:
+		return copyArbitraryArrayString(o)
+	case []bool:
+		return copyArbitraryArrayBool(o)
+	case []int:
+		return copyArbitraryArrayInt(o)
+	case []int8:
+		return copyArbitraryArrayInt8(o)
+	case []int16:
+		return copyArbitraryArrayInt16(o)
+	case []int32:
+		return copyArbitraryArrayInt32(o)
+	case []int64:
+		return copyArbitraryArrayInt64(o)
+	case []uint:
+		return copyArbitraryArrayUint(o)
+	case []uint8:
+		return copyArbitraryArrayUint8(o)
+	case []uint16:
+		return copyArbitraryArrayUint16(o)
+	case []uint32:
+		return copyArbitraryArrayUint32(o)
+	case []uint64:
+		return copyArbitraryArrayUint64(o)
+	case []float32:
+		return copyArbitraryArrayFloat32(o)
+	case []float64:
+		return copyArbitraryArrayFloat64(o)
 	case map[string]any:
 		return copyArbitraryValueMap(o)
 	case *map[string]any:
@@ -310,6 +338,34 @@ func CopyArbitraryValue(anyValue any) Value { //nolint:gocyclo // yes, we know i
 			return Null()
 		}
 		return CopyObject(*o)
+	case map[string]string:
+		return copyArbitraryMapString(o)
+	case map[string]bool:
+		return copyArbitraryMapBool(o)
+	case map[string]int:
+		return copyArbitraryMapInt(o)
+	case map[string]int8:
+		return copyArbitraryMapInt8(o)
+	case map[string]int16:
+		return copyArbitraryMapInt16(o)
+	case map[string]int32:
+		return copyArbitraryMapInt32(o)
+	case map[string]int64:
+		return copyArbitraryMapInt64(o)
+	case map[string]uint:
+		return copyArbitraryMapUint(o)
+	case map[string]uint8:
+		return copyArbitraryMapUint8(o)
+	case map[string]uint16:
+		return copyArbitraryMapUint16(o)
+	case map[string]uint32:
+		return copyArbitraryMapUint32(o)
+	case map[string]uint64:
+		return copyArbitraryMapUint64(o)
+	case map[string]float32:
+		return copyArbitraryMapFloat32(o)
+	case map[string]float64:
+		return copyArbitraryMapFloat64(o)
 	case json.RawMessage:
 		return Raw(o)
 	case *json.RawMessage:
diff --git a/ldvalue/value_collection_benchmark_test.go b/ldvalue/value_collection_benchmark_test.go
@@ -0,0 +1,111 @@
+package ldvalue
+
+import (
+	"math/rand"
+	"testing"
+)
+
+func BenchmarkCollectionCopyMapStringSmall(b *testing.B) {
+	input := generateRandomMap(10)
+
+	for i := 0; i < b.N; i++ {
+		_ = CopyArbitraryValue(input)
+	}
+}
+
+func BenchmarkCollectionCopyMapStringLarge(b *testing.B) {
+	input := generateRandomMap(1_000)
+
+	for i := 0; i < b.N; i++ {
+		_ = CopyArbitraryValue(input)
+	}
+}
+
+func generateRandomMap(i int) map[string]string {
+	m := make(map[string]string, i)
+
+	for j := 0; j < i; j++ {
+		m[generateRandomString()] = generateRandomString()
+	}
+
+	return m
+}
+
+func BenchmarkCollectionCopySliceStringSmall(b *testing.B) {
+	input := generateStringSlice(10)
+
+	for i := 0; i < b.N; i++ {
+		_ = CopyArbitraryValue(input)
+	}
+}
+
+func BenchmarkCollectionCopySliceStringMedium(b *testing.B) {
+	input := generateStringSlice(100)
+
+	for i := 0; i < b.N; i++ {
+		_ = CopyArbitraryValue(input)
+	}
+}
+
+func BenchmarkCollectionCopySliceStringLarge(b *testing.B) {
+	input := generateStringSlice(1_000)
+
+	for i := 0; i < b.N; i++ {
+		_ = CopyArbitraryValue(input)
+	}
+}
+
+func BenchmarkCollectionCopySliceIntSmall(b *testing.B) {
+	input := generateIntSlice(10)
+
+	for i := 0; i < b.N; i++ {
+		_ = CopyArbitraryValue(input)
+	}
+}
+
+func BenchmarkCollectionCopySliceIntMedium(b *testing.B) {
+	input := generateIntSlice(100)
+
+	for i := 0; i < b.N; i++ {
+		_ = CopyArbitraryValue(input)
+	}
+}
+
+func BenchmarkCollectionCopySliceIntLarge(b *testing.B) {
+	input := generateIntSlice(1_000)
+
+	for i := 0; i < b.N; i++ {
+		_ = CopyArbitraryValue(input)
+	}
+}
+
+func generateIntSlice(n int) []int {
+	s := make([]int, n)
+
+	for i := 0; i < n; i++ {
+		s[i] = rand.Int()
+	}
+
+	return s
+}
+
+func generateStringSlice(n int) []string {
+	s := make([]string, n)
+
+	for i := 0; i < n; i++ {
+		s[i] = generateRandomString()
+	}
+
+	return s
+}
+
+func generateRandomString() string {
+	alpha := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
+
+	b := make([]byte, 10)
+	for i := range b {
+		b[i] = alpha[rand.Intn(len(alpha))]
+	}
+
+	return string(b)
+}
diff --git a/ldvalue/value_map.go b/ldvalue/value_map.go
diff --git a/ldvalue/value_map_test.go b/ldvalue/value_map_test.go
