optimize mapUnstructured.Equals, apply feedback

Author: jpbetz

value/list.go

@@ -41,6 +41,20 @@ type ListRange interface {
 	Recycle()
 }
 
+var EmptyRange = &emptyRange{}
+
+type emptyRange struct{}
+
+func (_ *emptyRange) Next() bool {
+	return false
+}
+
+func (_ *emptyRange) Item() (index int, value Value) {
+	panic("Item called on empty ListRange")
+}
+
+func (_ *emptyRange) Recycle() {}
+
 // ListEquals compares two lists lexically.
 func ListEquals(lhs, rhs List) bool {
 	if lhs.Length() != rhs.Length() {

value/listreflect.go

@@ -16,7 +16,10 @@ limitations under the License.
 
 package value
 
-import "reflect"
+import (
+	"reflect"
+	"sync"
+)
 
 type listReflect struct {
 	Value reflect.Value
@@ -41,32 +44,44 @@ func (r listReflect) Unstructured() interface{} {
 	return result
 }
 
+var lrrPool = sync.Pool{
+	New: func() interface{} {
+		return &listReflectRange{vr: &valueReflect{}}
+	},
+}
+
 func (r listReflect) Range() ListRange {
-	return &listReflectRange{r.Value, newTempValuePooler(), -1, r.Value.Len()}
+	length := r.Value.Len()
+	if length == 0 {
+		return EmptyRange
+	}
+	rr := lrrPool.Get().(*listReflectRange)
+	rr.list = r.Value
+	rr.i = -1
+	return rr
 }
 
 type listReflectRange struct {
-	val    reflect.Value
-	pooler *tempValuePooler
-	i      int
-	length int
+	list reflect.Value
+	vr   *valueReflect
+	i    int
 }
 
 func (r *listReflectRange) Next() bool {
 	r.i += 1
-	return r.i < r.length
+	return r.i < r.list.Len()
 }
 
 func (r *listReflectRange) Item() (index int, value Value) {
 	if r.i < 0 {
 		panic("Item() called before first calling Next()")
 	}
-	if r.i >= r.length {
+	if r.i >= r.list.Len() {
 		panic("Item() called on ListRange with no more items")
 	}
-	return r.i, r.pooler.NewValueReflect(r.val.Index(r.i))
+	return r.i, r.vr.reuse(r.list.Index(r.i))
 }
 
 func (r *listReflectRange) Recycle() {
-	r.pooler.Recycle()
+	lrrPool.Put(r)
 }

value/listunstructured.go

@@ -16,6 +16,8 @@ limitations under the License.
 
 package value
 
+import "sync"
+
 type listUnstructured []interface{}
 
 func (l listUnstructured) Length() int {
@@ -26,40 +28,43 @@ func (l listUnstructured) At(i int) Value {
 	return NewValueInterface(l[i])
 }
 
+var lurPool = sync.Pool{
+	New: func() interface{} {
+		return &listUnstructuredRange{vv: &valueUnstructured{}}
+	},
+}
+
 func (l listUnstructured) Range() ListRange {
 	if len(l) == 0 {
-		return &listUnstructuredRange{l, nil, -1, 0}
+		return EmptyRange
 	}
-	vv := viPool.Get().(*valueUnstructured)
-	return &listUnstructuredRange{l, vv, -1, len(l)}
+	r := lurPool.Get().(*listUnstructuredRange)
+	r.list = l
+	r.i = -1
+	return r
 }
 
 type listUnstructuredRange struct {
-	list   listUnstructured
-	vv     *valueUnstructured
-	i      int
-	length int
+	list listUnstructured
+	vv   *valueUnstructured
+	i    int
 }
 
 func (r *listUnstructuredRange) Next() bool {
 	r.i += 1
-	return r.i < r.length
+	return r.i < len(r.list)
 }
 
 func (r *listUnstructuredRange) Item() (index int, value Value) {
 	if r.i < 0 {
 		panic("Item() called before first calling Next()")
 	}
-	if r.i >= r.length {
+	if r.i >= len(r.list) {
 		panic("Item() called on ListRange with no more items")
 	}
-
-	r.vv.Value = r.list[r.i]
-	return r.i, r.vv
+	return r.i, r.vv.reuse(r.list[r.i])
 }
 
 func (r *listUnstructuredRange) Recycle() {
-	if r.vv != nil {
-		r.vv.Recycle()
-	}
+	lurPool.Put(r)
 }

value/map.go

@@ -111,9 +111,6 @@ func MapEquals(lhs, rhs Map) bool {
 		}
 		equal := Equals(v, vo)
 		vo.Recycle()
-		if !equal {
-			return false
-		}
-		return true
+		return equal
 	})
 }

value/mapreflect.go

@@ -68,10 +68,10 @@ func (r mapReflect) toMapKey(key string) reflect.Value {
 }
 
 func (r mapReflect) Iterate(fn func(string, Value) bool) bool {
-	vp := newTempValuePooler()
-	defer vp.Recycle()
+	vr := reflectPool.Get().(*valueReflect)
+	defer vr.Recycle()
 	return eachMapEntry(r.Value, func(s string, value reflect.Value) bool {
-		return fn(s, vp.NewValueReflect(value))
+		return fn(s, vr.reuse(value))
 	})
 }
 
@@ -104,16 +104,16 @@ func (r mapReflect) Equals(m Map) bool {
 	if lhsLength != rhsLength {
 		return false
 	}
-	if lhsLength == 0 && rhsLength == 0 {
+	if lhsLength == 0 {
 		return true
 	}
-	vp := newTempValuePooler()
-	defer vp.Recycle()
+	vr := reflectPool.Get().(*valueReflect)
+	defer vr.Recycle()
 	return m.Iterate(func(key string, value Value) bool {
 		_, lhsVal, ok := r.get(key)
 		if !ok {
 			return false
 		}
-		return Equals(vp.NewValueReflect(lhsVal), value)
+		return Equals(vr.reuse(lhsVal), value)
 	})
 }

value/mapunstructured.go

@@ -49,8 +49,7 @@ func (m mapUnstructuredInterface) Iterate(fn func(key string, value Value) bool)
 		if ks, ok := k.(string); !ok {
 			continue
 		} else {
-			vv.Value = v
-			if !fn(ks, vv) {
+			if !fn(ks, vv.reuse(v)) {
 				return false
 			}
 		}
@@ -63,31 +62,23 @@ func (m mapUnstructuredInterface) Length() int {
 }
 
 func (m mapUnstructuredInterface) Equals(other Map) bool {
-	if m.Length() != other.Length() {
+	lhsLength := m.Length()
+	rhsLength := other.Length()
+	if lhsLength != rhsLength {
 		return false
 	}
-	if len(m) == 0 {
+	if lhsLength == 0 {
 		return true
 	}
 	vv := viPool.Get().(*valueUnstructured)
 	defer vv.Recycle()
-	for k, v := range m {
-		ks, ok := k.(string)
-		if !ok {
-			return false
-		}
-		vo, ok := other.Get(ks)
+	return other.Iterate(func(key string, value Value) bool {
+		lhsVal, ok := m[key]
 		if !ok {
 			return false
 		}
-		vv.Value = v
-		if !Equals(vv, vo) {
-			vo.Recycle()
-			return false
-		}
-		vo.Recycle()
-	}
-	return true
+		return Equals(vv.reuse(lhsVal), value)
+	})
 }
 
 type mapUnstructuredString map[string]interface{}
@@ -120,8 +111,7 @@ func (m mapUnstructuredString) Iterate(fn func(key string, value Value) bool) bo
 	vv := viPool.Get().(*valueUnstructured)
 	defer vv.Recycle()
 	for k, v := range m {
-		vv.Value = v
-		if !fn(k, vv) {
+		if !fn(k, vv.reuse(v)) {
 			return false
 		}
 	}
@@ -133,25 +123,21 @@ func (m mapUnstructuredString) Length() int {
 }
 
 func (m mapUnstructuredString) Equals(other Map) bool {
-	if m.Length() != other.Length() {
+	lhsLength := m.Length()
+	rhsLength := other.Length()
+	if lhsLength != rhsLength {
 		return false
 	}
-	if len(m) == 0 {
+	if lhsLength == 0 {
 		return true
 	}
 	vv := viPool.Get().(*valueUnstructured)
 	defer vv.Recycle()
-	for k, v := range m {
-		vo, ok := other.Get(k)
+	return other.Iterate(func(key string, value Value) bool {
+		lhsVal, ok := m[key]
 		if !ok {
 			return false
 		}
-		vv.Value = v
-		if !Equals(vv, vo) {
-			vo.Recycle()
-			return false
-		}
-		vo.Recycle()
-	}
-	return true
+		return Equals(vv.reuse(lhsVal), value)
+	})
 }

value/structreflect.go

@@ -198,10 +198,10 @@ func (r structReflect) update(fieldEntry *fieldCacheEntry, key string, oldVal, n
 }
 
 func (r structReflect) Iterate(fn func(string, Value) bool) bool {
-	vp := newTempValuePooler()
-	defer vp.Recycle()
+	vr := reflectPool.Get().(*valueReflect)
+	defer vr.Recycle()
 	return eachStructField(r.Value, func(s string, value reflect.Value) bool {
-		return fn(s, vp.NewValueReflect(value))
+		return fn(s, vr.reuse(value))
 	})
 }
 
@@ -234,20 +234,25 @@ func (r structReflect) Equals(m Map) bool {
 	if rhsStruct, ok := m.(structReflect); ok {
 		return reflect.DeepEqual(r.Value.Interface(), rhsStruct.Value.Interface())
 	}
-	if r.Length() != m.Length() {
+	length := r.Length()
+	if length != m.Length() {
 		return false
 	}
-	structCacheEntry := getStructCacheEntry(r.Value.Type())
 
-	vp := newTempValuePooler()
-	defer vp.Recycle()
+	if length == 0 {
+		return true
+	}
+
+	structCacheEntry := getStructCacheEntry(r.Value.Type())
+	vr := reflectPool.Get().(*valueReflect)
+	defer vr.Recycle()
 	return m.Iterate(func(s string, value Value) bool {
 		fieldCacheEntry, ok := structCacheEntry[s]
 		if !ok {
 			return false
 		}
 		lhsVal := fieldCacheEntry.getFieldFromStruct(r.Value)
-		return Equals(vp.NewValueReflect(lhsVal), value)
+		return Equals(vr.reuse(lhsVal), value)
 	})
 }