Merge pull request #3138 from weaveworks/single-owner-nodeset

Use single-owner code path to accumulate children when rendering

app/merger_test.go

@@ -67,13 +67,17 @@ func benchmarkMerger(b *testing.B, merger app.Merger) {
 	for i := 0; i < numHosts*5; i++ {
 		reports = append(reports, makeReport())
 	}
+	replacements := []report.Report{}
+	for i := 0; i < numHosts/3; i++ {
+		replacements = append(replacements, makeReport())
+	}
 
 	b.ResetTimer()
 
 	for i := 0; i < b.N; i++ {
 		// replace 1/3 of hosts work of reports & merge them all
-		for i := 0; i < numHosts/3; i++ {
-			reports[rand.Intn(len(reports))] = makeReport()
+		for i := 0; i < len(replacements); i++ {
+			reports[rand.Intn(len(reports))] = replacements[i]
 		}
 
 		merger.Merge(reports)

render/render.go

@@ -153,7 +153,8 @@ type joinResults struct {
 func newJoinResults(inputNodes report.Nodes) joinResults {
 	nodes := make(report.Nodes, len(inputNodes))
 	for id, n := range inputNodes {
-		n.Adjacency = nil // result() assumes all nodes start with no adjacencies
+		n.Adjacency = nil              // result() assumes all nodes start with no adjacencies
+		n.Children = n.Children.Copy() // so we can do unsafe adds
 		nodes[id] = n
 	}
 	return joinResults{nodes: nodes, mapped: map[string]string{}, multi: map[string][]string{}}
@@ -183,7 +184,7 @@ func (ret *joinResults) addUnmappedChild(m report.Node, id string, topology stri
 	if !exists {
 		result = report.MakeNode(id).WithTopology(topology)
 	}
-	result.Children = result.Children.Add(m)
+	result.Children.UnsafeAdd(m)
 	if m.Topology != report.Endpoint { // optimisation: we never look at endpoint counts
 		result.Counters = result.Counters.Add(m.Topology, 1)
 	}
@@ -211,6 +212,7 @@ func (ret *joinResults) addChildAndChildren(m report.Node, id string, topology s
 func (ret *joinResults) passThrough(n report.Node) {
 	n.Adjacency = nil // result() assumes all nodes start with no adjacencies
 	ret.nodes[n.ID] = n
+	n.Children = n.Children.Copy() // so we can do unsafe adds
 	ret.mapChild(n.ID, n.ID)
 }
 

report/node_set.go

@@ -24,8 +24,24 @@ func MakeNodeSet(nodes ...Node) NodeSet {
 	return emptyNodeSet.Add(nodes...)
 }
 
-// Add adds the nodes to the NodeSet. Add is the only valid way to grow a
-// NodeSet. Add returns the NodeSet to enable chaining.
+// Copy returns a value copy of the Nodes.
+func (n NodeSet) Copy() NodeSet {
+	result := ps.NewMap()
+	n.ForEach(func(node Node) {
+		result = result.UnsafeMutableSet(node.ID, node)
+	})
+	return NodeSet{result}
+}
+
+// UnsafeAdd adds a node to the NodeSet. Only call this if n has one owner.
+func (n *NodeSet) UnsafeAdd(node Node) {
+	if n.psMap == nil {
+		n.psMap = ps.NewMap()
+	}
+	n.psMap = n.psMap.UnsafeMutableSet(node.ID, node)
+}
+
+// Add adds the nodes to the NodeSet, and returns the NodeSet to enable chaining.
 func (n NodeSet) Add(nodes ...Node) NodeSet {
 	if len(nodes) == 0 {
 		return n