Merge pull request #153 from jpbetz/compare-optimizations

Optimize TypedValue.Compare for value reflector

Author: k8s-ci-robot

fieldpath/fromvalue.go

@@ -55,7 +55,9 @@ func (w *objectWalker) walk() {
 	case w.value.IsList():
 		// If the list were atomic, we'd break here, but we don't have
 		// a schema, so we can't tell.
-		iter := w.value.AsList().Range()
+		l := w.value.AsList()
+		defer l.Recycle()
+		iter := l.Range()
 		defer iter.Recycle()
 		for iter.Next() {
 			i, value := iter.Item()
@@ -69,7 +71,9 @@ func (w *objectWalker) walk() {
 		// If the map/struct were atomic, we'd break here, but we don't
 		// have a schema, so we can't tell.
 
-		w.value.AsMap().Iterate(func(k string, val value.Value) bool {
+		m := w.value.AsMap()
+		defer m.Recycle()
+		m.Iterate(func(k string, val value.Value) bool {
 			w2 := *w
 			w2.path = append(w.path, PathElement{FieldName: &k})
 			w2.value = val
@@ -106,9 +110,11 @@ func GuessBestListPathElement(index int, item value.Value) PathElement {
 		return PathElement{Index: &index}
 	}
 
+	m := item.AsMap()
+	defer m.Recycle()
 	var keys value.FieldList
 	for _, name := range AssociativeListCandidateFieldNames {
-		f, ok := item.AsMap().Get(name)
+		f, ok := m.Get(name)
 		if !ok {
 			continue
 		}

typed/helpers.go

@@ -191,8 +191,10 @@ func keyedAssociativeListItemToPathElement(list *schema.List, index int, child v
 		return pe, errors.New("associative list with keys may not have non-map elements")
 	}
 	keyMap := value.FieldList{}
+	m := child.AsMap()
+	defer m.Recycle()
 	for _, fieldName := range list.Keys {
-		if val, ok := child.AsMap().Get(fieldName); ok {
+		if val, ok := m.Get(fieldName); ok {
 			keyMap = append(keyMap, value.Field{Name: fieldName, Value: val})
 		} else {
 			return pe, fmt.Errorf("associative list with keys has an element that omits key field %q", fieldName)

typed/merge.go

@@ -148,18 +148,15 @@ func (w *mergingWalker) finishDescent(w2 *mergingWalker) {
 	*w.spareWalkers = append(*w.spareWalkers, w2)
 }
 
-func (w *mergingWalker) derefMap(prefix string, v value.Value, dest *value.Map) (errs ValidationErrors) {
-	// taking dest as input so that it can be called as a one-liner with
-	// append.
+func (w *mergingWalker) derefMap(prefix string, v value.Value) (value.Map, ValidationErrors) {
 	if v == nil {
-		return nil
+		return nil, nil
 	}
 	m, err := mapValue(v)
 	if err != nil {
-		return errorf("%v: %v", prefix, err)
+		return nil, errorf("%v: %v", prefix, err)
 	}
-	*dest = m
-	return nil
+	return m, nil
 }
 
 func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (errs ValidationErrors) {
@@ -253,24 +250,26 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs value.List) (err
 	return errs
 }
 
-func (w *mergingWalker) derefList(prefix string, v value.Value, dest *value.List) (errs ValidationErrors) {
-	// taking dest as input so that it can be called as a one-liner with
-	// append.
+func (w *mergingWalker) derefList(prefix string, v value.Value) (value.List, ValidationErrors) {
 	if v == nil {
-		return nil
+		return nil, nil
 	}
 	l, err := listValue(v)
 	if err != nil {
-		return errorf("%v: %v", prefix, err)
+		return nil, errorf("%v: %v", prefix, err)
 	}
-	*dest = l
-	return nil
+	return l, nil
 }
 
 func (w *mergingWalker) doList(t *schema.List) (errs ValidationErrors) {
-	var lhs, rhs value.List
-	w.derefList("lhs: ", w.lhs, &lhs)
-	w.derefList("rhs: ", w.rhs, &rhs)
+	lhs, _ := w.derefList("lhs: ", w.lhs)
+	if lhs != nil {
+		defer lhs.Recycle()
+	}
+	rhs, _ := w.derefList("rhs: ", w.rhs)
+	if rhs != nil {
+		defer rhs.Recycle()
+	}
 
 	// If both lhs and rhs are empty/null, treat it as a
 	// leaf: this helps preserve the empty/null
@@ -311,31 +310,10 @@ func (w *mergingWalker) visitMapItem(t *schema.Map, out map[string]interface{},
 func (w *mergingWalker) visitMapItems(t *schema.Map, lhs, rhs value.Map) (errs ValidationErrors) {
 	out := map[string]interface{}{}
 
-	if lhs != nil {
-		lhs.Iterate(func(key string, val value.Value) bool {
-			var rval value.Value
-			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)...)
-			return true
-		})
-	}
-
-	if rhs != nil {
-		rhs.Iterate(func(key string, val value.Value) bool {
-			if lhs != nil {
-				if lhs.Has(key) {
-					return true
-				}
-			}
-			errs = append(errs, w.visitMapItem(t, out, key, nil, val)...)
-			return true
-		})
-	}
+	value.MapZip(lhs, rhs, value.Unordered, func(key string, lhsValue, rhsValue value.Value) bool {
+		errs = append(errs, w.visitMapItem(t, out, key, lhsValue, rhsValue)...)
+		return true
+	})
 	if len(out) > 0 {
 		i := interface{}(out)
 		w.out = &i
@@ -345,14 +323,18 @@ func (w *mergingWalker) visitMapItems(t *schema.Map, lhs, rhs value.Map) (errs V
 }
 
 func (w *mergingWalker) doMap(t *schema.Map) (errs ValidationErrors) {
-	var lhs, rhs value.Map
-	w.derefMap("lhs: ", w.lhs, &lhs)
-	w.derefMap("rhs: ", w.rhs, &rhs)
-
+	lhs, _ := w.derefMap("lhs: ", w.lhs)
+	if lhs != nil {
+		defer lhs.Recycle()
+	}
+	rhs, _ := w.derefMap("rhs: ", w.rhs)
+	if rhs != nil {
+		defer rhs.Recycle()
+	}
 	// If both lhs and rhs are empty/null, treat it as a
 	// leaf: this helps preserve the empty/null
 	// distinction.
-	emptyPromoteToLeaf := (lhs == nil || lhs.Length() == 0) && (rhs == nil || rhs.Length() == 0)
+	emptyPromoteToLeaf := (lhs == nil || lhs.Empty()) && (rhs == nil || rhs.Empty())
 
 	if t.ElementRelationship == schema.Atomic || emptyPromoteToLeaf {
 		w.doLeaf()

typed/remove.go

@@ -43,7 +43,7 @@ func (w *removingWalker) doScalar(t *schema.Scalar) ValidationErrors {
 
 func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) {
 	l := w.value.AsList()
-
+	defer l.Recycle()
 	// If list is null, empty, or atomic just return
 	if l == nil || l.Length() == 0 || t.ElementRelationship == schema.Atomic {
 		return nil
@@ -73,9 +73,11 @@ func (w *removingWalker) doList(t *schema.List) (errs ValidationErrors) {
 
 func (w *removingWalker) doMap(t *schema.Map) ValidationErrors {
 	m := w.value.AsMap()
-
+	if m != nil {
+		defer m.Recycle()
+	}
 	// If map is null, empty, or atomic just return
-	if m == nil || m.Length() == 0 || t.ElementRelationship == schema.Atomic {
+	if m == nil || m.Empty() || t.ElementRelationship == schema.Atomic {
 		return nil
 	}
 

typed/tofieldset.go

@@ -144,7 +144,9 @@ func (v *toFieldSetWalker) visitMapItems(t *schema.Map, m value.Map) (errs Valid
 
 func (v *toFieldSetWalker) doMap(t *schema.Map) (errs ValidationErrors) {
 	m, _ := mapValue(v.value)
-
+	if m != nil {
+		defer m.Recycle()
+	}
 	if t.ElementRelationship == schema.Atomic {
 		v.set.Insert(v.path)
 		return nil

typed/validate.go

@@ -179,11 +179,10 @@ func (v *validatingObjectWalker) doMap(t *schema.Map) (errs ValidationErrors) {
 	if err != nil {
 		return errorf(err.Error())
 	}
-
 	if m == nil {
 		return nil
 	}
-
+	defer m.Recycle()
 	errs = v.visitMapItems(t, m)
 
 	return errs

value/list.go

@@ -25,6 +25,13 @@ type List interface {
 	At(int) Value
 	// Range returns a ListRange for iterating over the items in the list.
 	Range() ListRange
+
+	// Equals compares the two list, and return true if they are the same, false otherwise.
+	// Implementations can use ListEquals as a general implementation for this methods.
+	Equals(List) bool
+	// Recycle gives back this List once it is no longer needed. The
+	// value shouldn't be used after this call.
+	Recycle()
 }
 
 // ListRange represents a single iteration across the items of a list.
@@ -36,7 +43,7 @@ type ListRange interface {
 	// pointers to the value returned by Item() that escape the iteration loop since they become invalid once either
 	// Item() or Recycle() is called.
 	Item() (index int, value Value)
-	// Recycle returns a ListRange that is no longer needed. The value returned by Item() becomes invalid once this is
+	// Recycle gives back this ListRange once it is no longer needed. The value returned by Item() becomes invalid once this is
 	// called.
 	Recycle()
 }
@@ -56,6 +63,7 @@ func (_ *emptyRange) Item() (index int, value Value) {
 func (_ *emptyRange) Recycle() {}
 
 // ListEquals compares two lists lexically.
+// WARN: This is a naive implementation, calling lhs.Equals(rhs) is typically the most efficient.
 func ListEquals(lhs, rhs List) bool {
 	if lhs.Length() != rhs.Length() {
 		return false

value/listreflect.go

@@ -50,6 +50,10 @@ var lrrPool = sync.Pool{
 	},
 }
 
+func (r *listReflect) Recycle() {
+	listReflectPool.Put(r)
+}
+
 func (r listReflect) Range() ListRange {
 	length := r.Value.Len()
 	if length == 0 {
@@ -58,10 +62,17 @@ func (r listReflect) Range() ListRange {
 	rr := lrrPool.Get().(*listReflectRange)
 	rr.list = r.Value
 	rr.i = -1
-	rr.entry = nil
+	rr.entry = TypeReflectEntryOf(r.Value.Type().Elem())
 	return rr
 }
 
+func (r listReflect) Equals(other List) bool {
+	if otherReflectList, ok := other.(*listReflect); ok {
+		return reflect.DeepEqual(r.Value.Interface(), otherReflectList.Value.Interface())
+	}
+	return ListEquals(&r, other)
+}
+
 type listReflectRange struct {
 	list  reflect.Value
 	vr    *valueReflect
@@ -82,9 +93,6 @@ func (r *listReflectRange) Item() (index int, value Value) {
 		panic("Item() called on ListRange with no more items")
 	}
 	v := r.list.Index(r.i)
-	if r.entry == nil {
-		r.entry = TypeReflectEntryOf(v.Type())
-	}
 	return r.i, r.vr.mustReuse(v, r.entry, nil, nil)
 }
 

value/listunstructured.go

@@ -28,12 +28,20 @@ func (l listUnstructured) At(i int) Value {
 	return NewValueInterface(l[i])
 }
 
+func (l listUnstructured) Equals(other List) bool {
+	return ListEquals(&l, other)
+}
+
 var lurPool = sync.Pool{
 	New: func() interface{} {
 		return &listUnstructuredRange{vv: &valueUnstructured{}}
 	},
 }
 
+func (_ listUnstructured) Recycle() {
+
+}
+
 func (l listUnstructured) Range() ListRange {
 	if len(l) == 0 {
 		return EmptyRange