Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove and optimize more Copy()s #1739

Merged
merged 3 commits into from
Aug 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion probe/host/tagger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 7 additions & 9 deletions render/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

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
}
Expand Down Expand Up @@ -365,14 +364,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..
Expand Down
12 changes: 9 additions & 3 deletions render/detailed/connections.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion render/detailed/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,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)
}
Expand Down
57 changes: 6 additions & 51 deletions render/detailed/summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -94,35 +81,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 {
Expand All @@ -134,7 +98,7 @@ func baseNodeSummary(r report.Report, n report.Node) NodeSummary {
Metadata: NodeMetadata(r, n),
Metrics: NodeMetrics(r, n),
Tables: NodeTables(r, n),
Adjacency: n.Adjacency.Copy(),
Adjacency: n.Adjacency,
}
}

Expand Down Expand Up @@ -372,15 +336,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{
Expand Down
2 changes: 1 addition & 1 deletion render/detailed/topology_diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 3 additions & 4 deletions render/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
20 changes: 6 additions & 14 deletions report/metric_row.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions report/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
14 changes: 0 additions & 14 deletions report/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading