From ebfcbe185f5860601a9b4c5a54a6674913361cb3 Mon Sep 17 00:00:00 2001 From: Matt Joiner Date: Tue, 9 Apr 2019 15:28:54 +1000 Subject: [PATCH 1/2] Remove a lot of allocations, and remove some ambiguous naming --- sorting.go | 69 ++++++++++++++++++++++++++++++------------------------ table.go | 28 +++++++++++----------- 2 files changed, 52 insertions(+), 45 deletions(-) diff --git a/sorting.go b/sorting.go index da7315d..37e52dc 100644 --- a/sorting.go +++ b/sorting.go @@ -13,48 +13,55 @@ type peerDistance struct { distance ID } -// peerSorterArr implements sort.Interface to sort peers by xor distance -type peerSorterArr []*peerDistance +// peerDistanceSorter implements sort.Interface to sort peers by xor distance +type peerDistanceSorter struct { + peers []peerDistance + target ID +} -func (p peerSorterArr) Len() int { return len(p) } -func (p peerSorterArr) Swap(a, b int) { p[a], p[b] = p[b], p[a] } -func (p peerSorterArr) Less(a, b int) bool { - return p[a].distance.less(p[b].distance) +func (pds peerDistanceSorter) Len() int { return len(pds.peers) } +func (pds peerDistanceSorter) Swap(a, b int) { pds.peers[a], pds.peers[b] = pds.peers[b], pds.peers[a] } +func (pds peerDistanceSorter) Less(a, b int) bool { + return pds.peers[a].distance.less(pds.peers[b].distance) } -// +// Append the peer.ID to the sorter's slice. It may no longer be sorted. +func (pds peerDistanceSorter) appendPeer(p peer.ID) peerDistanceSorter { + pds.peers = append(pds.peers, peerDistance{ + p: p, + distance: xor(pds.target, ConvertPeerID(p)), + }) + return pds +} -func copyPeersFromList(target ID, peerArr peerSorterArr, peerList *list.List) peerSorterArr { - if cap(peerArr) < len(peerArr)+peerList.Len() { - newArr := make(peerSorterArr, len(peerArr), len(peerArr)+peerList.Len()) - copy(newArr, peerArr) - peerArr = newArr +// Append the peer.ID values in the list to the sorter's slice. It may no longer be sorted. +func (pds peerDistanceSorter) appendPeersFromList(l *list.List) peerDistanceSorter { + startLen := pds.Len() + for e := l.Front(); e != nil; e = e.Next() { + pds = pds.appendPeer(e.Value.(peer.ID)) } - for e := peerList.Front(); e != nil; e = e.Next() { - p := e.Value.(peer.ID) - pID := ConvertPeerID(p) - pd := peerDistance{ - p: p, - distance: xor(target, pID), - } - peerArr = append(peerArr, &pd) + if pds.Len() != startLen+l.Len() { + panic("len did not increase") } - return peerArr + return pds } +func (pds peerDistanceSorter) sort() { + sort.Sort(pds) +} + +// Sort the given peers by their ascending distance from the target. A new slice is returned. func SortClosestPeers(peers []peer.ID, target ID) []peer.ID { - psarr := make(peerSorterArr, 0, len(peers)) + sorter := peerDistanceSorter{ + peers: make([]peerDistance, 0, len(peers)), + target: target, + } for _, p := range peers { - pID := ConvertPeerID(p) - pd := &peerDistance{ - p: p, - distance: xor(target, pID), - } - psarr = append(psarr, pd) + sorter = sorter.appendPeer(p) } - sort.Sort(psarr) - out := make([]peer.ID, 0, len(psarr)) - for _, p := range psarr { + sorter.sort() + out := make([]peer.ID, 0, sorter.Len()) + for _, p := range sorter.peers { out = append(out, p.p) } return out diff --git a/table.go b/table.go index 881102f..57e0c0c 100644 --- a/table.go +++ b/table.go @@ -4,7 +4,6 @@ package kbucket import ( "errors" "fmt" - "sort" "sync" "time" @@ -169,6 +168,7 @@ func (rt *RoutingTable) NearestPeer(id ID) peer.ID { func (rt *RoutingTable) NearestPeers(id ID, count int) []peer.ID { cpl := CommonPrefixLen(id, rt.local) + // It's assumed that this also protects the buckets. rt.tabLock.RLock() // Get bucket at cpl index or last bucket @@ -178,32 +178,32 @@ func (rt *RoutingTable) NearestPeers(id ID, count int) []peer.ID { } bucket = rt.Buckets[cpl] - peerArr := make(peerSorterArr, 0, count) - peerArr = copyPeersFromList(id, peerArr, bucket.list) - if len(peerArr) < count { + pds := peerDistanceSorter{ + peers: make([]peerDistance, 0, 3*rt.bucketsize), + target: id, + } + pds = pds.appendPeersFromList(bucket.list) + if pds.Len() < count { // In the case of an unusual split, one bucket may be short or empty. // if this happens, search both surrounding buckets for nearby peers if cpl > 0 { - plist := rt.Buckets[cpl-1].list - peerArr = copyPeersFromList(id, peerArr, plist) + pds = pds.appendPeersFromList(rt.Buckets[cpl-1].list) } - if cpl < len(rt.Buckets)-1 { - plist := rt.Buckets[cpl+1].list - peerArr = copyPeersFromList(id, peerArr, plist) + pds = pds.appendPeersFromList(rt.Buckets[cpl+1].list) } } rt.tabLock.RUnlock() // Sort by distance to local peer - sort.Sort(peerArr) + pds.sort() - if count < len(peerArr) { - peerArr = peerArr[:count] + if count < pds.Len() { + pds.peers = pds.peers[:count] } - out := make([]peer.ID, 0, len(peerArr)) - for _, p := range peerArr { + out := make([]peer.ID, 0, pds.Len()) + for _, p := range pds.peers { out = append(out, p.p) } From 8285ec5fbe04e28632f88aa48649af9be4b71ddd Mon Sep 17 00:00:00 2001 From: Matt Joiner Date: Wed, 10 Apr 2019 14:43:06 +1000 Subject: [PATCH 2/2] Improve ergonomics Additionally, there it may require less allocations to pack the interface value to sort.Sort now, and lint warning about modifying a receiver in peerDistanceSorter.appendPeersFromList goes away. --- sorting.go | 22 ++++++++-------------- table.go | 6 +++--- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/sorting.go b/sorting.go index 37e52dc..b0bb361 100644 --- a/sorting.go +++ b/sorting.go @@ -19,34 +19,28 @@ type peerDistanceSorter struct { target ID } -func (pds peerDistanceSorter) Len() int { return len(pds.peers) } -func (pds peerDistanceSorter) Swap(a, b int) { pds.peers[a], pds.peers[b] = pds.peers[b], pds.peers[a] } -func (pds peerDistanceSorter) Less(a, b int) bool { +func (pds *peerDistanceSorter) Len() int { return len(pds.peers) } +func (pds *peerDistanceSorter) Swap(a, b int) { pds.peers[a], pds.peers[b] = pds.peers[b], pds.peers[a] } +func (pds *peerDistanceSorter) Less(a, b int) bool { return pds.peers[a].distance.less(pds.peers[b].distance) } // Append the peer.ID to the sorter's slice. It may no longer be sorted. -func (pds peerDistanceSorter) appendPeer(p peer.ID) peerDistanceSorter { +func (pds *peerDistanceSorter) appendPeer(p peer.ID) { pds.peers = append(pds.peers, peerDistance{ p: p, distance: xor(pds.target, ConvertPeerID(p)), }) - return pds } // Append the peer.ID values in the list to the sorter's slice. It may no longer be sorted. -func (pds peerDistanceSorter) appendPeersFromList(l *list.List) peerDistanceSorter { - startLen := pds.Len() +func (pds *peerDistanceSorter) appendPeersFromList(l *list.List) { for e := l.Front(); e != nil; e = e.Next() { - pds = pds.appendPeer(e.Value.(peer.ID)) + pds.appendPeer(e.Value.(peer.ID)) } - if pds.Len() != startLen+l.Len() { - panic("len did not increase") - } - return pds } -func (pds peerDistanceSorter) sort() { +func (pds *peerDistanceSorter) sort() { sort.Sort(pds) } @@ -57,7 +51,7 @@ func SortClosestPeers(peers []peer.ID, target ID) []peer.ID { target: target, } for _, p := range peers { - sorter = sorter.appendPeer(p) + sorter.appendPeer(p) } sorter.sort() out := make([]peer.ID, 0, sorter.Len()) diff --git a/table.go b/table.go index 57e0c0c..b62c6a2 100644 --- a/table.go +++ b/table.go @@ -182,15 +182,15 @@ func (rt *RoutingTable) NearestPeers(id ID, count int) []peer.ID { peers: make([]peerDistance, 0, 3*rt.bucketsize), target: id, } - pds = pds.appendPeersFromList(bucket.list) + pds.appendPeersFromList(bucket.list) if pds.Len() < count { // In the case of an unusual split, one bucket may be short or empty. // if this happens, search both surrounding buckets for nearby peers if cpl > 0 { - pds = pds.appendPeersFromList(rt.Buckets[cpl-1].list) + pds.appendPeersFromList(rt.Buckets[cpl-1].list) } if cpl < len(rt.Buckets)-1 { - pds = pds.appendPeersFromList(rt.Buckets[cpl+1].list) + pds.appendPeersFromList(rt.Buckets[cpl+1].list) } } rt.tabLock.RUnlock()