From c4fa72110c8bca982ff7e24ce85d322d89777f65 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 28 Jul 2016 22:21:06 +0100 Subject: [PATCH 1/2] don't Copy() self on Merge() Merge() is always returning a copy, so there is no need to Copy() struct fields first before calling Merge() on them. This reduces GC pressure (#1010) and helps overall app performance (#1457). --- report/node.go | 38 +++++++++++++++++++++----------------- report/report.go | 31 ++++++++++++++++--------------- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/report/node.go b/report/node.go index f7ff0ff68..b1143f23f 100644 --- a/report/node.go +++ b/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 } diff --git a/report/report.go b/report/report.go index 406471e3b..a92dc4914 100644 --- a/report/report.go +++ b/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 From 17fe25d8c3d228ecf720ac3f65324c10a0c41bc4 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Fri, 29 Jul 2016 09:44:05 +0100 Subject: [PATCH 2/2] shallow-copy instead of (deep) Copy() in Merge() Merge() must not modify self or other; shallow-copying is sufficient to achieve that. --- report/metadata_template.go | 11 +++++++---- report/metric_template.go | 11 +++++++---- report/table.go | 11 +++++++---- report/topology.go | 5 ++++- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/report/metadata_template.go b/report/metadata_template.go index 16d99af31..42d872b21 100644 --- a/report/metadata_template.go +++ b/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 } diff --git a/report/metric_template.go b/report/metric_template.go index 8cd46f244..092bbba97 100644 --- a/report/metric_template.go +++ b/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 } diff --git a/report/table.go b/report/table.go index 6ab4f5a20..08d3b88f9 100644 --- a/report/table.go +++ b/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) } diff --git a/report/topology.go b/report/topology.go index da57e994b..5fbe93a4a 100644 --- a/report/topology.go +++ b/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)