Optimize merge, make wildcards always take precedence

Author: jpbetz

Diff for: fieldpath/set.go

@@ -182,7 +182,7 @@ func PrefixMatcher(parts ...interface{}) (*SetMatcher, error) {
 			return nil, fmt.Errorf("unexpected type %T", t)
 		}
 		current = &SetMatcher{
-			Members: []*SetMemberMatcher{{
+			members: []*SetMemberMatcher{{
 				Path:  pattern,
 				Child: current,
 			}},
@@ -198,54 +198,61 @@ func MatchAnyPathElement() PathElementMatcher {
 
 // MatchAnySet returns a SetMatcher that matches any set.
 func MatchAnySet() *SetMatcher {
-	return &SetMatcher{Wildcard: true}
+	return &SetMatcher{wildcard: true}
+}
+
+// NewSetMatcher returns a new SetMatcher.
+// Wildcard members take precedent over non-wildcard members;
+// all non-wildcard members are ignored if there is a wildcard members.
+func NewSetMatcher(wildcard bool, members ...*SetMemberMatcher) *SetMatcher {
+	sort.Sort(sortedMemberMatcher(members))
+	return &SetMatcher{wildcard: wildcard, members: members}
 }
 
 // SetMatcher defines a pattern that matches fields in a Set.
 // SetMatcher is structured much like a Set but with wildcard support.
 type SetMatcher struct {
-	// Wildcard indicates that all members and children are included in the match.
-	// If set, the Members field is ignored.
-	Wildcard bool
-	// Members provides patterns to match the members of a Set.
-	Members []*SetMemberMatcher
+	// wildcard indicates that all members and children are included in the match.
+	// If set, the members field is ignored.
+	wildcard bool
+	// members provides patterns to match the members of a Set.
+	// Wildcard members are sorted before non-wildcards and take precedent over
+	// non-wildcard members.
+	members sortedMemberMatcher
+}
+
+type sortedMemberMatcher []*SetMemberMatcher
+
+func (s sortedMemberMatcher) Len() int           { return len(s) }
+func (s sortedMemberMatcher) Less(i, j int) bool { return s[i].Path.Less(s[j].Path) }
+func (s sortedMemberMatcher) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
+func (s sortedMemberMatcher) Find(p PathElementMatcher) (location int, ok bool) {
+	return sort.Find(len(s), func(i int) int {
+		return s[i].Path.Compare(p)
+	})
 }
 
-// Merge merges two SetMatchers into a single SetMatcher that matches the field paths of both SetMatchers.
-// During the merge, Members of s2 with the same PathElementMatcher as a member of s are merged into the member of s.
-// All other members of s2 are appended to the resulting member list in their original relative order.
-// When members are merged, the child SetMatchers are merged by calling this function recursively.
+// Merge merges s and s2 and returns a SetMatcher that matches all field paths matched by either s or s2.
+// During the merge, members of s and s2 with the same PathElementMatcher merged into a single member
+// with the children of each merged by calling this function recursively.
 func (s *SetMatcher) Merge(s2 *SetMatcher) *SetMatcher {
-	if s.Wildcard || s2.Wildcard {
-		return &SetMatcher{Wildcard: true}
-	}
-
-	// TODO: Optimize. This is O(n^2). In practice, usually a single member is being inserted at a time,
-	//       but that's still O(n). Sorting would help a ton.
-	var unionedMembers []*SetMemberMatcher
-	for _, m := range s.Members {
-		for _, m2 := range s2.Members {
-			if m.Path.PathElement.Equals(m2.Path.PathElement) && m.Path.Wildcard == m2.Path.Wildcard {
-				unionedMembers = append(unionedMembers, &SetMemberMatcher{
-					Path:  m.Path,
-					Child: m.Child.Merge(m2.Child),
-				})
-			} else {
-				unionedMembers = append(unionedMembers, m)
-			}
-		}
-	}
-	for _, m2 := range s2.Members {
-		for _, existing := range unionedMembers {
-			if !m2.Path.PathElement.Equals(existing.Path.PathElement) {
-				unionedMembers = append(unionedMembers, m2)
+	if s.wildcard || s2.wildcard {
+		return NewSetMatcher(true)
+	}
+	merged := make(sortedMemberMatcher, len(s.members), len(s.members)+len(s2.members))
+	copy(merged, s.members)
+	for _, m := range s2.members {
+		if i, ok := s.members.Find(m.Path); ok {
+			// since merged is a shallow copy, do not modify elements in place
+			merged[i] = &SetMemberMatcher{
+				Path:  merged[i].Path,
+				Child: merged[i].Child.Merge(m.Child),
 			}
+		} else {
+			merged = append(merged, m)
 		}
 	}
-
-	return &SetMatcher{
-		Members: unionedMembers,
-	}
+	return NewSetMatcher(false, merged...) // sort happens here
 }
 
 // SetMemberMatcher defines a pattern that matches the members of a Set.
@@ -274,15 +281,37 @@ type PathElementMatcher struct {
 	PathElement
 }
 
+func (p PathElementMatcher) Equals(p2 PathElementMatcher) bool {
+	return p.Wildcard != p2.Wildcard && p.PathElement.Equals(p2.PathElement)
+}
+
+func (p PathElementMatcher) Less(p2 PathElementMatcher) bool {
+	if p.Wildcard && !p2.Wildcard {
+		return true
+	} else if p2.Wildcard {
+		return false
+	}
+	return p.PathElement.Less(p2.PathElement)
+}
+
+func (p PathElementMatcher) Compare(p2 PathElementMatcher) int {
+	if p.Wildcard && !p2.Wildcard {
+		return -1
+	} else if p2.Wildcard {
+		return 1
+	}
+	return p.PathElement.Compare(p2.PathElement)
+}
+
 // FilterIncludeMatches returns a Set with only the field paths that match.
 func (s *Set) FilterIncludeMatches(pattern *SetMatcher) *Set {
-	if pattern.Wildcard {
+	if pattern.wildcard {
 		return s
 	}
 
 	members := PathElementSet{}
 	for _, m := range s.Members.members {
-		for _, pm := range pattern.Members {
+		for _, pm := range pattern.members {
 			if pm.Path.Wildcard || pm.Path.PathElement.Equals(m) {
 				members.Insert(m)
 				break
@@ -637,13 +666,13 @@ func (s *SetNodeMap) EnsureNamedFieldsAreMembers(sc *schema.Schema, tr schema.Ty
 
 // FilterIncludeMatches returns a SetNodeMap with only the field paths that match the pattern.
 func (s *SetNodeMap) FilterIncludeMatches(pattern *SetMatcher) *SetNodeMap {
-	if pattern.Wildcard {
+	if pattern.wildcard {
 		return s
 	}
 
 	var out sortedSetNode
 	for _, member := range s.members {
-		for _, c := range pattern.Members {
+		for _, c := range pattern.members {
 			if c.Path.Wildcard || c.Path.PathElement.Equals(member.pathElement) {
 				childSet := member.set.FilterIncludeMatches(c.Child)
 				if childSet.Size() > 0 {
@@ -727,7 +756,7 @@ func (t excludeFilter) Filter(set *Set) *Set {
 // PrefixMatcher and MakePrefixMatcherOrDie can help create basic SetPatterns.
 func NewIncludeMatcherFilter(patterns ...*SetMatcher) Filter {
 	if len(patterns) == 0 {
-		return includeMatcherFilter{&SetMatcher{Wildcard: true}}
+		return includeMatcherFilter{&SetMatcher{wildcard: true}}
 	}
 	pattern := patterns[0]
 	for i := 1; i < len(patterns); i++ {

Diff for: fieldpath/set_test.go

@@ -988,8 +988,8 @@ func TestFilterByPattern(t *testing.T) {
 				MakePathOrDie("spec", "list", 2, "f2"),
 			),
 			filter: NewIncludeMatcherFilter(
-				MakePrefixMatcherOrDie("spec", "list", MatchAnyPathElement(), "f1"), // wildcard matches first and runs
-				MakePrefixMatcherOrDie("spec", "list", 1, "f2"),                     // already matched by wildcard so this is ignored
+				MakePrefixMatcherOrDie("spec", "list", MatchAnyPathElement(), "f1"), // takes precedence
+				MakePrefixMatcherOrDie("spec", "list", 1, "f2"),                     // ignored
 			),
 			expect: NewSet(
 				MakePathOrDie("spec", "list", 0, "f1"),