Skip to content

Commit

Permalink
Merge pull request #3253 from weaveworks/optimise-merge-subset
Browse files Browse the repository at this point in the history
Optimise merge where one side is a subset of the other
  • Loading branch information
bboreham authored Jul 13, 2018
2 parents 4bd0be0 + 692214b commit 9ff89c8
Show file tree
Hide file tree
Showing 11 changed files with 242 additions and 71 deletions.
4 changes: 4 additions & 0 deletions app/benchmark_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package app

import (
"flag"
"math/rand"
"net/http"
"net/url"
"os"
Expand Down Expand Up @@ -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++ {
Expand Down
48 changes: 39 additions & 9 deletions extras/generate_latest_map
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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++
Expand Down
8 changes: 5 additions & 3 deletions report/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion report/id_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
96 changes: 78 additions & 18 deletions report/latest_map_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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++
Expand Down Expand Up @@ -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])
Expand All @@ -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++
Expand Down
23 changes: 16 additions & 7 deletions report/latest_map_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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))
}
}
}

Expand Down
9 changes: 4 additions & 5 deletions report/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}
Expand Down
12 changes: 10 additions & 2 deletions report/sets.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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)
})
Expand Down
Loading

0 comments on commit 9ff89c8

Please sign in to comment.