diff --git a/CHANGELOG.md b/CHANGELOG.md index 26dcb817bf0..18eb7103214 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed - Introduce the `EMPTY` Type in `go.opentelemetry.io/otel/attribute` to reflect that an empty value is now a valid value, with `INVALID` remaining as a deprecated alias of `EMPTY`. (#8038) +- Refactor slice handling in `go.opentelemetry.io/otel/attribute` to optimize short slice values with fixed-size fast paths. (#8039) ### Deprecated diff --git a/attribute/benchmark_test.go b/attribute/benchmark_test.go index 65eda55371e..01b06fbbb50 100644 --- a/attribute/benchmark_test.go +++ b/attribute/benchmark_test.go @@ -60,28 +60,38 @@ func BenchmarkBool(b *testing.B) { } func BenchmarkBoolSlice(b *testing.B) { - k, v := "bool slice", []bool{true, false, true} - kv := attribute.BoolSlice(k, v) + for _, bench := range []struct { + name string + v []bool + }{ + {name: "Len2", v: []bool{true, false}}, + {name: "Len8", v: []bool{true, false, true, false, true, false, true, false}}, + } { + b.Run(bench.name, func(b *testing.B) { + k, v := "bool slice", bench.v + kv := attribute.BoolSlice(k, v) - b.Run("Value", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outV = attribute.BoolSliceValue(v) - } - }) - b.Run("KeyValue", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outKV = attribute.BoolSlice(k, v) - } - }) - b.Run("AsBoolSlice", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outBoolSlice = kv.Value.AsBoolSlice() - } - }) - b.Run("Emit", benchmarkEmit(kv)) + b.Run("Value", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outV = attribute.BoolSliceValue(v) + } + }) + b.Run("KeyValue", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outKV = attribute.BoolSlice(k, v) + } + }) + b.Run("AsBoolSlice", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outBoolSlice = kv.Value.AsBoolSlice() + } + }) + b.Run("Emit", benchmarkEmit(kv)) + }) + } } func BenchmarkInt(b *testing.B) { @@ -104,22 +114,32 @@ func BenchmarkInt(b *testing.B) { } func BenchmarkIntSlice(b *testing.B) { - k, v := "int slice", []int{42, -3, 12} - kv := attribute.IntSlice(k, v) + for _, bench := range []struct { + name string + v []int + }{ + {name: "Len2", v: []int{42, -3}}, + {name: "Len8", v: []int{42, -3, 12, 7, 9, 11, -5, 0}}, + } { + b.Run(bench.name, func(b *testing.B) { + k, v := "int slice", bench.v + kv := attribute.IntSlice(k, v) - b.Run("Value", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outV = attribute.IntSliceValue(v) - } - }) - b.Run("KeyValue", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outKV = attribute.IntSlice(k, v) - } - }) - b.Run("Emit", benchmarkEmit(kv)) + b.Run("Value", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outV = attribute.IntSliceValue(v) + } + }) + b.Run("KeyValue", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outKV = attribute.IntSlice(k, v) + } + }) + b.Run("Emit", benchmarkEmit(kv)) + }) + } } func BenchmarkInt64(b *testing.B) { @@ -148,28 +168,38 @@ func BenchmarkInt64(b *testing.B) { } func BenchmarkInt64Slice(b *testing.B) { - k, v := "int64 slice", []int64{42, -3, 12} - kv := attribute.Int64Slice(k, v) + for _, bench := range []struct { + name string + v []int64 + }{ + {name: "Len2", v: []int64{42, -3}}, + {name: "Len8", v: []int64{42, -3, 12, 7, 9, 11, -5, 0}}, + } { + b.Run(bench.name, func(b *testing.B) { + k, v := "int64 slice", bench.v + kv := attribute.Int64Slice(k, v) - b.Run("Value", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outV = attribute.Int64SliceValue(v) - } - }) - b.Run("KeyValue", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outKV = attribute.Int64Slice(k, v) - } - }) - b.Run("AsInt64Slice", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outInt64Slice = kv.Value.AsInt64Slice() - } - }) - b.Run("Emit", benchmarkEmit(kv)) + b.Run("Value", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outV = attribute.Int64SliceValue(v) + } + }) + b.Run("KeyValue", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outKV = attribute.Int64Slice(k, v) + } + }) + b.Run("AsInt64Slice", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outInt64Slice = kv.Value.AsInt64Slice() + } + }) + b.Run("Emit", benchmarkEmit(kv)) + }) + } } func BenchmarkFloat64(b *testing.B) { @@ -198,28 +228,38 @@ func BenchmarkFloat64(b *testing.B) { } func BenchmarkFloat64Slice(b *testing.B) { - k, v := "float64 slice", []float64{42, -3, 12} - kv := attribute.Float64Slice(k, v) + for _, bench := range []struct { + name string + v []float64 + }{ + {name: "Len2", v: []float64{42, -3}}, + {name: "Len8", v: []float64{42, -3, 12, 7, 9, 11, -5, 0}}, + } { + b.Run(bench.name, func(b *testing.B) { + k, v := "float64 slice", bench.v + kv := attribute.Float64Slice(k, v) - b.Run("Value", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outV = attribute.Float64SliceValue(v) - } - }) - b.Run("KeyValue", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outKV = attribute.Float64Slice(k, v) - } - }) - b.Run("AsFloat64Slice", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outFloat64Slice = kv.Value.AsFloat64Slice() - } - }) - b.Run("Emit", benchmarkEmit(kv)) + b.Run("Value", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outV = attribute.Float64SliceValue(v) + } + }) + b.Run("KeyValue", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outKV = attribute.Float64Slice(k, v) + } + }) + b.Run("AsFloat64Slice", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outFloat64Slice = kv.Value.AsFloat64Slice() + } + }) + b.Run("Emit", benchmarkEmit(kv)) + }) + } } func BenchmarkString(b *testing.B) { @@ -248,28 +288,38 @@ func BenchmarkString(b *testing.B) { } func BenchmarkStringSlice(b *testing.B) { - k, v := "float64 slice", []string{"forty-two", "negative three", "twelve"} - kv := attribute.StringSlice(k, v) + for _, bench := range []struct { + name string + v []string + }{ + {name: "Len2", v: []string{"forty-two", "negative three"}}, + {name: "Len8", v: []string{"forty-two", "negative three", "twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen"}}, + } { + b.Run(bench.name, func(b *testing.B) { + k, v := "string slice", bench.v + kv := attribute.StringSlice(k, v) - b.Run("Value", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outV = attribute.StringSliceValue(v) - } - }) - b.Run("KeyValue", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outKV = attribute.StringSlice(k, v) - } - }) - b.Run("AsStringSlice", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - outStrSlice = kv.Value.AsStringSlice() - } - }) - b.Run("Emit", benchmarkEmit(kv)) + b.Run("Value", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outV = attribute.StringSliceValue(v) + } + }) + b.Run("KeyValue", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outKV = attribute.StringSlice(k, v) + } + }) + b.Run("AsStringSlice", func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + outStrSlice = kv.Value.AsStringSlice() + } + }) + b.Run("Emit", benchmarkEmit(kv)) + }) + } } func BenchmarkSetEquals(b *testing.B) { diff --git a/attribute/internal/attribute.go b/attribute/internal/attribute.go index 7f5eae877da..d9f51fa2d7f 100644 --- a/attribute/internal/attribute.go +++ b/attribute/internal/attribute.go @@ -11,80 +11,63 @@ import ( "reflect" ) -// BoolSliceValue converts a bool slice into an array with same elements as slice. -func BoolSliceValue(v []bool) any { - cp := reflect.New(reflect.ArrayOf(len(v), reflect.TypeFor[bool]())).Elem() - reflect.Copy(cp, reflect.ValueOf(v)) - return cp.Interface() +// sliceElem is the exact set of element types stored in attribute slice values. +// Using a closed set prevents accidental instantiations for unsupported types. +type sliceElem interface { + bool | int64 | float64 | string } -// Int64SliceValue converts an int64 slice into an array with same elements as slice. -func Int64SliceValue(v []int64) any { - cp := reflect.New(reflect.ArrayOf(len(v), reflect.TypeFor[int64]())).Elem() - reflect.Copy(cp, reflect.ValueOf(v)) - return cp.Interface() -} - -// Float64SliceValue converts a float64 slice into an array with same elements as slice. -func Float64SliceValue(v []float64) any { - cp := reflect.New(reflect.ArrayOf(len(v), reflect.TypeFor[float64]())).Elem() - reflect.Copy(cp, reflect.ValueOf(v)) - return cp.Interface() -} +// SliceValue converts a slice into an array with the same elements. +func SliceValue[T sliceElem](v []T) any { + // Keep only the common tiny-slice cases out of reflection. Extending this + // much further increases code size for diminishing benefit while larger + // slices still need the generic reflective path to preserve comparability. + // This matches the short lengths that show up most often in local + // benchmarks and semantic convention examples while leaving larger, less + // predictable slices on the generic reflective path. + switch len(v) { + case 0: + return [0]T{} + case 1: + return [1]T{v[0]} + case 2: + return [2]T{v[0], v[1]} + case 3: + return [3]T{v[0], v[1], v[2]} + } -// StringSliceValue converts a string slice into an array with same elements as slice. -func StringSliceValue(v []string) any { - cp := reflect.New(reflect.ArrayOf(len(v), reflect.TypeFor[string]())).Elem() - reflect.Copy(cp, reflect.ValueOf(v)) - return cp.Interface() + return sliceValueReflect(v) } -// AsBoolSlice converts a bool array into a slice into with same elements as array. -func AsBoolSlice(v any) []bool { - rv := reflect.ValueOf(v) - if rv.Type().Kind() != reflect.Array { - return nil +// AsSlice converts an array into a slice with the same elements. +func AsSlice[T sliceElem](v any) []T { + // Mirror the small fixed-array fast path used by SliceValue. + switch a := v.(type) { + case [0]T: + return []T{} + case [1]T: + return []T{a[0]} + case [2]T: + return []T{a[0], a[1]} + case [3]T: + return []T{a[0], a[1], a[2]} } - cpy := make([]bool, rv.Len()) - if len(cpy) > 0 { - _ = reflect.Copy(reflect.ValueOf(cpy), rv) - } - return cpy -} -// AsInt64Slice converts an int64 array into a slice into with same elements as array. -func AsInt64Slice(v any) []int64 { - rv := reflect.ValueOf(v) - if rv.Type().Kind() != reflect.Array { - return nil - } - cpy := make([]int64, rv.Len()) - if len(cpy) > 0 { - _ = reflect.Copy(reflect.ValueOf(cpy), rv) - } - return cpy + return asSliceReflect[T](v) } -// AsFloat64Slice converts a float64 array into a slice into with same elements as array. -func AsFloat64Slice(v any) []float64 { - rv := reflect.ValueOf(v) - if rv.Type().Kind() != reflect.Array { - return nil - } - cpy := make([]float64, rv.Len()) - if len(cpy) > 0 { - _ = reflect.Copy(reflect.ValueOf(cpy), rv) - } - return cpy +func sliceValueReflect[T sliceElem](v []T) any { + cp := reflect.New(reflect.ArrayOf(len(v), reflect.TypeFor[T]())).Elem() + reflect.Copy(cp, reflect.ValueOf(v)) + return cp.Interface() } -// AsStringSlice converts a string array into a slice into with same elements as array. -func AsStringSlice(v any) []string { +func asSliceReflect[T sliceElem](v any) []T { rv := reflect.ValueOf(v) - if rv.Type().Kind() != reflect.Array { + if !rv.IsValid() || rv.Kind() != reflect.Array || rv.Type().Elem() != reflect.TypeFor[T]() { return nil } - cpy := make([]string, rv.Len()) + cpy := make([]T, rv.Len()) if len(cpy) > 0 { _ = reflect.Copy(reflect.ValueOf(cpy), rv) } diff --git a/attribute/internal/attribute_test.go b/attribute/internal/attribute_test.go index e0ebb06439a..59d1d4138c6 100644 --- a/attribute/internal/attribute_test.go +++ b/attribute/internal/attribute_test.go @@ -10,37 +10,37 @@ import ( var wrapFloat64SliceValue = func(v any) any { if vi, ok := v.([]float64); ok { - return Float64SliceValue(vi) + return SliceValue(vi) } return nil } var wrapInt64SliceValue = func(v any) any { if vi, ok := v.([]int64); ok { - return Int64SliceValue(vi) + return SliceValue(vi) } return nil } var wrapBoolSliceValue = func(v any) any { if vi, ok := v.([]bool); ok { - return BoolSliceValue(vi) + return SliceValue(vi) } return nil } var wrapStringSliceValue = func(v any) any { if vi, ok := v.([]string); ok { - return StringSliceValue(vi) + return SliceValue(vi) } return nil } var ( - wrapAsBoolSlice = func(v any) any { return AsBoolSlice(v) } - wrapAsInt64Slice = func(v any) any { return AsInt64Slice(v) } - wrapAsFloat64Slice = func(v any) any { return AsFloat64Slice(v) } - wrapAsStringSlice = func(v any) any { return AsStringSlice(v) } + wrapAsBoolSlice = func(v any) any { return AsSlice[bool](v) } + wrapAsInt64Slice = func(v any) any { return AsSlice[int64](v) } + wrapAsFloat64Slice = func(v any) any { return AsSlice[float64](v) } + wrapAsStringSlice = func(v any) any { return AsSlice[string](v) } ) func TestSliceValue(t *testing.T) { @@ -95,50 +95,112 @@ func TestSliceValue(t *testing.T) { } } +func TestAsSliceMismatchedType(t *testing.T) { + tests := []struct { + name string + fn func() any + }{ + {name: "bool from int64 array", fn: func() any { return AsSlice[bool]([2]int64{1, 2}) }}, + {name: "int64 from float64 array", fn: func() any { return AsSlice[int64]([2]float64{1, 2}) }}, + {name: "float64 from string array", fn: func() any { return AsSlice[float64]([2]string{"1", "2"}) }}, + {name: "string from bool array", fn: func() any { return AsSlice[string]([2]bool{true, false}) }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.fn() + rv := reflect.ValueOf(got) + if !rv.IsNil() { + t.Fatalf("got %v, want nil", got) + } + }) + } +} + // sync is a global used to ensure the benchmark are not optimized away. var sync any func BenchmarkBoolSliceValue(b *testing.B) { - b.ReportAllocs() - s := []bool{true, false, true, false} - - for b.Loop() { - sync = BoolSliceValue(s) + for _, bench := range []struct { + name string + s []bool + }{ + {name: "Len2", s: []bool{true, false}}, + {name: "Len8", s: []bool{true, false, true, false, true, false, true, false}}, + } { + b.Run(bench.name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + sync = SliceValue(bench.s) + } + }) } } func BenchmarkInt64SliceValue(b *testing.B) { - b.ReportAllocs() - s := []int64{1, 2, 3, 4} - - for b.Loop() { - sync = Int64SliceValue(s) + for _, bench := range []struct { + name string + s []int64 + }{ + {name: "Len2", s: []int64{1, 2}}, + {name: "Len8", s: []int64{1, 2, 3, 4, 5, 6, 7, 8}}, + } { + b.Run(bench.name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + sync = SliceValue(bench.s) + } + }) } } func BenchmarkFloat64SliceValue(b *testing.B) { - b.ReportAllocs() - s := []float64{1.2, 3.4, 5.6, 7.8} - - for b.Loop() { - sync = Float64SliceValue(s) + for _, bench := range []struct { + name string + s []float64 + }{ + {name: "Len2", s: []float64{1.2, 3.4}}, + {name: "Len8", s: []float64{1.2, 3.4, 5.6, 7.8, 9.1, 2.3, 4.5, 6.7}}, + } { + b.Run(bench.name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + sync = SliceValue(bench.s) + } + }) } } func BenchmarkStringSliceValue(b *testing.B) { - b.ReportAllocs() - s := []string{"a", "b", "c", "d"} - - for b.Loop() { - sync = StringSliceValue(s) + for _, bench := range []struct { + name string + s []string + }{ + {name: "Len2", s: []string{"a", "b"}}, + {name: "Len8", s: []string{"a", "b", "c", "d", "e", "f", "g", "h"}}, + } { + b.Run(bench.name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + sync = SliceValue(bench.s) + } + }) } } func BenchmarkAsFloat64Slice(b *testing.B) { - b.ReportAllocs() - var in any = [2]float64{1, 2.3} - - for b.Loop() { - sync = AsFloat64Slice(in) + for _, bench := range []struct { + name string + in any + }{ + {name: "Len2", in: [2]float64{1, 2.3}}, + {name: "Len8", in: [8]float64{1, 2.3, 3.4, 4.5, 5.6, 6.7, 7.8, 8.9}}, + } { + b.Run(bench.name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + sync = AsSlice[float64](bench.in) + } + }) } } diff --git a/attribute/value.go b/attribute/value.go index fe0f62be78d..db04b1326c3 100644 --- a/attribute/value.go +++ b/attribute/value.go @@ -6,7 +6,6 @@ package attribute // import "go.opentelemetry.io/otel/attribute" import ( "encoding/json" "fmt" - "reflect" "strconv" attribute "go.opentelemetry.io/otel/attribute/internal" @@ -62,7 +61,7 @@ func BoolValue(v bool) Value { // BoolSliceValue creates a BOOLSLICE Value. func BoolSliceValue(v []bool) Value { - return Value{vtype: BOOLSLICE, slice: attribute.BoolSliceValue(v)} + return Value{vtype: BOOLSLICE, slice: attribute.SliceValue(v)} } // IntValue creates an INT64 Value. @@ -70,16 +69,30 @@ func IntValue(v int) Value { return Int64Value(int64(v)) } -// IntSliceValue creates an INTSLICE Value. +// IntSliceValue creates an INT64SLICE Value. func IntSliceValue(v []int) Value { - cp := reflect.New(reflect.ArrayOf(len(v), reflect.TypeFor[int64]())) - for i, val := range v { - cp.Elem().Index(i).SetInt(int64(val)) - } - return Value{ - vtype: INT64SLICE, - slice: cp.Elem().Interface(), + val := Value{vtype: INT64SLICE} + + // Avoid the common tiny-slice cases from allocating a new slice. + switch len(v) { + case 0: + val.slice = [0]int64{} + case 1: + val.slice = [1]int64{int64(v[0])} + case 2: + val.slice = [2]int64{int64(v[0]), int64(v[1])} + case 3: + val.slice = [3]int64{int64(v[0]), int64(v[1]), int64(v[2])} + default: + // Fallback to a new slice for larger slices. + cp := make([]int64, len(v)) + for i, val := range v { + cp[i] = int64(val) + } + val.slice = attribute.SliceValue(cp) } + + return val } // Int64Value creates an INT64 Value. @@ -92,7 +105,7 @@ func Int64Value(v int64) Value { // Int64SliceValue creates an INT64SLICE Value. func Int64SliceValue(v []int64) Value { - return Value{vtype: INT64SLICE, slice: attribute.Int64SliceValue(v)} + return Value{vtype: INT64SLICE, slice: attribute.SliceValue(v)} } // Float64Value creates a FLOAT64 Value. @@ -105,7 +118,7 @@ func Float64Value(v float64) Value { // Float64SliceValue creates a FLOAT64SLICE Value. func Float64SliceValue(v []float64) Value { - return Value{vtype: FLOAT64SLICE, slice: attribute.Float64SliceValue(v)} + return Value{vtype: FLOAT64SLICE, slice: attribute.SliceValue(v)} } // StringValue creates a STRING Value. @@ -118,7 +131,7 @@ func StringValue(v string) Value { // StringSliceValue creates a STRINGSLICE Value. func StringSliceValue(v []string) Value { - return Value{vtype: STRINGSLICE, slice: attribute.StringSliceValue(v)} + return Value{vtype: STRINGSLICE, slice: attribute.SliceValue(v)} } // Type returns a type of the Value. @@ -142,7 +155,7 @@ func (v Value) AsBoolSlice() []bool { } func (v Value) asBoolSlice() []bool { - return attribute.AsBoolSlice(v.slice) + return attribute.AsSlice[bool](v.slice) } // AsInt64 returns the int64 value. Make sure that the Value's type is @@ -161,7 +174,7 @@ func (v Value) AsInt64Slice() []int64 { } func (v Value) asInt64Slice() []int64 { - return attribute.AsInt64Slice(v.slice) + return attribute.AsSlice[int64](v.slice) } // AsFloat64 returns the float64 value. Make sure that the Value's @@ -180,7 +193,7 @@ func (v Value) AsFloat64Slice() []float64 { } func (v Value) asFloat64Slice() []float64 { - return attribute.AsFloat64Slice(v.slice) + return attribute.AsSlice[float64](v.slice) } // AsString returns the string value. Make sure that the Value's type @@ -199,7 +212,7 @@ func (v Value) AsStringSlice() []string { } func (v Value) asStringSlice() []string { - return attribute.AsStringSlice(v.slice) + return attribute.AsSlice[string](v.slice) } type unknownValueType struct{}