don't use maps for PathElementSet; saves a couple hundred allocations on all benchmarks; needs more testing of the lexical operator

Author: lavalamp

fieldpath/element.go

@@ -47,6 +47,56 @@ type PathElement struct {
 	Index *int
 }
 
+// less provides an order for path elements.
+func (e PathElement) less(rhs PathElement) bool {
+	// For testing the concept, just use the existing string.
+	//return e.String() < rhs.String()
+	if e.FieldName != nil {
+		if rhs.FieldName == nil {
+			return true
+		}
+		return *e.FieldName < *rhs.FieldName
+	} else if rhs.FieldName != nil {
+		return false
+	}
+
+	if len(e.Key) != 0 {
+		if len(rhs.Key) == 0 {
+			return true
+		}
+		return value.FieldCollectionLess(e.Key, rhs.Key)
+	} else if len(rhs.Key) != 0 {
+		return false
+	}
+
+	if e.Value != nil {
+		if rhs.Value == nil {
+			return true
+		}
+		return e.Value.Less(*rhs.Value)
+	} else if rhs.Value != nil {
+		return false
+	}
+
+	if e.Index != nil {
+		if rhs.Index == nil {
+			return true
+		}
+		return *e.Index < *rhs.Index
+	} else if rhs.Index != nil {
+		// Yes, I know the next statement is the same.  But this way
+		// the obvious way of extending the function wil be bug-free.
+		return false
+	}
+
+	return false
+}
+
+// Equal returns true if both path elements are equal.
+func (e PathElement) Equal(rhs PathElement) bool {
+	return !e.less(rhs) && !rhs.less(e)
+}
+
 // String presents the path element as a human-readable string.
 func (e PathElement) String() string {
 	switch {
@@ -94,60 +144,103 @@ func KeyByFields(nameValues ...interface{}) []value.Field {
 type PathElementSet struct {
 	// The strange construction is because there's no way to test
 	// PathElements for equality (it can't be used as a key for a map).
-	members map[string]PathElement
+	members sortedPathElements
 }
 
+type sortedPathElements []PathElement
+
+func (spe sortedPathElements) Len() int           { return len(spe) }
+func (spe sortedPathElements) Less(i, j int) bool { return spe[i].less(spe[j]) }
+func (spe sortedPathElements) Swap(i, j int)      { spe[i], spe[j] = spe[j], spe[i] }
+
 // Insert adds pe to the set.
 func (s *PathElementSet) Insert(pe PathElement) {
-	serialized := pe.String()
-	if s.members == nil {
-		s.members = map[string]PathElement{
-			serialized: pe,
-		}
+	loc := sort.Search(len(s.members), func(i int) bool {
+		//return !pe.less(s.members[i])
+		return !s.members[i].less(pe)
+	})
+	if loc == len(s.members) {
+		s.members = append(s.members, pe)
 		return
 	}
-	if _, ok := s.members[serialized]; !ok {
-		s.members[serialized] = pe
+	if s.members[loc].Equal(pe) {
+		return
+	}
+	n := len(s.members) - 1
+	s.members = append(s.members, s.members[n])
+	for n > loc {
+		s.members[n] = s.members[n-1]
+		n--
 	}
+	s.members[loc] = pe
 }
 
 // Union returns a set containing elements that appear in either s or s2.
 func (s *PathElementSet) Union(s2 *PathElementSet) *PathElementSet {
-	out := &PathElementSet{
-		members: map[string]PathElement{},
+	out := &PathElementSet{}
+
+	i, j := 0, 0
+	for i < len(s.members) && j < len(s2.members) {
+		if s.members[i].less(s2.members[j]) {
+			out.members = append(out.members, s.members[i])
+			i++
+		} else {
+			out.members = append(out.members, s2.members[j])
+			if !s2.members[j].less(s.members[i]) {
+				i++
+			}
+			j++
+		}
 	}
-	for k, v := range s.members {
-		out.members[k] = v
+
+	if i < len(s.members) {
+		out.members = append(out.members, s.members[i:]...)
 	}
-	for k, v := range s2.members {
-		out.members[k] = v
+	if j < len(s2.members) {
+		out.members = append(out.members, s2.members[j:]...)
 	}
 	return out
 }
 
 // Intersection returns a set containing elements which appear in both s and s2.
 func (s *PathElementSet) Intersection(s2 *PathElementSet) *PathElementSet {
-	out := &PathElementSet{
-		members: map[string]PathElement{},
-	}
-	for k, v := range s.members {
-		if _, ok := s2.members[k]; ok {
-			out.members[k] = v
+	out := &PathElementSet{}
+
+	i, j := 0, 0
+	for i < len(s.members) && j < len(s2.members) {
+		if s.members[i].less(s2.members[j]) {
+			i++
+		} else {
+			if !s2.members[j].less(s.members[i]) {
+				out.members = append(out.members, s.members[i])
+				i++
+			}
+			j++
 		}
 	}
+
 	return out
 }
 
 // Difference returns a set containing elements which appear in s but not in s2.
 func (s *PathElementSet) Difference(s2 *PathElementSet) *PathElementSet {
-	out := &PathElementSet{
-		members: map[string]PathElement{},
-	}
-	for k, v := range s.members {
-		if _, ok := s2.members[k]; !ok {
-			out.members[k] = v
+	out := &PathElementSet{}
+
+	i, j := 0, 0
+	for i < len(s.members) && j < len(s2.members) {
+		if s.members[i].less(s2.members[j]) {
+			out.members = append(out.members, s.members[i])
+			i++
+		} else {
+			if !s2.members[j].less(s.members[i]) {
+				i++
+			}
+			j++
 		}
 	}
+	if i < len(s.members) {
+		out.members = append(out.members, s.members[i:]...)
+	}
 	return out
 }
 
@@ -156,11 +249,17 @@ func (s *PathElementSet) Size() int { return len(s.members) }
 
 // Has returns true if pe is a member of the set.
 func (s *PathElementSet) Has(pe PathElement) bool {
-	if s.members == nil {
+	loc := sort.Search(len(s.members), func(i int) bool {
+		return !s.members[i].less(pe)
+		//return !pe.less(s.members[i])
+	})
+	if loc == len(s.members) {
 		return false
 	}
-	_, ok := s.members[pe.String()]
-	return ok
+	if s.members[loc].Equal(pe) {
+		return true
+	}
+	return false
 }
 
 // Equals returns true if s and s2 have exactly the same members.
@@ -169,14 +268,14 @@ func (s *PathElementSet) Equals(s2 *PathElementSet) bool {
 		return false
 	}
 	for k := range s.members {
-		if _, ok := s2.members[k]; !ok {
+		if !s.members[k].Equal(s2.members[k]) {
 			return false
 		}
 	}
 	return true
 }
 
-// Iterate calls f for each PathElement in the set.
+// Iterate calls f for each PathElement in the set. The order is deterministic.
 func (s *PathElementSet) Iterate(f func(PathElement)) {
 	for _, pe := range s.members {
 		f(pe)

fieldpath/element_test.go

@@ -28,4 +28,34 @@ func TestPathElementSet(t *testing.T) {
 	if s2.Equals(s) {
 		t.Errorf("unequal sets should not equal")
 	}
+	if !s2.Has(PathElement{}) {
+		t.Errorf("expected to have something: %#v", s2)
+	}
+
+	n1 := "aoeu"
+	n2 := "asdf"
+	s2.Insert(PathElement{FieldName: &n1})
+	if !s2.Has(PathElement{FieldName: &n1}) {
+		t.Errorf("expected to have something: %#v", s2)
+	}
+	if s2.Has(PathElement{FieldName: &n2}) {
+		t.Errorf("expected to not have something: %#v", s2)
+	}
+
+	s2.Insert(PathElement{FieldName: &n2})
+	expected := []*string{&n1, &n2, nil}
+	i := 0
+	s2.Iterate(func(pe PathElement) {
+		e, a := expected[i], pe.FieldName
+		if e == nil || a == nil {
+			if e != a {
+				t.Errorf("index %v wanted %#v, got %#v", i, e, a)
+			}
+		} else {
+			if *e != *a {
+				t.Errorf("index %v wanted %#v, got %#v", i, *e, *a)
+			}
+		}
+		i++
+	})
 }