Merge pull request #1728 from weaveworks/no-copy-merge

don't Copy() self on Merge()

report/metadata_template.go

@@ -124,11 +124,14 @@ func (e MetadataTemplates) Copy() MetadataTemplates {
 // Merge merges two sets of MetadataTemplates so far just ignores based
 // on duplicate id key
 func (e MetadataTemplates) Merge(other MetadataTemplates) MetadataTemplates {
-	result := e.Copy()
+	if e == nil && other == nil {
+		return nil
+	}
+	result := make(MetadataTemplates, len(e))
+	for k, v := range e {
+		result[k] = v
+	}
 	for k, v := range other {
-		if result == nil {
-			result = MetadataTemplates{}
-		}
 		if existing, ok := result[k]; !ok || existing.Priority < v.Priority {
 			result[k] = v
 		}

report/metric_template.go

@@ -67,11 +67,14 @@ func (e MetricTemplates) Copy() MetricTemplates {
 // Merge merges two sets of MetricTemplates so far just ignores based
 // on duplicate id key
 func (e MetricTemplates) Merge(other MetricTemplates) MetricTemplates {
-	result := e.Copy()
+	if e == nil && other == nil {
+		return nil
+	}
+	result := make(MetricTemplates, len(e))
+	for k, v := range e {
+		result[k] = v
+	}
 	for k, v := range other {
-		if result == nil {
-			result = MetricTemplates{}
-		}
 		if existing, ok := result[k]; !ok || existing.Priority < v.Priority {
 			result[k] = v
 		}

report/node.go

@@ -194,23 +194,27 @@ func (n Node) Copy() Node {
 // Merge mergses the individual components of a node and returns a
 // fresh node.
 func (n Node) Merge(other Node) Node {
-	cp := n.Copy()
-	if cp.ID == "" {
-		cp.ID = other.ID
+	id := n.ID
+	if id == "" {
+		id = other.ID
 	}
-	if cp.Topology == "" {
-		cp.Topology = other.Topology
-	} else if other.Topology != "" && cp.Topology != other.Topology {
-		panic("Cannot merge nodes with different topology types: " + cp.Topology + " != " + other.Topology)
+	topology := n.Topology
+	if topology == "" {
+		topology = other.Topology
+	} else if other.Topology != "" && topology != other.Topology {
+		panic("Cannot merge nodes with different topology types: " + topology + " != " + other.Topology)
+	}
+	return Node{
+		ID:        id,
+		Topology:  topology,
+		Counters:  n.Counters.Merge(other.Counters),
+		Sets:      n.Sets.Merge(other.Sets),
+		Adjacency: n.Adjacency.Merge(other.Adjacency),
+		Edges:     n.Edges.Merge(other.Edges),
+		Controls:  n.Controls.Merge(other.Controls),
+		Latest:    n.Latest.Merge(other.Latest),
+		Metrics:   n.Metrics.Merge(other.Metrics),
+		Parents:   n.Parents.Merge(other.Parents),
+		Children:  n.Children.Merge(other.Children),
 	}
-	cp.Counters = cp.Counters.Merge(other.Counters)
-	cp.Sets = cp.Sets.Merge(other.Sets)
-	cp.Adjacency = cp.Adjacency.Merge(other.Adjacency)
-	cp.Edges = cp.Edges.Merge(other.Edges)
-	cp.Controls = cp.Controls.Merge(other.Controls)
-	cp.Latest = cp.Latest.Merge(other.Latest)
-	cp.Metrics = cp.Metrics.Merge(other.Metrics)
-	cp.Parents = cp.Parents.Merge(other.Parents)
-	cp.Children = cp.Children.Merge(other.Children)
-	return cp
 }

report/report.go

@@ -178,21 +178,22 @@ func (r Report) Copy() Report {
 // Merge merges another Report into the receiver and returns the result. The
 // original is not modified.
 func (r Report) Merge(other Report) Report {
-	cp := r.Copy()
-	cp.Endpoint = r.Endpoint.Merge(other.Endpoint)
-	cp.Process = r.Process.Merge(other.Process)
-	cp.Container = r.Container.Merge(other.Container)
-	cp.ContainerImage = r.ContainerImage.Merge(other.ContainerImage)
-	cp.Host = r.Host.Merge(other.Host)
-	cp.Pod = r.Pod.Merge(other.Pod)
-	cp.Service = r.Service.Merge(other.Service)
-	cp.Deployment = r.Deployment.Merge(other.Deployment)
-	cp.ReplicaSet = r.ReplicaSet.Merge(other.ReplicaSet)
-	cp.Overlay = r.Overlay.Merge(other.Overlay)
-	cp.Sampling = r.Sampling.Merge(other.Sampling)
-	cp.Window += other.Window
-	cp.Plugins = r.Plugins.Merge(other.Plugins)
-	return cp
+	return Report{
+		Endpoint:       r.Endpoint.Merge(other.Endpoint),
+		Process:        r.Process.Merge(other.Process),
+		Container:      r.Container.Merge(other.Container),
+		ContainerImage: r.ContainerImage.Merge(other.ContainerImage),
+		Host:           r.Host.Merge(other.Host),
+		Pod:            r.Pod.Merge(other.Pod),
+		Service:        r.Service.Merge(other.Service),
+		Deployment:     r.Deployment.Merge(other.Deployment),
+		ReplicaSet:     r.ReplicaSet.Merge(other.ReplicaSet),
+		Overlay:        r.Overlay.Merge(other.Overlay),
+		Sampling:       r.Sampling.Merge(other.Sampling),
+		Window:         r.Window + other.Window,
+		Plugins:        r.Plugins.Merge(other.Plugins),
+		ID:             fmt.Sprintf("%d", rand.Int63()),
+	}
 }
 
 // Topologies returns a slice of Topologies in this report

report/table.go

@@ -154,11 +154,14 @@ func (t TableTemplates) Copy() TableTemplates {
 
 // Merge merges two sets of TableTemplates
 func (t TableTemplates) Merge(other TableTemplates) TableTemplates {
-	result := t.Copy()
+	if t == nil && other == nil {
+		return nil
+	}
+	result := make(TableTemplates, len(t))
+	for k, v := range t {
+		result[k] = v
+	}
 	for k, v := range other {
-		if result == nil {
-			result = TableTemplates{}
-		}
 		if existing, ok := result[k]; ok {
 			v = v.Merge(existing)
 		}

report/topology.go

@@ -175,7 +175,10 @@ func (n Nodes) Copy() Nodes {
 // Merge merges the other object into this one, and returns the result object.
 // The original is not modified.
 func (n Nodes) Merge(other Nodes) Nodes {
-	cp := n.Copy()
+	cp := make(Nodes, len(n))
+	for k, v := range n {
+		cp[k] = v
+	}
 	for k, v := range other {
 		if n, ok := cp[k]; ok { // don't overwrite
 			v = v.Merge(n)