Rent ValueInterfaces to avoid allocations

Antoine Pelisse · Antoine Pelisse · commit 475340671809 · 2019-12-07T16:53:57.000-08:00
diff --git a/fieldpath/element.go b/fieldpath/element.go
@@ -127,7 +127,7 @@ func KeyByFields(nameValues ...interface{}) *value.FieldList {
 	}
 	out := value.FieldList{}
 	for i := 0; i < len(nameValues)-1; i += 2 {
-		out = append(out, value.Field{Name: nameValues[i].(string), Value: value.ValueInterface{Value: nameValues[i+1]}})
+		out = append(out, value.Field{Name: nameValues[i].(string), Value: value.NewValueInterface(nameValues[i+1])})
 	}
 	out.Sort()
 	return &out
diff --git a/fieldpath/element_test.go b/fieldpath/element_test.go
@@ -65,7 +65,7 @@ func TestPathElementSet(t *testing.T) {
 func strptr(s string) *string { return &s }
 func intptr(i int) *int       { return &i }
 func valptr(i interface{}) *value.Value {
-	v := value.Value(value.ValueInterface{Value: i})
+	v := value.NewValueInterface(i)
 	return &v
 }
 
diff --git a/fieldpath/fromvalue_test.go b/fieldpath/fromvalue_test.go
@@ -71,7 +71,7 @@ func TestFromValue(t *testing.T) {
 			if err != nil {
 				t.Fatalf("couldn't parse: %v", err)
 			}
-			got := SetFromValue(value.ValueInterface{Value: v})
+			got := SetFromValue(value.NewValueInterface(v))
 			if !got.Equals(tt.set) {
 				t.Errorf("wanted\n%s\nbut got\n%s\n", tt.set, got)
 			}
diff --git a/fieldpath/path_test.go b/fieldpath/path_test.go
@@ -24,7 +24,7 @@ import (
 
 var (
 	_V = func(i interface{}) *value.Value {
-		v := value.Value(value.ValueInterface{Value: i})
+		v := value.NewValueInterface(i)
 		return &v
 	}
 )
diff --git a/fieldpath/pathelementmap_test.go b/fieldpath/pathelementmap_test.go
@@ -29,22 +29,22 @@ func TestPathElementValueMap(t *testing.T) {
 		t.Fatal("Unexpected path-element found in empty map")
 	}
 
-	m.Insert(PathElement{FieldName: strptr("carrot")}, value.ValueInterface{Value: "knife"})
-	m.Insert(PathElement{FieldName: strptr("chive")}, value.ValueInterface{Value: 2})
+	m.Insert(PathElement{FieldName: strptr("carrot")}, value.NewValueInterface("knife"))
+	m.Insert(PathElement{FieldName: strptr("chive")}, value.NewValueInterface(2))
 
 	if _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
 		t.Fatal("Unexpected path-element in map")
 	}
 
 	if val, ok := m.Get(PathElement{FieldName: strptr("carrot")}); !ok {
 		t.Fatal("Missing path-element in map")
-	} else if !value.Equals(val, value.ValueInterface{Value: "knife"}) {
+	} else if !value.Equals(val, value.NewValueInterface("knife")) {
 		t.Fatalf("Unexpected value found: %#v", val)
 	}
 
 	if val, ok := m.Get(PathElement{FieldName: strptr("chive")}); !ok {
 		t.Fatal("Missing path-element in map")
-	} else if !value.Equals(val, value.ValueInterface{Value: 2}) {
+	} else if !value.Equals(val, value.NewValueInterface(2)) {
 		t.Fatalf("Unexpected value found: %#v", val)
 	}
 }
diff --git a/merge/conflict_test.go b/merge/conflict_test.go
@@ -30,7 +30,7 @@ var (
 	_P   = fieldpath.MakePathOrDie
 	_KBF = fieldpath.KeyByFields
 	_V   = func(i interface{}) *value.Value {
-		v := value.Value(value.ValueInterface{Value: i})
+		v := value.NewValueInterface(i)
 		return &v
 	}
 )
diff --git a/typed/merge.go b/typed/merge.go
@@ -321,6 +321,7 @@ func (w *mergingWalker) visitMapItems(t *schema.Map, lhs, rhs value.Map) (errs V
 			if rhs != nil {
 				if item, ok := rhs.Get(key); ok {
 					rval = &item
+					defer (*rval).Recycle()
 				}
 			}
 			errs = append(errs, w.visitMapItem(t, out, key, &val, rval)...)
@@ -331,7 +332,8 @@ func (w *mergingWalker) visitMapItems(t *schema.Map, lhs, rhs value.Map) (errs V
 	if rhs != nil {
 		rhs.Iterate(func(key string, val value.Value) bool {
 			if lhs != nil {
-				if _, ok := lhs.Get(key); ok {
+				if v, ok := lhs.Get(key); ok {
+					v.Recycle()
 					return true
 				}
 			}
diff --git a/typed/parser.go b/typed/parser.go
@@ -99,13 +99,13 @@ func (p ParseableType) FromYAML(object YAMLObject) (*TypedValue, error) {
 	if err != nil {
 		return nil, err
 	}
-	return AsTyped(value.ValueInterface{Value: v}, p.Schema, p.TypeRef)
+	return AsTyped(value.NewValueInterface(v), p.Schema, p.TypeRef)
 }
 
 // FromUnstructured converts a go interface to a TypedValue. It will return an
 // error if the resulting object fails schema validation.
 func (p ParseableType) FromUnstructured(in interface{}) (*TypedValue, error) {
-	return AsTyped(value.ValueInterface{Value: in}, p.Schema, p.TypeRef)
+	return AsTyped(value.NewValueInterface(in), p.Schema, p.TypeRef)
 }
 
 // DeducedParseableType is a ParseableType that deduces the type from
diff --git a/typed/remove.go b/typed/remove.go
@@ -33,7 +33,7 @@ func removeItemsWithSchema(val value.Value, toRemove *fieldpath.Set, schema *sch
 		toRemove: toRemove,
 	}
 	resolveSchema(schema, typeRef, &val, w)
-	return value.ValueInterface{Value: w.out}
+	return value.NewValueInterface(w.out)
 }
 
 func (w *removingWalker) doScalar(t *schema.Scalar) ValidationErrors {
diff --git a/typed/toset_test.go b/typed/toset_test.go
@@ -43,7 +43,7 @@ var (
 	_P   = fieldpath.MakePathOrDie
 	_KBF = fieldpath.KeyByFields
 	_V   = func(i interface{}) *value.Value {
-		v := value.Value(value.ValueInterface{Value: i})
+		v := value.NewValueInterface(i)
 		return &v
 	}
 )
diff --git a/typed/typed.go b/typed/typed.go
@@ -202,7 +202,7 @@ func (tv TypedValue) NormalizeUnionsApply(new *TypedValue) (*TypedValue, error)
 }
 
 func (tv TypedValue) Empty() *TypedValue {
-	tv.value = value.ValueInterface{Value: nil}
+	tv.value = value.NewValueInterface(nil)
 	return &tv
 }
 
@@ -249,7 +249,7 @@ func merge(lhs, rhs *TypedValue, rule, postRule mergeRule) (*TypedValue, error)
 		typeRef: lhs.typeRef,
 	}
 	if mw.out != nil {
-		out.value = value.ValueInterface{Value: *mw.out}
+		out.value = value.NewValueInterface(*mw.out)
 	}
 	return out, nil
 }
diff --git a/typed/union.go b/typed/union.go
@@ -112,7 +112,7 @@ func (d *discriminator) Set(m value.Map, v discriminated) {
 	if d == nil {
 		return
 	}
-	m.Set(d.name, value.ValueInterface{Value: string(v)})
+	m.Set(d.name, value.NewValueInterface(string(v)))
 }
 
 func (d *discriminator) Get(m value.Map) discriminated {
diff --git a/value/less_test.go b/value/less_test.go
@@ -221,7 +221,7 @@ func TestValueLess(t *testing.T) {
 		i := i
 		t.Run(table[i].name, func(t *testing.T) {
 			tt := table[i]
-			a, b := ValueInterface{Value: tt.a}, ValueInterface{Value: tt.b}
+			a, b := NewValueInterface(tt.a), NewValueInterface(tt.b)
 			if tt.eq {
 				if !Equals(a, b) {
 					t.Errorf("oops, a != b: %#v, %#v", tt.a, tt.b)
diff --git a/value/list.go b/value/list.go
@@ -33,7 +33,7 @@ func (l ListInterface) Length() int {
 }
 
 func (l ListInterface) At(i int) Value {
-	return ValueInterface{Value: l[i]}
+	return NewValueInterface(l[i])
 }
 
 // Equals compares two lists lexically.
@@ -44,9 +44,14 @@ func ListEquals(lhs, rhs List) bool {
 
 	for i := 0; i < lhs.Length(); i++ {
 		lv := lhs.At(i)
-		if !Equals(lv, rhs.At(i)) {
+		rv := rhs.At(i)
+		if !Equals(lv, rv) {
+			lv.Recycle()
+			rv.Recycle()
 			return false
 		}
+		lv.Recycle()
+		rv.Recycle()
 	}
 	return true
 }
@@ -73,9 +78,13 @@ func ListCompare(lhs, rhs List) int {
 			// RHS is shorter.
 			return 1
 		}
-		if c := Compare(lhs.At(i), rhs.At(i)); c != 0 {
+		lv := lhs.At(i)
+		rv := rhs.At(i)
+		if c := Compare(lv, rv); c != 0 {
 			return c
 		}
+		lv.Recycle()
+		rv.Recycle()
 		// The items are equal; continue.
 		i++
 	}
diff --git a/value/map.go b/value/map.go
@@ -72,6 +72,8 @@ func MapCompare(lhs, rhs Map) int {
 		if c := Compare(litem, ritem); c != 0 {
 			return c
 		}
+		litem.Recycle()
+		ritem.Recycle()
 		// The items are equal; continue.
 		i++
 	}
@@ -87,7 +89,7 @@ func (m MapInterface) Get(key string) (Value, bool) {
 	if v, ok := m[key]; !ok {
 		return nil, false
 	} else {
-		return ValueInterface{Value: v}, true
+		return NewValueInterface(v), true
 	}
 }
 
@@ -100,9 +102,12 @@ func (m MapInterface) Iterate(fn func(key string, value Value) bool) bool {
 		if ks, ok := k.(string); !ok {
 			continue
 		} else {
-			if !fn(ks, ValueInterface{Value: v}) {
+			vv := NewValueInterface(v)
+			if !fn(ks, vv) {
+				vv.Recycle()
 				return false
 			}
+			vv.Recycle()
 		}
 	}
 	return true
@@ -113,6 +118,9 @@ func (m MapInterface) Length() int {
 }
 
 func (m MapInterface) Equals(other Map) bool {
+	if m.Length() != other.Length() {
+		return false
+	}
 	for k, v := range m {
 		ks, ok := k.(string)
 		if !ok {
@@ -122,9 +130,14 @@ func (m MapInterface) Equals(other Map) bool {
 		if !ok {
 			return false
 		}
-		if !Equals(ValueInterface{Value: v}, vo) {
+		vv := NewValueInterface(v)
+		if !Equals(vv, vo) {
+			vv.Recycle()
+			vo.Recycle()
 			return false
 		}
+		vo.Recycle()
+		vv.Recycle()
 	}
 	return true
 }
@@ -139,7 +152,7 @@ func (m MapString) Get(key string) (Value, bool) {
 	if v, ok := m[key]; !ok {
 		return nil, false
 	} else {
-		return ValueInterface{Value: v}, true
+		return NewValueInterface(v), true
 	}
 }
 
@@ -149,9 +162,12 @@ func (m MapString) Delete(key string) {
 
 func (m MapString) Iterate(fn func(key string, value Value) bool) bool {
 	for k, v := range m {
-		if !fn(k, ValueInterface{v}) {
+		vv := NewValueInterface(v)
+		if !fn(k, vv) {
+			vv.Recycle()
 			return false
 		}
+		vv.Recycle()
 	}
 	return true
 }
@@ -161,14 +177,22 @@ func (m MapString) Length() int {
 }
 
 func (m MapString) Equals(other Map) bool {
+	if m.Length() != other.Length() {
+		return false
+	}
 	for k, v := range m {
 		vo, ok := other.Get(k)
 		if !ok {
 			return false
 		}
-		if !Equals(ValueInterface{Value: v}, vo) {
+		vv := NewValueInterface(v)
+		if !Equals(vv, vo) {
+			vo.Recycle()
+			vv.Recycle()
 			return false
 		}
+		vo.Recycle()
+		vv.Recycle()
 	}
 	return true
 }
diff --git a/value/value.go b/value/value.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io"
 	"strings"
+	"sync"
 
 	jsoniter "github.com/json-iterator/go"
 )
@@ -60,7 +61,7 @@ func ReadJSONIter(iter *jsoniter.Iterator) (Value, error) {
 	if iter.Error != nil && iter.Error != io.EOF {
 		return nil, iter.Error
 	}
-	return ValueInterface{Value: v}, nil
+	return NewValueInterface(v), nil
 }
 
 func WriteJSONStream(v Value, stream *jsoniter.Stream) {
@@ -83,10 +84,26 @@ type Value interface {
 	Float() float64
 	String() string
 
+	// Returns a value of this type that is no longer needed. The
+	// value shouldn't be used after this call.
+	Recycle()
+
 	Copy() Value
 	Interface() interface{}
 }
 
+var viPool = sync.Pool{
+	New: func() interface{} {
+		return &ValueInterface{}
+	},
+}
+
+func NewValueInterface(v interface{}) Value {
+	vi := viPool.Get().(*ValueInterface)
+	vi.Value = v
+	return Value(vi)
+}
+
 type ValueInterface struct {
 	Value interface{}
 }
@@ -202,25 +219,29 @@ func (v ValueInterface) IsNull() bool {
 	return v.Value == nil
 }
 
+func (v *ValueInterface) Recycle() {
+	viPool.Put(v)
+}
+
 func (v ValueInterface) Interface() interface{} {
 	return v.Value
 }
 
-func (v ValueInterface) Copy() Value {
+func (v *ValueInterface) Copy() Value {
 	if v.IsList() {
 		l := make([]interface{}, 0, v.List().Length())
 		for i := 0; i < v.List().Length(); i++ {
 			l = append(l, v.List().At(i).Copy().Interface())
 		}
-		return ValueInterface{Value: l}
+		return NewValueInterface(l)
 	}
 	if v.IsMap() {
 		m := make(map[string]interface{}, v.Map().Length())
 		v.Map().Iterate(func(key string, item Value) bool {
 			m[key] = item.Copy().Interface()
 			return true
 		})
-		return ValueInterface{Value: m}
+		return NewValueInterface(m)
 	}
 	// Scalars don't have to be copied
 	return v

Original file line number	Diff line number	Diff line change
`@@ -127,7 +127,7 @@ func KeyByFields(nameValues ...interface{}) *value.FieldList {`
`127`	`127`	`}`
`128`	`128`	`out := value.FieldList{}`
`129`	`129`	`for i := 0; i < len(nameValues)-1; i += 2 {`
`130`		`- out = append(out, value.Field{Name: nameValues[i].(string), Value: value.ValueInterface{Value: nameValues[i+1]}})`
	`130`	`+ out = append(out, value.Field{Name: nameValues[i].(string), Value: value.NewValueInterface(nameValues[i+1])})`
`131`	`131`	`}`
`132`	`132`	`out.Sort()`
`133`	`133`	`return &out`
Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ func TestPathElementSet(t *testing.T) {`
`65`	`65`	`func strptr(s string) *string { return &s }`
`66`	`66`	`func intptr(i int) *int { return &i }`
`67`	`67`	`func valptr(i interface{}) *value.Value {`
`68`		`- v := value.Value(value.ValueInterface{Value: i})`
	`68`	`+ v := value.NewValueInterface(i)`
`69`	`69`	`return &v`
`70`	`70`	`}`
`71`	`71`
Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ func TestFromValue(t *testing.T) {`
`71`	`71`	`if err != nil {`
`72`	`72`	`t.Fatalf("couldn't parse: %v", err)`
`73`	`73`	`}`
`74`		`- got := SetFromValue(value.ValueInterface{Value: v})`
	`74`	`+ got := SetFromValue(value.NewValueInterface(v))`
`75`	`75`	`if !got.Equals(tt.set) {`
`76`	`76`	`t.Errorf("wanted\n%s\nbut got\n%s\n", tt.set, got)`
`77`	`77`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ import (`
`24`	`24`
`25`	`25`	`var (`
`26`	`26`	`_V = func(i interface{}) *value.Value {`
`27`		`- v := value.Value(value.ValueInterface{Value: i})`
	`27`	`+ v := value.NewValueInterface(i)`
`28`	`28`	`return &v`
`29`	`29`	`}`
`30`	`30`	`)`
Original file line number	Diff line number	Diff line change
`@@ -29,22 +29,22 @@ func TestPathElementValueMap(t *testing.T) {`
`29`	`29`	`t.Fatal("Unexpected path-element found in empty map")`
`30`	`30`	`}`
`31`	`31`
`32`		`- m.Insert(PathElement{FieldName: strptr("carrot")}, value.ValueInterface{Value: "knife"})`
`33`		`- m.Insert(PathElement{FieldName: strptr("chive")}, value.ValueInterface{Value: 2})`
	`32`	`+ m.Insert(PathElement{FieldName: strptr("carrot")}, value.NewValueInterface("knife"))`
	`33`	`+ m.Insert(PathElement{FieldName: strptr("chive")}, value.NewValueInterface(2))`
`34`	`34`
`35`	`35`	`if _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {`
`36`	`36`	`t.Fatal("Unexpected path-element in map")`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`if val, ok := m.Get(PathElement{FieldName: strptr("carrot")}); !ok {`
`40`	`40`	`t.Fatal("Missing path-element in map")`
`41`		`- } else if !value.Equals(val, value.ValueInterface{Value: "knife"}) {`
	`41`	`+ } else if !value.Equals(val, value.NewValueInterface("knife")) {`
`42`	`42`	`t.Fatalf("Unexpected value found: %#v", val)`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`if val, ok := m.Get(PathElement{FieldName: strptr("chive")}); !ok {`
`46`	`46`	`t.Fatal("Missing path-element in map")`
`47`		`- } else if !value.Equals(val, value.ValueInterface{Value: 2}) {`
	`47`	`+ } else if !value.Equals(val, value.NewValueInterface(2)) {`
`48`	`48`	`t.Fatalf("Unexpected value found: %#v", val)`
`49`	`49`	`}`
`50`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ var (`
`30`	`30`	`_P = fieldpath.MakePathOrDie`
`31`	`31`	`_KBF = fieldpath.KeyByFields`
`32`	`32`	`_V = func(i interface{}) *value.Value {`
`33`		`- v := value.Value(value.ValueInterface{Value: i})`
	`33`	`+ v := value.NewValueInterface(i)`
`34`	`34`	`return &v`
`35`	`35`	`}`
`36`	`36`	`)`
Original file line number	Diff line number	Diff line change
`@@ -321,6 +321,7 @@ func (w mergingWalker) visitMapItems(t schema.Map, lhs, rhs value.Map) (errs V`
`321`	`321`	`if rhs != nil {`
`322`	`322`	`if item, ok := rhs.Get(key); ok {`
`323`	`323`	`rval = &item`
	`324`	`+ defer (*rval).Recycle()`
`324`	`325`	`}`
`325`	`326`	`}`
`326`	`327`	`errs = append(errs, w.visitMapItem(t, out, key, &val, rval)...)`
`@@ -331,7 +332,8 @@ func (w mergingWalker) visitMapItems(t schema.Map, lhs, rhs value.Map) (errs V`
`331`	`332`	`if rhs != nil {`
`332`	`333`	`rhs.Iterate(func(key string, val value.Value) bool {`
`333`	`334`	`if lhs != nil {`
`334`		`- if _, ok := lhs.Get(key); ok {`
	`335`	`+ if v, ok := lhs.Get(key); ok {`
	`336`	`+ v.Recycle()`
`335`	`337`	`return true`
`336`	`338`	`}`
`337`	`339`	`}`
Original file line number	Diff line number	Diff line change
`@@ -99,13 +99,13 @@ func (p ParseableType) FromYAML(object YAMLObject) (*TypedValue, error) {`
`99`	`99`	`if err != nil {`
`100`	`100`	`return nil, err`
`101`	`101`	`}`
`102`		`- return AsTyped(value.ValueInterface{Value: v}, p.Schema, p.TypeRef)`
	`102`	`+ return AsTyped(value.NewValueInterface(v), p.Schema, p.TypeRef)`
`103`	`103`	`}`
`104`	`104`
`105`	`105`	`// FromUnstructured converts a go interface to a TypedValue. It will return an`
`106`	`106`	`// error if the resulting object fails schema validation.`
`107`	`107`	`func (p ParseableType) FromUnstructured(in interface{}) (*TypedValue, error) {`
`108`		`- return AsTyped(value.ValueInterface{Value: in}, p.Schema, p.TypeRef)`
	`108`	`+ return AsTyped(value.NewValueInterface(in), p.Schema, p.TypeRef)`
`109`	`109`	`}`
`110`	`110`
`111`	`111`	`// DeducedParseableType is a ParseableType that deduces the type from`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ func removeItemsWithSchema(val value.Value, toRemove fieldpath.Set, schema sch`
`33`	`33`	`toRemove: toRemove,`
`34`	`34`	`}`
`35`	`35`	`resolveSchema(schema, typeRef, &val, w)`
`36`		`- return value.ValueInterface{Value: w.out}`
	`36`	`+ return value.NewValueInterface(w.out)`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`func (w removingWalker) doScalar(t schema.Scalar) ValidationErrors {`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ var (`
`43`	`43`	`_P = fieldpath.MakePathOrDie`
`44`	`44`	`_KBF = fieldpath.KeyByFields`
`45`	`45`	`_V = func(i interface{}) *value.Value {`
`46`		`- v := value.Value(value.ValueInterface{Value: i})`
	`46`	`+ v := value.NewValueInterface(i)`
`47`	`47`	`return &v`
`48`	`48`	`}`
`49`	`49`	`)`