river/internal/value: change representation of objects and maps

pkg/river/internal/stdlib/stdlib.go

@@ -4,13 +4,10 @@ package stdlib
 import (
 	"encoding/json"
 	"os"
-	"reflect"
 
 	"github.com/grafana/agent/pkg/river/internal/value"
 )
 
-var goAny = reflect.TypeOf((*interface{})(nil)).Elem()
-
 // Functions returns the list of stdlib functions by name. The interface{}
 // value is always a River-compatible function value, where functions have at
 // least one non-error return value, with an optionally supported error return
@@ -53,39 +50,14 @@ var Functions = map[string]interface{}{
 			return args[0], nil
 		}
 
-		// If the imcoming Go slices have the same type, we can have our resulting
-		// slice use the same type. This will allow decoding to use the direct
-		// assignment optimization.
-		//
-		// However, if the types don't match, then we're forced to fall back to
-		// returning []interface{}.
-		//
-		// TODO(rfratto): This could fall back to checking the elements if the
-		// array/slice types don't match. It would be slower, but the direct
-		// assignment optimization probably justifies it.
-		useType := args[0].Reflect().Type()
-		for i := 1; i < len(args); i++ {
-			if args[i].Reflect().Type() != useType {
-				useType = reflect.SliceOf(goAny)
-				break
-			}
-		}
-
-		// Build out the final array.
-		raw := reflect.MakeSlice(useType, finalSize, finalSize)
-
-		var argNum int
+		raw := make([]value.Value, 0, finalSize)
 		for _, arg := range args {
 			for i := 0; i < arg.Len(); i++ {
-				elem := arg.Index(i)
-				if elem.Type() != value.TypeNull {
-					raw.Index(argNum).Set(elem.Reflect())
-				}
-				argNum++
+				raw = append(raw, arg.Index(i))
 			}
 		}
 
-		return value.Encode(raw.Interface()), nil
+		return value.Array(raw...), nil
 	}),
 
 	"json_decode": func(in string) (interface{}, error) {

pkg/river/internal/value/decode_benchmarks_test.go

@@ -28,3 +28,63 @@ func BenchmarkObjectDecode(b *testing.B) {
 		_ = value.Decode(sourceVal, &dst)
 	}
 }
+
+func BenchmarkObject(b *testing.B) {
+	b.Run("Non-capsule", func(b *testing.B) {
+		b.StopTimer()
+
+		vals := make(map[string]value.Value)
+		for i := 0; i < 20; i++ {
+			vals[fmt.Sprintf("%d", i)] = value.Int(int64(i))
+		}
+
+		b.StartTimer()
+		for i := 0; i < b.N; i++ {
+			_ = value.Object(vals)
+		}
+	})
+
+	b.Run("Capsule", func(b *testing.B) {
+		b.StopTimer()
+
+		vals := make(map[string]value.Value)
+		for i := 0; i < 20; i++ {
+			vals[fmt.Sprintf("%d", i)] = value.Encapsulate(make(chan int))
+		}
+
+		b.StartTimer()
+		for i := 0; i < b.N; i++ {
+			_ = value.Object(vals)
+		}
+	})
+}
+
+func BenchmarkArray(b *testing.B) {
+	b.Run("Non-capsule", func(b *testing.B) {
+		b.StopTimer()
+
+		var vals []value.Value
+		for i := 0; i < 20; i++ {
+			vals = append(vals, value.Int(int64(i)))
+		}
+
+		b.StartTimer()
+		for i := 0; i < b.N; i++ {
+			_ = value.Array(vals...)
+		}
+	})
+
+	b.Run("Capsule", func(b *testing.B) {
+		b.StopTimer()
+
+		var vals []value.Value
+		for i := 0; i < 20; i++ {
+			vals = append(vals, value.Encapsulate(make(chan int)))
+		}
+
+		b.StartTimer()
+		for i := 0; i < b.N; i++ {
+			_ = value.Array(vals...)
+		}
+	})
+}

pkg/river/internal/value/type.go

@@ -43,6 +43,9 @@ func (t Type) String() string {
 	return fmt.Sprintf("Type(%d)", t)
 }
 
+// GoString returns the name of t.
+func (t Type) GoString() string { return t.String() }
+
 // RiverType returns the River type from the Go type.
 //
 // Go types map to River types using the following rules:

pkg/river/internal/value/value.go

@@ -25,6 +25,7 @@ var (
 	goDurationPtr     = reflect.TypeOf((*time.Duration)(nil))
 	goRiverDecoder    = reflect.TypeOf((*Unmarshaler)(nil)).Elem()
 	goRawRiverFunc    = reflect.TypeOf((RawFunction)(nil))
+	goRiverValue      = reflect.TypeOf(Null)
 )
 
 // NOTE(rfratto): This package is extremely sensitive to performance, so
@@ -61,31 +62,19 @@ func Bool(b bool) Value { return Value{rv: reflect.ValueOf(b), ty: TypeBool} }
 // Object returns a new value from m. A copy of m is made for producing the
 // Value.
 func Object(m map[string]Value) Value {
-	raw := reflect.MakeMapWithSize(reflect.TypeOf(map[string]interface{}(nil)), len(m))
-
-	for k, v := range m {
-		raw.SetMapIndex(reflect.ValueOf(k), v.rv)
+	return Value{
+		rv: reflect.ValueOf(m),
+		ty: TypeObject,
 	}
-
-	return Value{rv: raw, ty: TypeObject}
 }
 
 // Array creates an array from the given values. A copy of the vv slice is made
 // for producing the Value.
 func Array(vv ...Value) Value {
-	// Arrays should be slices otherwise any reference to them gets copied by
-	// value into a new pointer.
-	arrayType := reflect.SliceOf(goAny)
-	raw := reflect.MakeSlice(arrayType, len(vv), len(vv))
-
-	for i, v := range vv {
-		if v.ty == TypeNull {
-			continue
-		}
-		raw.Index(i).Set(v.rv)
+	return Value{
+		rv: reflect.ValueOf(vv),
+		ty: TypeArray,
 	}
-
-	return Value{rv: raw, ty: TypeArray}
 }
 
 // Func makes a new function Value from f. Func panics if f does not map to a
@@ -271,6 +260,14 @@ func makeValue(v reflect.Value) Value {
 		v = v.Elem()
 	}
 
+	// Special case: a reflect.Value may be a value.Value when it's coming from a
+	// River array or object. We can unwrap the inner value here before
+	// continuing.
+	if v.IsValid() && v.Type() == goRiverValue {
+		// Unwrap the inner value.
+		v = v.Interface().(Value).rv
+	}
+
 	// Before we get the River type of the Value, we need to see if it's possible
 	// to get a pointer to v. This ensures that if v is a non-pointer field of an
 	// addressable struct, still detect the type of v as if it was a pointer.

pkg/river/internal/value/value_test.go

@@ -2,6 +2,7 @@ package value_test
 
 import (
 	"fmt"
+	"io"
 	"testing"
 
 	"github.com/grafana/agent/pkg/river/internal/value"
@@ -143,3 +144,17 @@ func TestValue_Call(t *testing.T) {
 		})
 	})
 }
+
+func TestValue_Interface_In_Array(t *testing.T) {
+	type Container struct {
+		Field io.Closer `river:"field,attr"`
+	}
+
+	val := value.Encode(Container{Field: io.NopCloser(nil)})
+	fieldVal, ok := val.Key("field")
+	require.True(t, ok, "field not found in object")
+	require.Equal(t, value.TypeCapsule, fieldVal.Type())
+
+	arrVal := value.Array(fieldVal)
+	require.Equal(t, value.TypeCapsule, arrVal.Index(0).Type())
+}

pkg/river/vm/vm_stdlib_test.go

@@ -40,17 +40,19 @@ func TestVM_Stdlib(t *testing.T) {
 }
 
 func BenchmarkConcat(b *testing.B) {
-	// There's a bit of setup work to do here: we want to create a scope
-	// holding a slice of the Data type, which has a fair amount of data in
-	// it.
+	// There's a bit of setup work to do here: we want to create a scope holding
+	// a slice of the Person type, which has a fair amount of data in it.
 	//
 	// We then want to pass it through concat.
 	//
 	// If the code path is fully optimized, there will be no intermediate
 	// translations to interface{}.
-	type Data map[string]string
+	type Person struct {
+		Name  string            `river:"name,attr"`
+		Attrs map[string]string `river:"attrs,attr"`
+	}
 	type Body struct {
-		Values []Data `river:"values,attr"`
+		Values []Person `river:"values,attr"`
 	}
 
 	in := `values = concat(values_ref)`
@@ -59,17 +61,20 @@ func BenchmarkConcat(b *testing.B) {
 
 	eval := vm.New(f)
 
-	valuesRef := make([]Data, 0, 20)
+	valuesRef := make([]Person, 0, 20)
 	for i := 0; i < 20; i++ {
-		data := make(Data, 20)
+		data := make(map[string]string, 20)
 		for j := 0; j < 20; j++ {
 			var (
 				key   = fmt.Sprintf("key_%d", i+1)
 				value = fmt.Sprintf("value_%d", i+1)
 			)
 			data[key] = value
 		}
-		valuesRef = append(valuesRef, data)
+		valuesRef = append(valuesRef, Person{
+			Name:  "Test Person",
+			Attrs: data,
+		})
 	}
 	scope := &vm.Scope{
 		Variables: map[string]interface{}{