From 76d658b1936d50666c07d435d1ad63e19ee9dfc9 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 7 Jul 2015 22:15:07 +0100 Subject: [PATCH 1/3] Make MakeIDList() enforce invariant --- report/id_list.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/report/id_list.go b/report/id_list.go index 9d89d7d08..29d8b4f48 100644 --- a/report/id_list.go +++ b/report/id_list.go @@ -8,6 +8,12 @@ type IDList []string // MakeIDList makes a new IDList. func MakeIDList(ids ...string) IDList { sort.Strings(ids) + for i := 1; i < len(ids); i++ { // shuffle down any duplicates + if ids[i-1] == ids[i] { + ids = append(ids[:i-1], ids[i:]...) + i-- + } + } return IDList(ids) } From baf0d94af9fe7be2d328c8dcb26b6484ded83e13 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 7 Jul 2015 21:59:39 +0100 Subject: [PATCH 2/3] Implement a Merge function on IDLists which is more efficient than repeated Add --- render/render.go | 2 +- render/renderable_node.go | 4 ++-- report/id_list.go | 28 ++++++++++++++++++++++++++++ report/merge.go | 2 +- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/render/render.go b/render/render.go index 4d426e47c..65e5b6204 100644 --- a/render/render.go +++ b/render/render.go @@ -85,7 +85,7 @@ func (m Map) render(rpt report.Report) (RenderableNodes, map[string]string) { output[outRenderable.ID] = outRenderable mapped[inRenderable.ID] = outRenderable.ID - adjacencies[outRenderable.ID] = adjacencies[outRenderable.ID].Add(inRenderable.Adjacency...) + adjacencies[outRenderable.ID] = adjacencies[outRenderable.ID].Merge(inRenderable.Adjacency) } // Rewrite Adjacency for new node IDs. diff --git a/render/renderable_node.go b/render/renderable_node.go index c5ccad8f4..fec35f9e6 100644 --- a/render/renderable_node.go +++ b/render/renderable_node.go @@ -53,8 +53,8 @@ func (rn *RenderableNode) Merge(other RenderableNode) { panic(rn.ID) } - rn.Adjacency = rn.Adjacency.Add(other.Adjacency...) - rn.Origins = rn.Origins.Add(other.Origins...) + rn.Adjacency = rn.Adjacency.Merge(other.Adjacency) + rn.Origins = rn.Origins.Merge(other.Origins) rn.AggregateMetadata.Merge(other.AggregateMetadata) rn.NodeMetadata.Merge(other.NodeMetadata) diff --git a/report/id_list.go b/report/id_list.go index 29d8b4f48..a85486485 100644 --- a/report/id_list.go +++ b/report/id_list.go @@ -31,6 +31,34 @@ func (a IDList) Add(ids ...string) IDList { return a } +// Merge all elements from a and b into a new list +func (a IDList) Merge(b IDList) IDList { + if len(b) == 0 { // Optimise special case, to avoid allocating + return a // (note unit test DeepEquals breaks if we don't do this) + } + d := make(IDList, len(a)+len(b)) + for i, j, k := 0, 0, 0; ; k++ { + switch { + case i >= len(a): + copy(d[k:], b[j:]) + return d[:k+len(b)-j] + case j >= len(b): + copy(d[k:], a[i:]) + return d[:k+len(a)-i] + case a[i] < b[j]: + d[k] = a[i] + i++ + case a[i] > b[j]: + d[k] = b[j] + j++ + default: // equal + d[k] = a[i] + i++ + j++ + } + } +} + // Contains returns true if id is in the list. func (a IDList) Contains(id string) bool { i := sort.Search(len(a), func(i int) bool { return a[i] >= id }) diff --git a/report/merge.go b/report/merge.go index 6c1e06d75..c57bec087 100644 --- a/report/merge.go +++ b/report/merge.go @@ -24,7 +24,7 @@ func (t *Topology) Merge(other Topology) { // Merge merges another Adjacency list into the receiver. func (a *Adjacency) Merge(other Adjacency) { for addr, adj := range other { - (*a)[addr] = (*a)[addr].Add(adj...) + (*a)[addr] = (*a)[addr].Merge(adj) } } From 35f9dc622e57f2d10b3e61a76ee0ad8a7b0f79cb Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 7 Jul 2015 17:00:28 +0100 Subject: [PATCH 3/3] More efficient slice-insert - less copying, less garbage --- report/id_list.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/report/id_list.go b/report/id_list.go index a85486485..6fd9596e1 100644 --- a/report/id_list.go +++ b/report/id_list.go @@ -26,7 +26,9 @@ func (a IDList) Add(ids ...string) IDList { continue } // It a new element, insert it in order. - a = append(a[:i], append(IDList{s}, a[i:]...)...) + a = append(a, "") + copy(a[i+1:], a[i:]) + a[i] = s } return a }