Merge pull request #1739 from weaveworks/1730-no-node-copy

Remove and optimize more Copy()s

probe/host/tagger_test.go

@@ -17,7 +17,7 @@ func TestTagger(t *testing.T) {
 	r := report.MakeReport()
 	r.Process.AddNode(node)
 	rpt, _ := host.NewTagger(hostID).Tag(r)
-	have := rpt.Process.Nodes[endpointNodeID].Copy()
+	have := rpt.Process.Nodes[endpointNodeID]
 
 	// It should now have the host ID
 	wantHostID := report.MakeHostNodeID(hostID)

render/container.go

@@ -191,13 +191,12 @@ func (r containerWithImageNameRenderer) Render(rpt report.Report, dct Decorator)
 		imageNameWithoutVersion := docker.ImageNameWithoutVersion(imageName)
 		imageNodeID := report.MakeContainerImageNodeID(imageNameWithoutVersion)
 
-		output := c.Copy()
-		output = propagateLatest(docker.ImageName, image, output)
-		output = propagateLatest(docker.ImageLabelPrefix+"works.weave.role", image, output)
-		output.Parents = output.Parents.
+		c = propagateLatest(docker.ImageName, image, c)
+		c = propagateLatest(docker.ImageLabelPrefix+"works.weave.role", image, c)
+		c.Parents = c.Parents.
 			Delete(report.ContainerImage).
 			Add(report.ContainerImage, report.MakeStringSet(imageNodeID))
-		outputs[id] = output
+		outputs[id] = c
 	}
 	return outputs
 }
@@ -370,14 +369,13 @@ func MapContainerImage2Name(n report.Node, _ report.Networks) report.Nodes {
 	}
 
 	imageNameWithoutVersion := docker.ImageNameWithoutVersion(imageName)
-	output := n.Copy()
-	output.ID = report.MakeContainerImageNodeID(imageNameWithoutVersion)
+	n.ID = report.MakeContainerImageNodeID(imageNameWithoutVersion)
 
 	if imageID, ok := report.ParseContainerImageNodeID(n.ID); ok {
-		output.Sets = output.Sets.Add(docker.ImageID, report.EmptyStringSet.Add(imageID))
+		n.Sets = n.Sets.Add(docker.ImageID, report.EmptyStringSet.Add(imageID))
 	}
 
-	return report.Nodes{output.ID: output}
+	return report.Nodes{n.ID: n}
 }
 
 // MapContainer2Hostname maps container Nodes to 'hostname' renderabled nodes..

render/detailed/connections.go

@@ -75,7 +75,9 @@ func incomingConnectionsSummary(topologyID string, r report.Report, n report.Nod
 		if !node.Adjacency.Contains(n.ID) {
 			continue
 		}
-		remoteNode := node.Copy()
+		// Shallow-copy the node to obtain a different pointer
+		// on the remote side
+		remoteNode := node
 
 		// Work out what port they are talking to, and count the number of
 		// connections to that port.
@@ -127,8 +129,12 @@ func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Nod
 		if !ok {
 			continue
 		}
-		remoteNode := node.Copy()
-		remoteEndpointIDs := endpointChildIDsOf(remoteNode)
+
+		remoteEndpointIDs := endpointChildIDsOf(node)
+
+		// Shallow-copy the node to obtain a different pointer
+		// on the remote side
+		remoteNode := node
 
 		for _, localEndpoint := range localEndpoints {
 			_, localAddr, _, ok := report.ParseEndpointNodeID(localEndpoint.ID)

render/detailed/node.go

@@ -224,7 +224,7 @@ func children(r report.Report, n report.Node) []NodeSummaryGroup {
 	for _, spec := range nodeSummaryGroupSpecs {
 		if len(summaries[spec.topologyID]) > 0 {
 			sort.Sort(nodeSummariesByID(summaries[spec.TopologyID]))
-			group := spec.NodeSummaryGroup.Copy()
+			group := spec.NodeSummaryGroup
 			group.Nodes = summaries[spec.topologyID]
 			nodeSummaryGroups = append(nodeSummaryGroups, group)
 		}

render/detailed/summary.go

@@ -32,19 +32,6 @@ type NodeSummaryGroup struct {
 	Columns    []Column      `json:"columns"`
 }
 
-// Copy returns a value copy of the NodeSummaryGroup
-func (g NodeSummaryGroup) Copy() NodeSummaryGroup {
-	result := NodeSummaryGroup{
-		TopologyID: g.TopologyID,
-		Label:      g.Label,
-		Columns:    g.Columns,
-	}
-	for _, node := range g.Nodes {
-		result.Nodes = append(result.Nodes, node.Copy())
-	}
-	return result
-}
-
 // Column provides special json serialization for column ids, so they include
 // their label for the frontend.
 type Column struct {
@@ -95,35 +82,12 @@ func MakeNodeSummary(r report.Report, n report.Node) (NodeSummary, bool) {
 // SummarizeMetrics returns a copy of the NodeSummary where the metrics are
 // replaced with their summaries
 func (n NodeSummary) SummarizeMetrics() NodeSummary {
-	cp := n.Copy()
-	for i, m := range cp.Metrics {
-		cp.Metrics[i] = m.Summary()
+	summarizedMetrics := make([]report.MetricRow, len(n.Metrics))
+	for i, m := range n.Metrics {
+		summarizedMetrics[i] = m.Summary()
 	}
-	return cp
-}
-
-// Copy returns a value copy of the NodeSummary
-func (n NodeSummary) Copy() NodeSummary {
-	result := NodeSummary{
-		ID:         n.ID,
-		Label:      n.Label,
-		LabelMinor: n.LabelMinor,
-		Rank:       n.Rank,
-		Shape:      n.Shape,
-		Stack:      n.Stack,
-		Linkable:   n.Linkable,
-		Adjacency:  n.Adjacency.Copy(),
-	}
-	for _, row := range n.Metadata {
-		result.Metadata = append(result.Metadata, row.Copy())
-	}
-	for _, table := range n.Tables {
-		result.Tables = append(result.Tables, table.Copy())
-	}
-	for _, row := range n.Metrics {
-		result.Metrics = append(result.Metrics, row.Copy())
-	}
-	return result
+	n.Metrics = summarizedMetrics
+	return n
 }
 
 func baseNodeSummary(r report.Report, n report.Node) NodeSummary {
@@ -136,7 +100,7 @@ func baseNodeSummary(r report.Report, n report.Node) NodeSummary {
 		Metrics:   NodeMetrics(r, n),
 		Parents:   Parents(r, n),
 		Tables:    NodeTables(r, n),
-		Adjacency: n.Adjacency.Copy(),
+		Adjacency: n.Adjacency,
 	}
 }
 
@@ -376,15 +340,6 @@ func Summaries(r report.Report, rns report.Nodes) NodeSummaries {
 	return result
 }
 
-// Copy returns a deep value-copy of NodeSummaries
-func (n NodeSummaries) Copy() NodeSummaries {
-	result := NodeSummaries{}
-	for k, v := range n {
-		result[k] = v.Copy()
-	}
-	return result
-}
-
 // getRenderableContainerName obtains a user-friendly container name, to render in the UI
 func getRenderableContainerName(nmd report.Node) string {
 	for _, key := range []string{

render/detailed/topology_diff_test.go

@@ -25,7 +25,7 @@ func TestTopoDiff(t *testing.T) {
 		Pseudo:     false,
 		Adjacency:  report.MakeIDList("nodeb"),
 	}
-	nodeap := nodea.Copy()
+	nodeap := nodea
 	nodeap.Adjacency = report.MakeIDList("nodeb", "nodeq") // not the same anymore
 	nodeb := detailed.NodeSummary{
 		ID:    "nodeb",

render/process.go

@@ -63,12 +63,11 @@ func (r processWithContainerNameRenderer) Render(rpt report.Report, dct Decorato
 		if !ok {
 			continue
 		}
-		output := p.Copy()
-		output.Latest = output.Latest.Set(docker.ContainerID, timestamp, containerID)
+		p.Latest = p.Latest.Set(docker.ContainerID, timestamp, containerID)
 		if containerName, timestamp, ok := container.Latest.LookupEntry(docker.ContainerName); ok {
-			output.Latest = output.Latest.Set(docker.ContainerName, timestamp, containerName)
+			p.Latest = p.Latest.Set(docker.ContainerName, timestamp, containerName)
 		}
-		outputs[id] = output
+		outputs[id] = p
 	}
 	return outputs
 }

report/metric_row.go

@@ -27,21 +27,13 @@ type MetricRow struct {
 
 // Summary returns a copy of the MetricRow, without the samples, just the value if there is one.
 func (m MetricRow) Summary() MetricRow {
-	row := m.Copy()
-	if m.Metric != nil {
-		row.Metric.Samples = nil
+	if m.Metric.Len() > 0 {
+		// shallow-copy
+		metricCopy := *m.Metric
+		metricCopy.Samples = nil
+		m.Metric = &metricCopy
 	}
-	return row
-}
-
-// Copy returns a value copy of the MetricRow
-func (m MetricRow) Copy() MetricRow {
-	row := m
-	if m.Metric != nil {
-		var metric = m.Metric.Copy()
-		row.Metric = &metric
-	}
-	return row
+	return m
 }
 
 // MarshalJSON shouldn't be used, use CodecEncodeSelf instead

report/metrics.go

@@ -94,16 +94,6 @@ func MakeMetric(samples []Sample) Metric {
 	}
 }
 
-// Copy returns a copy of the Metric.
-func (m Metric) Copy() Metric {
-	c := m
-	if c.Samples != nil {
-		c.Samples = make([]Sample, len(m.Samples))
-		copy(c.Samples, m.Samples)
-	}
-	return c
-}
-
 // WithMax returns a fresh copy of m, with Max set to max
 func (m Metric) WithMax(max float64) Metric {
 	return Metric{

report/metrics_test.go

@@ -122,20 +122,6 @@ func TestMetricMerge(t *testing.T) {
 	}
 }
 
-func TestMetricCopy(t *testing.T) {
-	want := report.MakeMetric(nil)
-	have := want.Copy()
-	if !reflect.DeepEqual(want, have) {
-		t.Errorf("diff: %s", test.Diff(want, have))
-	}
-
-	want = report.MakeSingletonMetric(time.Now(), 1)
-	have = want.Copy()
-	if !reflect.DeepEqual(want, have) {
-		t.Errorf("diff: %s", test.Diff(want, have))
-	}
-}
-
 func TestMetricDiv(t *testing.T) {
 	t1 := time.Now()
 	t2 := time.Now().Add(1 * time.Minute)

report/node.go

@@ -45,16 +45,14 @@ func MakeNodeWith(id string, m map[string]string) Node {
 
 // WithID returns a fresh copy of n, with ID changed.
 func (n Node) WithID(id string) Node {
-	result := n.Copy()
-	result.ID = id
-	return result
+	n.ID = id
+	return n
 }
 
 // WithTopology returns a fresh copy of n, with ID changed.
 func (n Node) WithTopology(topology string) Node {
-	result := n.Copy()
-	result.Topology = topology
-	return result
+	n.Topology = topology
+	return n
 }
 
 // Before is used for sorting nodes by topology and id
@@ -74,121 +72,92 @@ func (n Node) After(other Node) bool {
 
 // WithLatests returns a fresh copy of n, with Metadata m merged in.
 func (n Node) WithLatests(m map[string]string) Node {
-	result := n.Copy()
 	ts := mtime.Now()
 	for k, v := range m {
-		result.Latest = result.Latest.Set(k, ts, v)
+		n.Latest = n.Latest.Set(k, ts, v)
 	}
-	return result
+	return n
 }
 
 // WithLatest produces a new Node with k mapped to v in the Latest metadata.
 func (n Node) WithLatest(k string, ts time.Time, v string) Node {
-	result := n.Copy()
-	result.Latest = result.Latest.Set(k, ts, v)
-	return result
+	n.Latest = n.Latest.Set(k, ts, v)
+	return n
 }
 
 // WithCounters returns a fresh copy of n, with Counters c merged in.
 func (n Node) WithCounters(c map[string]int) Node {
-	result := n.Copy()
-	result.Counters = result.Counters.Merge(Counters{}.fromIntermediate(c))
-	return result
+	n.Counters = n.Counters.Merge(Counters{}.fromIntermediate(c))
+	return n
 }
 
 // WithSet returns a fresh copy of n, with set merged in at key.
 func (n Node) WithSet(key string, set StringSet) Node {
-	result := n.Copy()
-	result.Sets = result.Sets.Add(key, set)
-	return result
+	n.Sets = n.Sets.Add(key, set)
+	return n
 }
 
 // WithSets returns a fresh copy of n, with sets merged in.
 func (n Node) WithSets(sets Sets) Node {
-	result := n.Copy()
-	result.Sets = result.Sets.Merge(sets)
-	return result
+	n.Sets = n.Sets.Merge(sets)
+	return n
 }
 
 // WithMetric returns a fresh copy of n, with metric merged in at key.
 func (n Node) WithMetric(key string, metric Metric) Node {
-	result := n.Copy()
-	result.Metrics[key] = n.Metrics[key].Merge(metric)
-	return result
+	n.Metrics = n.Metrics.Copy()
+	n.Metrics[key] = n.Metrics[key].Merge(metric)
+	return n
 }
 
 // WithMetrics returns a fresh copy of n, with metrics merged in.
 func (n Node) WithMetrics(metrics Metrics) Node {
-	result := n.Copy()
-	result.Metrics = result.Metrics.Merge(metrics)
-	return result
+	n.Metrics = n.Metrics.Merge(metrics)
+	return n
 }
 
 // WithAdjacent returns a fresh copy of n, with 'a' added to Adjacency
 func (n Node) WithAdjacent(a ...string) Node {
-	result := n.Copy()
-	result.Adjacency = result.Adjacency.Add(a...)
-	return result
+	n.Adjacency = n.Adjacency.Add(a...)
+	return n
 }
 
 // WithEdge returns a fresh copy of n, with 'dst' added to Adjacency and md
 // added to EdgeMetadata.
 func (n Node) WithEdge(dst string, md EdgeMetadata) Node {
-	result := n.Copy()
-	result.Adjacency = result.Adjacency.Add(dst)
-	result.Edges = result.Edges.Add(dst, md)
-	return result
+	n.Adjacency = n.Adjacency.Add(dst)
+	n.Edges = n.Edges.Add(dst, md)
+	return n
 }
 
 // WithControls returns a fresh copy of n, with cs added to Controls.
 func (n Node) WithControls(cs ...string) Node {
-	result := n.Copy()
-	result.Controls = result.Controls.Add(cs...)
-	return result
+	n.Controls = n.Controls.Add(cs...)
+	return n
 }
 
 // WithParents returns a fresh copy of n, with sets merged in.
 func (n Node) WithParents(parents Sets) Node {
-	result := n.Copy()
-	result.Parents = result.Parents.Merge(parents)
-	return result
+	n.Parents = n.Parents.Merge(parents)
+	return n
 }
 
 // PruneParents returns a fresh copy of n, without any parents.
 func (n Node) PruneParents() Node {
-	result := n.Copy()
-	result.Parents = EmptySets
-	return result
+	n.Parents = EmptySets
+	return n
 }
 
 // WithChildren returns a fresh copy of n, with children merged in.
 func (n Node) WithChildren(children NodeSet) Node {
-	result := n.Copy()
-	result.Children = result.Children.Merge(children)
-	return result
+	n.Children = n.Children.Merge(children)
+	return n
 }
 
 // WithChild returns a fresh copy of n, with one child merged in.
 func (n Node) WithChild(child Node) Node {
-	result := n.Copy()
-	result.Children = result.Children.Merge(MakeNodeSet(child))
-	return result
-}
-
-// Copy returns a value copy of the Node.
-func (n Node) Copy() Node {
-	cp := MakeNode(n.ID)
-	cp.Topology = n.Topology
-	cp.Counters = n.Counters.Copy()
-	cp.Sets = n.Sets.Copy()
-	cp.Adjacency = n.Adjacency.Copy()
-	cp.Edges = n.Edges.Copy()
-	cp.Controls = n.Controls.Copy()
-	cp.Latest = n.Latest.Copy()
-	cp.Metrics = n.Metrics.Copy()
-	cp.Parents = n.Parents.Copy()
-	cp.Children = n.Children.Copy()
-	return cp
+	n.Children = n.Children.Merge(MakeNodeSet(child))
+	return n
 }
 
 // Merge mergses the individual components of a node and returns a

report/topology.go

@@ -167,7 +167,7 @@ type Nodes map[string]Node
 func (n Nodes) Copy() Nodes {
 	cp := make(Nodes, len(n))
 	for k, v := range n {
-		cp[k] = v.Copy()
+		cp[k] = v
 	}
 	return cp
 }