Optimise merge where one side is a subset of the other

app/benchmark_internal_test.go

@@ -2,6 +2,7 @@ package app
 
 import (
 	"flag"
+	"math/rand"
 	"net/http"
 	"net/url"
 	"os"
@@ -65,6 +66,9 @@ func BenchmarkReportUpgrade(b *testing.B) {
 
 func BenchmarkReportMerge(b *testing.B) {
 	reports := upgradeReports(readReportFiles(b, *benchReportPath))
+	rand.Shuffle(len(reports), func(i, j int) {
+		reports[i], reports[j] = reports[j], reports[i]
+	})
 	merger := NewFastMerger()
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {

extras/generate_latest_map

@@ -74,27 +74,54 @@ function generate_latest_map() {
         return len(m)
     }
 
-    // Merge produces a fresh ${latest_map_type} containing the keys from both inputs.
+    // Merge produces a ${latest_map_type} containing the keys from both inputs.
     // When both inputs contain the same key, the newer value is used.
+    // Tries to return one of its inputs, if that already holds the correct result.
     func (m ${latest_map_type}) Merge(n ${latest_map_type}) ${latest_map_type} {
         switch {
-        case m == nil:
+        case len(m) == 0:
             return n
-        case n == nil:
+        case len(n) == 0:
             return m
         }
-        l := len(m)
-        if len(n) > l {
-            l = len(n)
+        if len(n) > len(m) {
+            m, n = n, m //swap so m is always at least as long as n
+        } else if len(n) == len(m) && m[0].Timestamp.Before(n[0].Timestamp) {
+            // Optimise common case where we merge two nodes with the same contents
+            // sampled at different times.
+            m, n = n, m // swap equal-length arrays so first element of m is newer
         }
-        out := make([]${entry_type}, 0, l)
 
         i, j := 0, 0
+    loop:
         for i < len(m) {
             switch {
-            case j >= len(n) || m[i].key < n[j].key:
-                out = append(out, m[i])
+            case j >= len(n):
+                return m
+            case m[i].key == n[j].key:
+                if m[i].Timestamp.Before(n[j].Timestamp) {
+                    break loop
+                }
                 i++
+                j++
+            case m[i].key < n[j].key:
+                i++
+            default:
+                break loop
+            }
+        }
+        if i >= len(m) && j >= len(n) {
+            return m
+        }
+
+        out := make([]${entry_type}, i, len(m))
+        copy(out, m[:i])
+
+        for i < len(m) {
+            switch {
+            case j >= len(n):
+                out = append(out, m[i:]...)
+                return out
             case m[i].key == n[j].key:
                 if m[i].Timestamp.Before(n[j].Timestamp) {
                     out = append(out, n[j])
@@ -103,6 +130,9 @@ function generate_latest_map() {
                 }
                 i++
                 j++
+            case m[i].key < n[j].key:
+                out = append(out, m[i])
+                i++
             default:
                 out = append(out, n[j])
                 j++

report/dns.go

@@ -27,10 +27,12 @@ func (r DNSRecords) Merge(other DNSRecords) DNSRecords {
 	cp := r.Copy()
 	for k, v := range other {
 		if v2, ok := cp[k]; ok {
-			cp[k] = DNSRecord{
-				Forward: v.Forward.Merge(v2.Forward),
-				Reverse: v.Reverse.Merge(v2.Reverse),
+			fMerged, fUnchanged := v.Forward.Merge(v2.Forward)
+			rMerged, rUnchanged := v.Reverse.Merge(v2.Reverse)
+			if fUnchanged && rUnchanged {
+				continue
 			}
+			cp[k] = DNSRecord{Forward: fMerged, Reverse: rMerged}
 		} else {
 			cp[k] = v
 		}

report/id_list.go

@@ -23,7 +23,8 @@ func (a IDList) Add(ids ...string) IDList {
 
 // Merge all elements from a and b into a new list
 func (a IDList) Merge(b IDList) IDList {
-	return IDList(StringSet(a).Merge(StringSet(b)))
+	merged, _ := StringSet(a).Merge(StringSet(b))
+	return IDList(merged)
 }
 
 // Contains returns true if id is in the list.

report/latest_map_generated.go

@@ -42,27 +42,54 @@ func (m StringLatestMap) Size() int {
 	return len(m)
 }
 
-// Merge produces a fresh StringLatestMap containing the keys from both inputs.
+// Merge produces a StringLatestMap containing the keys from both inputs.
 // When both inputs contain the same key, the newer value is used.
+// Tries to return one of its inputs, if that already holds the correct result.
 func (m StringLatestMap) Merge(n StringLatestMap) StringLatestMap {
 	switch {
-	case m == nil:
+	case len(m) == 0:
 		return n
-	case n == nil:
+	case len(n) == 0:
 		return m
 	}
-	l := len(m)
-	if len(n) > l {
-		l = len(n)
+	if len(n) > len(m) {
+		m, n = n, m //swap so m is always at least as long as n
+	} else if len(n) == len(m) && m[0].Timestamp.Before(n[0].Timestamp) {
+		// Optimise common case where we merge two nodes with the same contents
+		// sampled at different times.
+		m, n = n, m // swap equal-length arrays so first element of m is newer
 	}
-	out := make([]stringLatestEntry, 0, l)
 
 	i, j := 0, 0
+loop:
 	for i < len(m) {
 		switch {
-		case j >= len(n) || m[i].key < n[j].key:
-			out = append(out, m[i])
+		case j >= len(n):
+			return m
+		case m[i].key == n[j].key:
+			if m[i].Timestamp.Before(n[j].Timestamp) {
+				break loop
+			}
 			i++
+			j++
+		case m[i].key < n[j].key:
+			i++
+		default:
+			break loop
+		}
+	}
+	if i >= len(m) && j >= len(n) {
+		return m
+	}
+
+	out := make([]stringLatestEntry, i, len(m))
+	copy(out, m[:i])
+
+	for i < len(m) {
+		switch {
+		case j >= len(n):
+			out = append(out, m[i:]...)
+			return out
 		case m[i].key == n[j].key:
 			if m[i].Timestamp.Before(n[j].Timestamp) {
 				out = append(out, n[j])
@@ -71,6 +98,9 @@ func (m StringLatestMap) Merge(n StringLatestMap) StringLatestMap {
 			}
 			i++
 			j++
+		case m[i].key < n[j].key:
+			out = append(out, m[i])
+			i++
 		default:
 			out = append(out, n[j])
 			j++
@@ -264,27 +294,54 @@ func (m NodeControlDataLatestMap) Size() int {
 	return len(m)
 }
 
-// Merge produces a fresh NodeControlDataLatestMap containing the keys from both inputs.
+// Merge produces a NodeControlDataLatestMap containing the keys from both inputs.
 // When both inputs contain the same key, the newer value is used.
+// Tries to return one of its inputs, if that already holds the correct result.
 func (m NodeControlDataLatestMap) Merge(n NodeControlDataLatestMap) NodeControlDataLatestMap {
 	switch {
-	case m == nil:
+	case len(m) == 0:
 		return n
-	case n == nil:
+	case len(n) == 0:
 		return m
 	}
-	l := len(m)
-	if len(n) > l {
-		l = len(n)
+	if len(n) > len(m) {
+		m, n = n, m //swap so m is always at least as long as n
+	} else if len(n) == len(m) && m[0].Timestamp.Before(n[0].Timestamp) {
+		// Optimise common case where we merge two nodes with the same contents
+		// sampled at different times.
+		m, n = n, m // swap equal-length arrays so first element of m is newer
 	}
-	out := make([]nodeControlDataLatestEntry, 0, l)
 
 	i, j := 0, 0
+loop:
 	for i < len(m) {
 		switch {
-		case j >= len(n) || m[i].key < n[j].key:
-			out = append(out, m[i])
+		case j >= len(n):
+			return m
+		case m[i].key == n[j].key:
+			if m[i].Timestamp.Before(n[j].Timestamp) {
+				break loop
+			}
 			i++
+			j++
+		case m[i].key < n[j].key:
+			i++
+		default:
+			break loop
+		}
+	}
+	if i >= len(m) && j >= len(n) {
+		return m
+	}
+
+	out := make([]nodeControlDataLatestEntry, i, len(m))
+	copy(out, m[:i])
+
+	for i < len(m) {
+		switch {
+		case j >= len(n):
+			out = append(out, m[i:]...)
+			return out
 		case m[i].key == n[j].key:
 			if m[i].Timestamp.Before(n[j].Timestamp) {
 				out = append(out, n[j])
@@ -293,6 +350,9 @@ func (m NodeControlDataLatestMap) Merge(n NodeControlDataLatestMap) NodeControlD
 			}
 			i++
 			j++
+		case m[i].key < n[j].key:
+			out = append(out, m[i])
+			i++
 		default:
 			out = append(out, n[j])
 			j++

report/latest_map_internal_test.go

@@ -93,13 +93,6 @@ func TestLatestMapMerge(t *testing.T) {
 			want: MakeStringLatestMap().
 				Set("foo", now, "bar"),
 		},
-		"Empty b": {
-			a: MakeStringLatestMap().
-				Set("foo", now, "bar"),
-			b: MakeStringLatestMap(),
-			want: MakeStringLatestMap().
-				Set("foo", now, "bar"),
-		},
 		"Disjoint a & b": {
 			a: MakeStringLatestMap().
 				Set("foo", now, "bar"),
@@ -117,10 +110,26 @@ func TestLatestMapMerge(t *testing.T) {
 			want: MakeStringLatestMap().
 				Set("foo", now, "bar"),
 		},
+		"Longer": {
+			a: MakeStringLatestMap().
+				Set("PID", now, "23128").
+				Set("Name", now, "curl"),
+			b: MakeStringLatestMap().
+				Set("PID", then, "0").
+				Set("Name", now, "curl").
+				Set("Domain", now, "node-a.local"),
+			want: MakeStringLatestMap().
+				Set("PID", now, "23128").
+				Set("Name", now, "curl").
+				Set("Domain", now, "node-a.local"),
+		},
 	} {
 		if have := c.a.Merge(c.b); !reflect.DeepEqual(c.want, have) {
 			t.Errorf("%s:\n%s", name, test.Diff(c.want, have))
 		}
+		if have := c.b.Merge(c.a); !reflect.DeepEqual(c.want, have) {
+			t.Errorf("%s:\n%s", name, test.Diff(c.want, have))
+		}
 	}
 }
 

report/report.go

@@ -531,13 +531,12 @@ func (r Report) upgradeDNSRecords() Report {
 		if ok && (foundS || foundR) {
 			// Add address and names to report-level map
 			if existing, found := dns[addr]; found {
-				// Optimise the expected case that they are equal
-				if existing.Forward.Equal(snoopedNames) && existing.Reverse.Equal(reverseNames) {
+				var sUnchanged, rUnchanged bool
+				snoopedNames, sUnchanged = snoopedNames.Merge(existing.Forward)
+				reverseNames, rUnchanged = reverseNames.Merge(existing.Reverse)
+				if sUnchanged && rUnchanged {
 					continue
 				}
-				// Not equal - merge this node's data into existing data,
-				snoopedNames = snoopedNames.Merge(existing.Forward)
-				reverseNames = reverseNames.Merge(existing.Reverse)
 			}
 			dns[addr] = DNSRecord{Forward: snoopedNames, Reverse: reverseNames}
 		}

report/sets.go

@@ -35,7 +35,11 @@ func (s Sets) Add(key string, value StringSet) Sets {
 		s = emptySets
 	}
 	if existingValue, ok := s.psMap.Lookup(key); ok {
-		value = value.Merge(existingValue.(StringSet))
+		var unchanged bool
+		value, unchanged = existingValue.(StringSet).Merge(value)
+		if unchanged {
+			return s
+		}
 	}
 	return Sets{
 		psMap: s.psMap.Set(key, value),
@@ -94,7 +98,11 @@ func (s Sets) Merge(other Sets) Sets {
 	iter.ForEach(func(key string, value interface{}) {
 		set := value.(StringSet)
 		if existingSet, ok := result.Lookup(key); ok {
-			set = set.Merge(existingSet.(StringSet))
+			var unchanged bool
+			set, unchanged = existingSet.(StringSet).Merge(set)
+			if unchanged {
+				return
+			}
 		}
 		result = result.Set(key, set)
 	})