Merge pull request #114 from apelisse/perf/pathelement-set

Use PathElementSet and similar to index PathElement

Author: k8s-ci-robot

fieldpath/element.go

@@ -143,6 +143,12 @@ type PathElementSet struct {
 	members sortedPathElements
 }
 
+func MakePathElementSet(size int) PathElementSet {
+	return PathElementSet{
+		members: make(sortedPathElements, 0, size),
+	}
+}
+
 type sortedPathElements []PathElement
 
 // Implement the sort interface; this would permit bulk creation, which would

fieldpath/pathelementmap.go

@@ -0,0 +1,85 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fieldpath
+
+import (
+	"sort"
+
+	"sigs.k8s.io/structured-merge-diff/value"
+)
+
+// PathElementValueMap is a map from PathElement to value.Value.
+//
+// TODO(apelisse): We have multiple very similar implementation of this
+// for PathElementSet and SetNodeMap, so we could probably share the
+// code.
+type PathElementValueMap struct {
+	members sortedPathElementValues
+}
+
+func MakePathElementValueMap(size int) PathElementValueMap {
+	return PathElementValueMap{
+		members: make(sortedPathElementValues, 0, size),
+	}
+}
+
+type pathElementValue struct {
+	PathElement PathElement
+	Value       value.Value
+}
+
+type sortedPathElementValues []pathElementValue
+
+// Implement the sort interface; this would permit bulk creation, which would
+// be faster than doing it one at a time via Insert.
+func (spev sortedPathElementValues) Len() int { return len(spev) }
+func (spev sortedPathElementValues) Less(i, j int) bool {
+	return spev[i].PathElement.Less(spev[j].PathElement)
+}
+func (spev sortedPathElementValues) Swap(i, j int) { spev[i], spev[j] = spev[j], spev[i] }
+
+// Insert adds the pathelement and associated value in the map.
+func (s *PathElementValueMap) Insert(pe PathElement, v value.Value) {
+	loc := sort.Search(len(s.members), func(i int) bool {
+		return !s.members[i].PathElement.Less(pe)
+	})
+	if loc == len(s.members) {
+		s.members = append(s.members, pathElementValue{pe, v})
+		return
+	}
+	if s.members[loc].PathElement.Equals(pe) {
+		return
+	}
+	s.members = append(s.members, pathElementValue{})
+	copy(s.members[loc+1:], s.members[loc:])
+	s.members[loc] = pathElementValue{pe, v}
+}
+
+// Get retrieves the value associated with the given PathElement from the map.
+// (nil, false) is returned if there is no such PathElement.
+func (s *PathElementValueMap) Get(pe PathElement) (value.Value, bool) {
+	loc := sort.Search(len(s.members), func(i int) bool {
+		return !s.members[i].PathElement.Less(pe)
+	})
+	if loc == len(s.members) {
+		return value.Value{}, false
+	}
+	if s.members[loc].PathElement.Equals(pe) {
+		return s.members[loc].Value, true
+	}
+	return value.Value{}, false
+}

fieldpath/pathelementmap_test.go

@@ -0,0 +1,50 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package fieldpath
+
+import (
+	"testing"
+
+	"sigs.k8s.io/structured-merge-diff/value"
+)
+
+func TestPathElementValueMap(t *testing.T) {
+	m := PathElementValueMap{}
+
+	if _, ok := m.Get(PathElement{FieldName: strptr("onion")}); ok {
+		t.Fatal("Unexpected path-element found in empty map")
+	}
+
+	m.Insert(PathElement{FieldName: strptr("carrot")}, value.StringValue("knife"))
+	m.Insert(PathElement{FieldName: strptr("chive")}, value.IntValue(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 !val.Equals(value.StringValue("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 !val.Equals(value.IntValue(2)) {
+		t.Fatalf("Unexpected value found: %#v", val)
+	}
+}

typed/merge.go

@@ -168,8 +168,9 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs *value.List) (er
 	rhsOrder := []fieldpath.PathElement{}
 
 	// First, collect all RHS children.
-	observedRHS := map[string]value.Value{}
+	var observedRHS fieldpath.PathElementValueMap
 	if rhs != nil {
+		observedRHS = fieldpath.MakePathElementValueMap(len(rhs.Items))
 		for i, child := range rhs.Items {
 			pe, err := listItemToPathElement(t, i, child)
 			if err != nil {
@@ -179,18 +180,18 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs *value.List) (er
 				// this element.
 				continue
 			}
-			keyStr := pe.String()
-			if _, found := observedRHS[keyStr]; found {
-				errs = append(errs, w.errorf("rhs: duplicate entries for key %v", keyStr)...)
+			if _, ok := observedRHS.Get(pe); ok {
+				errs = append(errs, w.errorf("rhs: duplicate entries for key %v", pe.String())...)
 			}
-			observedRHS[keyStr] = child
+			observedRHS.Insert(pe, child)
 			rhsOrder = append(rhsOrder, pe)
 		}
 	}
 
 	// Then merge with LHS children.
-	observedLHS := map[string]struct{}{}
+	var observedLHS fieldpath.PathElementSet
 	if lhs != nil {
+		observedLHS = fieldpath.MakePathElementSet(len(lhs.Items))
 		for i, child := range lhs.Items {
 			pe, err := listItemToPathElement(t, i, child)
 			if err != nil {
@@ -200,15 +201,14 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs *value.List) (er
 				// this element.
 				continue
 			}
-			keyStr := pe.String()
-			if _, found := observedLHS[keyStr]; found {
-				errs = append(errs, w.errorf("lhs: duplicate entries for key %v", keyStr)...)
+			if observedLHS.Has(pe) {
+				errs = append(errs, w.errorf("lhs: duplicate entries for key %v", pe.String())...)
 				continue
 			}
-			observedLHS[keyStr] = struct{}{}
+			observedLHS.Insert(pe)
 			w2 := w.prepareDescent(pe, t.ElementType)
 			w2.lhs = &child
-			if rchild, ok := observedRHS[keyStr]; ok {
+			if rchild, ok := observedRHS.Get(pe); ok {
 				w2.rhs = &rchild
 			}
 			if newErrs := w2.merge(); len(newErrs) > 0 {
@@ -217,22 +217,22 @@ func (w *mergingWalker) visitListItems(t *schema.List, lhs, rhs *value.List) (er
 				out.Items = append(out.Items, *w2.out)
 			}
 			w.finishDescent(w2)
-			// Keep track of children that have been handled
-			delete(observedRHS, keyStr)
 		}
 	}
 
-	for _, rhsToCheck := range rhsOrder {
-		if unmergedChild, ok := observedRHS[rhsToCheck.String()]; ok {
-			w2 := w.prepareDescent(rhsToCheck, t.ElementType)
-			w2.rhs = &unmergedChild
-			if newErrs := w2.merge(); len(newErrs) > 0 {
-				errs = append(errs, newErrs...)
-			} else if w2.out != nil {
-				out.Items = append(out.Items, *w2.out)
-			}
-			w.finishDescent(w2)
+	for _, pe := range rhsOrder {
+		if observedLHS.Has(pe) {
+			continue
+		}
+		value, _ := observedRHS.Get(pe)
+		w2 := w.prepareDescent(pe, t.ElementType)
+		w2.rhs = &value
+		if newErrs := w2.merge(); len(newErrs) > 0 {
+			errs = append(errs, newErrs...)
+		} else if w2.out != nil {
+			out.Items = append(out.Items, *w2.out)
 		}
+		w.finishDescent(w2)
 	}
 
 	if len(out.Items) > 0 {

typed/validate.go

@@ -142,7 +142,7 @@ func (v *validatingObjectWalker) doScalar(t *schema.Scalar) ValidationErrors {
 }
 
 func (v *validatingObjectWalker) visitListItems(t *schema.List, list *value.List) (errs ValidationErrors) {
-	observedKeys := map[string]struct{}{}
+	observedKeys := fieldpath.MakePathElementSet(len(list.Items))
 	for i, child := range list.Items {
 		pe, err := listItemToPathElement(t, i, child)
 		if err != nil {
@@ -152,11 +152,10 @@ func (v *validatingObjectWalker) visitListItems(t *schema.List, list *value.List
 			// this element.
 			continue
 		}
-		keyStr := pe.String()
-		if _, found := observedKeys[keyStr]; found {
-			errs = append(errs, v.errorf("duplicate entries for key %v", keyStr)...)
+		if observedKeys.Has(pe) {
+			errs = append(errs, v.errorf("duplicate entries for key %v", pe.String())...)
 		}
-		observedKeys[keyStr] = struct{}{}
+		observedKeys.Insert(pe)
 		v2 := v.prepareDescent(pe, t.ElementType)
 		v2.value = child
 		errs = append(errs, v2.validate()...)