From ef6047635c8d3af7b1e9d60b124d7880a34cacc4 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Mon, 16 Oct 2017 10:30:52 -0700
Subject: [PATCH 1/2] faster, cleaner connection closing

1. Don't ask the system for the time when checking if we should close each
connection (potentially thousands of systemcalls!).
2. Log from outside the lock.
3. Log the event around the entire connection closing operation.
4. Preallocate the slice holding the connections to be closed.
---
 p2p/net/connmgr/connmgr.go | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/p2p/net/connmgr/connmgr.go b/p2p/net/connmgr/connmgr.go
index 610e3bec1d..4305424400 100644
--- a/p2p/net/connmgr/connmgr.go
+++ b/p2p/net/connmgr/connmgr.go
@@ -2,7 +2,6 @@ package connmgr
 
 import (
 	"context"
-	"io"
 	"sort"
 	"sync"
 	"time"
@@ -53,20 +52,23 @@ type peerInfo struct {
 }
 
 func (cm *connManager) TrimOpenConns(ctx context.Context) {
+	defer log.EventBegin(ctx, "connCleanup").Done()
 	for _, c := range cm.getConnsToClose(ctx) {
+		log.Info("closing conn: ", c.RemotePeer())
+		log.Event(ctx, "closeConn", c.RemotePeer())
 		c.Close()
 	}
 }
 
-func (cm *connManager) getConnsToClose(ctx context.Context) []io.Closer {
+func (cm *connManager) getConnsToClose(ctx context.Context) []inet.Conn {
 	cm.lk.Lock()
 	defer cm.lk.Unlock()
-	defer log.EventBegin(ctx, "connCleanup").Done()
 	if cm.lowWater == 0 || cm.highWater == 0 {
 		// disabled
 		return nil
 	}
-	cm.lastTrim = time.Now()
+	now := time.Now()
+	cm.lastTrim = now
 
 	if len(cm.peers) < cm.lowWater {
 		log.Info("open connection count below limit")
@@ -83,20 +85,22 @@ func (cm *connManager) getConnsToClose(ctx context.Context) []io.Closer {
 		return infos[i].value < infos[j].value
 	})
 
-	close_count := len(infos) - cm.lowWater
-	toclose := infos[:close_count]
+	closeCount := len(infos) - cm.lowWater
+	toclose := infos[:closeCount]
 
-	var closed []io.Closer
+	// 2x number of peers we're disconnecting from because we may have more
+	// than one connection per peer. Slightly over allocating isn't an issue
+	// as this is a very short-lived array.
+	closed := make([]inet.Conn, 0, len(toclose)*2)
 
 	for _, inf := range toclose {
-		if time.Since(inf.firstSeen) < cm.gracePeriod {
+		// TODO: should we be using firstSeen or the time associated with the connection itself?
+		if inf.firstSeen.Add(cm.gracePeriod).After(now) {
 			continue
 		}
 
 		// TODO: if a peer has more than one connection, maybe only close one?
-		for c, _ := range inf.conns {
-			log.Info("closing conn: ", c.RemotePeer())
-			log.Event(ctx, "closeConn", c.RemotePeer())
+		for c := range inf.conns {
 			// TODO: probably don't want to always do this in a goroutine
 			closed = append(closed, c)
 		}

From 25616aa1782c1147da53cada8d31ae9dbffd032f Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Mon, 16 Oct 2017 11:41:15 -0700
Subject: [PATCH 2/2] remove the DefaultGracePeriod

It's unused.
---
 p2p/net/connmgr/connmgr.go | 2 --
 1 file changed, 2 deletions(-)

diff --git a/p2p/net/connmgr/connmgr.go b/p2p/net/connmgr/connmgr.go
index 4305424400..cd712b1e3d 100644
--- a/p2p/net/connmgr/connmgr.go
+++ b/p2p/net/connmgr/connmgr.go
@@ -31,8 +31,6 @@ type connManager struct {
 
 var _ ifconnmgr.ConnManager = (*connManager)(nil)
 
-var DefaultGracePeriod = time.Second * 10
-
 func NewConnManager(low, hi int, grace time.Duration) ifconnmgr.ConnManager {
 	return &connManager{
 		highWater:   hi,