From 74b3f8d44e1ab9b06ab4725697b02a657f6e68b8 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 6 Apr 2015 11:35:11 +0100 Subject: [PATCH 1/3] use map[...]struct{} for sets instead of map[...]bool See https://groups.google.com/d/msg/golang-nuts/lb4xLHq7wug/MhrSLkyS4F8J We don't really care about the performance aspect of this, but using struct{}{} values is in some way canonical since it means the representation carries no redundant information. --- router/connection_maker.go | 34 +++++++++++++++++++--------------- router/mocks_test.go | 4 ++-- router/peers.go | 2 +- router/router.go | 6 +++--- 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/router/connection_maker.go b/router/connection_maker.go index 67a002d2..8f2045b1 100644 --- a/router/connection_maker.go +++ b/router/connection_maker.go @@ -18,7 +18,7 @@ type ConnectionMaker struct { ourself *LocalPeer peers *Peers targets map[string]*Target - cmdLineAddress map[string]bool + cmdLineAddress map[string]struct{} actionChan chan<- ConnectionMakerAction } @@ -35,7 +35,7 @@ func NewConnectionMaker(ourself *LocalPeer, peers *Peers) *ConnectionMaker { return &ConnectionMaker{ ourself: ourself, peers: peers, - cmdLineAddress: make(map[string]bool), + cmdLineAddress: make(map[string]struct{}), targets: make(map[string]*Target)} } @@ -47,7 +47,7 @@ func (cm *ConnectionMaker) Start() { func (cm *ConnectionMaker) InitiateConnection(address string) { cm.actionChan <- func() bool { - cm.cmdLineAddress[NormalisePeerAddr(address)] = true + cm.cmdLineAddress[NormalisePeerAddr(address)] = struct{}{} if target, found := cm.targets[address]; found { target.tryAfter, target.tryInterval = tryImmediately() } @@ -111,19 +111,19 @@ func (cm *ConnectionMaker) queryLoop(actionChan <-chan ConnectionMakerAction) { } func (cm *ConnectionMaker) checkStateAndAttemptConnections() time.Duration { - validTarget := make(map[string]bool) + validTarget := make(map[string]struct{}) // copy the set of things we are connected to, so we can access them without locking - ourConnectedPeers := make(map[PeerName]bool) - ourConnectedTargets := make(map[string]bool) + ourConnectedPeers := make(map[PeerName]struct{}) + ourConnectedTargets := make(map[string]struct{}) for _, conn := range cm.ourself.Connections() { - ourConnectedPeers[conn.Remote().Name] = true - ourConnectedTargets[conn.RemoteTCPAddr()] = true + ourConnectedPeers[conn.Remote().Name] = struct{}{} + ourConnectedTargets[conn.RemoteTCPAddr()] = struct{}{} } addTarget := func(address string) { - if !ourConnectedTargets[address] { - validTarget[address] = true + if _, connected := ourConnectedTargets[address]; !connected { + validTarget[address] = struct{}{} cm.addTarget(address) } } @@ -138,7 +138,10 @@ func (cm *ConnectionMaker) checkStateAndAttemptConnections() time.Duration { cm.peers.ForEach(func(peer *Peer) { for _, conn := range peer.Connections() { otherPeer := conn.Remote().Name - if otherPeer == cm.ourself.Name || ourConnectedPeers[otherPeer] { + if otherPeer == cm.ourself.Name { + continue + } + if _, connected := ourConnectedPeers[otherPeer]; connected { continue } address := conn.RemoteTCPAddr() @@ -163,25 +166,26 @@ func (cm *ConnectionMaker) addTarget(address string) { } } -func (cm *ConnectionMaker) connectToTargets(validTarget map[string]bool, ourConnectedTargets map[string]bool) time.Duration { +func (cm *ConnectionMaker) connectToTargets(validTarget map[string]struct{}, ourConnectedTargets map[string]struct{}) time.Duration { now := time.Now() // make sure we catch items just added after := MaxDuration for address, target := range cm.targets { - if ourConnectedTargets[address] { + if _, connected := ourConnectedTargets[address]; connected { delete(cm.targets, address) continue } if target.attempting { continue } - if !validTarget[address] { + if _, valid := validTarget[address]; !valid { delete(cm.targets, address) continue } switch duration := target.tryAfter.Sub(now); { case duration <= 0: target.attempting = true - go cm.attemptConnection(address, cm.cmdLineAddress[address]) + _, isCmdLineAddress := cm.cmdLineAddress[address] + go cm.attemptConnection(address, isCmdLineAddress) case duration < after: after = duration } diff --git a/router/mocks_test.go b/router/mocks_test.go index 3d5c9178..d71aa280 100644 --- a/router/mocks_test.go +++ b/router/mocks_test.go @@ -48,9 +48,9 @@ func newMockConnection(from, to *Peer) Connection { } func checkEqualConns(t *testing.T, ourName PeerName, got, wanted map[PeerName]Connection) { - checkConns := make(map[PeerName]bool) + checkConns := make(map[PeerName]struct{}) for _, conn := range wanted { - checkConns[conn.Remote().Name] = true + checkConns[conn.Remote().Name] = struct{}{} } for _, conn := range got { remoteName := conn.Remote().Name diff --git a/router/peers.go b/router/peers.go index 5edf5639..a0ff2cbe 100644 --- a/router/peers.go +++ b/router/peers.go @@ -169,7 +169,7 @@ func (peers *Peers) garbageCollect() []*Peer { func setFromPeersMap(peers map[PeerName]*Peer) PeerNameSet { names := make(PeerNameSet) for name := range peers { - names[name] = true + names[name] = struct{}{} } return names } diff --git a/router/router.go b/router/router.go index af245e8e..297eca08 100644 --- a/router/router.go +++ b/router/router.go @@ -350,7 +350,7 @@ func handleSpecialFrame(relayConn *LocalConnection, sender *net.UDPAddr, frame [ // Gossiper methods - the Router is the topology Gossiper -type PeerNameSet map[PeerName]bool +type PeerNameSet map[PeerName]struct{} type TopologyGossipData struct { peers *Peers @@ -360,14 +360,14 @@ type TopologyGossipData struct { func NewTopologyGossipData(peers *Peers, update ...*Peer) *TopologyGossipData { names := make(PeerNameSet) for _, p := range update { - names[p.Name] = true + names[p.Name] = struct{}{} } return &TopologyGossipData{peers: peers, update: names} } func (d *TopologyGossipData) Merge(other GossipData) { for name := range other.(*TopologyGossipData).update { - d.update[name] = true + d.update[name] = struct{}{} } } From 6db12d682e77163546c9a513b08faa369d7021f1 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 6 Apr 2015 11:45:17 +0100 Subject: [PATCH 2/3] make PeerNameSet a more broadly used type --- router/connection_maker.go | 2 +- router/mocks_test.go | 2 +- router/peers.go | 2 ++ router/router.go | 2 -- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/router/connection_maker.go b/router/connection_maker.go index 8f2045b1..349098be 100644 --- a/router/connection_maker.go +++ b/router/connection_maker.go @@ -114,7 +114,7 @@ func (cm *ConnectionMaker) checkStateAndAttemptConnections() time.Duration { validTarget := make(map[string]struct{}) // copy the set of things we are connected to, so we can access them without locking - ourConnectedPeers := make(map[PeerName]struct{}) + ourConnectedPeers := make(PeerNameSet) ourConnectedTargets := make(map[string]struct{}) for _, conn := range cm.ourself.Connections() { ourConnectedPeers[conn.Remote().Name] = struct{}{} diff --git a/router/mocks_test.go b/router/mocks_test.go index d71aa280..82016bc2 100644 --- a/router/mocks_test.go +++ b/router/mocks_test.go @@ -48,7 +48,7 @@ func newMockConnection(from, to *Peer) Connection { } func checkEqualConns(t *testing.T, ourName PeerName, got, wanted map[PeerName]Connection) { - checkConns := make(map[PeerName]struct{}) + checkConns := make(PeerNameSet) for _, conn := range wanted { checkConns[conn.Remote().Name] = struct{}{} } diff --git a/router/peers.go b/router/peers.go index a0ff2cbe..1e9d7eca 100644 --- a/router/peers.go +++ b/router/peers.go @@ -23,6 +23,8 @@ type NameCollisionError struct { Name PeerName } +type PeerNameSet map[PeerName]struct{} + func NewPeers(ourself *Peer, onGC func(*Peer)) *Peers { return &Peers{ ourself: ourself, diff --git a/router/router.go b/router/router.go index 297eca08..6e22b6d4 100644 --- a/router/router.go +++ b/router/router.go @@ -350,8 +350,6 @@ func handleSpecialFrame(relayConn *LocalConnection, sender *net.UDPAddr, frame [ // Gossiper methods - the Router is the topology Gossiper -type PeerNameSet map[PeerName]struct{} - type TopologyGossipData struct { peers *Peers update PeerNameSet From e8e9a43abdbd5dbdc609015728bd658cf757450a Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 6 Apr 2015 11:53:51 +0100 Subject: [PATCH 3/3] tired of typing "struct{}{}" --- router/connection_maker.go | 8 ++++---- router/mocks_test.go | 2 +- router/peers.go | 2 +- router/router.go | 4 ++-- router/utils.go | 2 ++ 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/router/connection_maker.go b/router/connection_maker.go index 349098be..50d65cfb 100644 --- a/router/connection_maker.go +++ b/router/connection_maker.go @@ -47,7 +47,7 @@ func (cm *ConnectionMaker) Start() { func (cm *ConnectionMaker) InitiateConnection(address string) { cm.actionChan <- func() bool { - cm.cmdLineAddress[NormalisePeerAddr(address)] = struct{}{} + cm.cmdLineAddress[NormalisePeerAddr(address)] = void if target, found := cm.targets[address]; found { target.tryAfter, target.tryInterval = tryImmediately() } @@ -117,13 +117,13 @@ func (cm *ConnectionMaker) checkStateAndAttemptConnections() time.Duration { ourConnectedPeers := make(PeerNameSet) ourConnectedTargets := make(map[string]struct{}) for _, conn := range cm.ourself.Connections() { - ourConnectedPeers[conn.Remote().Name] = struct{}{} - ourConnectedTargets[conn.RemoteTCPAddr()] = struct{}{} + ourConnectedPeers[conn.Remote().Name] = void + ourConnectedTargets[conn.RemoteTCPAddr()] = void } addTarget := func(address string) { if _, connected := ourConnectedTargets[address]; !connected { - validTarget[address] = struct{}{} + validTarget[address] = void cm.addTarget(address) } } diff --git a/router/mocks_test.go b/router/mocks_test.go index 82016bc2..3e10bc0c 100644 --- a/router/mocks_test.go +++ b/router/mocks_test.go @@ -50,7 +50,7 @@ func newMockConnection(from, to *Peer) Connection { func checkEqualConns(t *testing.T, ourName PeerName, got, wanted map[PeerName]Connection) { checkConns := make(PeerNameSet) for _, conn := range wanted { - checkConns[conn.Remote().Name] = struct{}{} + checkConns[conn.Remote().Name] = void } for _, conn := range got { remoteName := conn.Remote().Name diff --git a/router/peers.go b/router/peers.go index 1e9d7eca..31a8cc1f 100644 --- a/router/peers.go +++ b/router/peers.go @@ -171,7 +171,7 @@ func (peers *Peers) garbageCollect() []*Peer { func setFromPeersMap(peers map[PeerName]*Peer) PeerNameSet { names := make(PeerNameSet) for name := range peers { - names[name] = struct{}{} + names[name] = void } return names } diff --git a/router/router.go b/router/router.go index 6e22b6d4..7ef3a9a2 100644 --- a/router/router.go +++ b/router/router.go @@ -358,14 +358,14 @@ type TopologyGossipData struct { func NewTopologyGossipData(peers *Peers, update ...*Peer) *TopologyGossipData { names := make(PeerNameSet) for _, p := range update { - names[p.Name] = struct{}{} + names[p.Name] = void } return &TopologyGossipData{peers: peers, update: names} } func (d *TopologyGossipData) Merge(other GossipData) { for name := range other.(*TopologyGossipData).update { - d.update[name] = struct{}{} + d.update[name] = void } } diff --git a/router/utils.go b/router/utils.go index 3153db7f..1545b23d 100644 --- a/router/utils.go +++ b/router/utils.go @@ -10,6 +10,8 @@ import ( "net" ) +var void = struct{}{} + func checkFatal(e error) { if e != nil { log.Fatal(e)