From b1621332cc6d5860f58a7526088569e92af05ae0 Mon Sep 17 00:00:00 2001
From: David Hill <dhill@mindcry.org>
Date: Tue, 12 Apr 2016 21:19:13 -0400
Subject: [PATCH] Optimize by removing defers

defer's are nice for readability but they do add overhead.  This
gets rid of defer's where it is just as easy as not to use one.
---
 blockchain/chain.go |  13 +++---
 mempool/mempool.go  |  49 ++++++++++-----------
 peer/mruinvmap.go   |  11 ++---
 peer/mrunoncemap.go |  11 ++---
 peer/peer.go        | 103 ++++++++++++++++++++++++++------------------
 rpcwebsocket.go     |   5 ++-
 server.go           |   5 ++-
 7 files changed, 104 insertions(+), 93 deletions(-)

diff --git a/blockchain/chain.go b/blockchain/chain.go
index 56717aaf..1094fea7 100644
--- a/blockchain/chain.go
+++ b/blockchain/chain.go
@@ -245,9 +245,9 @@ func (b *BlockChain) DisableVerify(disable bool) {
 // This function is safe for concurrent access.
 func (b *BlockChain) HaveBlock(hash *chainhash.Hash) (bool, error) {
 	b.chainLock.RLock()
-	defer b.chainLock.RUnlock()
-
 	exists, err := b.blockExists(hash)
+	b.chainLock.RUnlock()
+
 	if err != nil {
 		return false, err
 	}
@@ -268,13 +268,10 @@ func (b *BlockChain) IsKnownOrphan(hash *chainhash.Hash) bool {
 	// Protect concurrent access.  Using a read lock only so multiple
 	// readers can query without blocking each other.
 	b.orphanLock.RLock()
-	defer b.orphanLock.RUnlock()
+	_, exists := b.orphans[*hash]
+	b.orphanLock.RUnlock()
 
-	if _, exists := b.orphans[*hash]; exists {
-		return true
-	}
-
-	return false
+	return exists
 }
 
 // GetOrphanRoot returns the head of the chain for the provided hash from the
diff --git a/mempool/mempool.go b/mempool/mempool.go
index 81399640..9d232897 100644
--- a/mempool/mempool.go
+++ b/mempool/mempool.go
@@ -284,9 +284,10 @@ func (mp *TxPool) isTransactionInPool(hash *chainhash.Hash) bool {
 func (mp *TxPool) IsTransactionInPool(hash *chainhash.Hash) bool {
 	// Protect concurrent access.
 	mp.mtx.RLock()
-	defer mp.mtx.RUnlock()
+	inPool := mp.isTransactionInPool(hash)
+	mp.mtx.RUnlock()
 
-	return mp.isTransactionInPool(hash)
+	return inPool
 }
 
 // isOrphanInPool returns whether or not the passed transaction already exists
@@ -308,9 +309,10 @@ func (mp *TxPool) isOrphanInPool(hash *chainhash.Hash) bool {
 func (mp *TxPool) IsOrphanInPool(hash *chainhash.Hash) bool {
 	// Protect concurrent access.
 	mp.mtx.RLock()
-	defer mp.mtx.RUnlock()
+	inPool := mp.isOrphanInPool(hash)
+	mp.mtx.RUnlock()
 
-	return mp.isOrphanInPool(hash)
+	return inPool
 }
 
 // haveTransaction returns whether or not the passed transaction already exists
@@ -328,9 +330,10 @@ func (mp *TxPool) haveTransaction(hash *chainhash.Hash) bool {
 func (mp *TxPool) HaveTransaction(hash *chainhash.Hash) bool {
 	// Protect concurrent access.
 	mp.mtx.RLock()
-	defer mp.mtx.RUnlock()
+	haveTx := mp.haveTransaction(hash)
+	mp.mtx.RUnlock()
 
-	return mp.haveTransaction(hash)
+	return haveTx
 }
 
 // removeTransaction is the internal function which implements the public
@@ -375,9 +378,8 @@ func (mp *TxPool) removeTransaction(tx *btcutil.Tx, removeRedeemers bool) {
 func (mp *TxPool) RemoveTransaction(tx *btcutil.Tx, removeRedeemers bool) {
 	// Protect concurrent access.
 	mp.mtx.Lock()
-	defer mp.mtx.Unlock()
-
 	mp.removeTransaction(tx, removeRedeemers)
+	mp.mtx.Unlock()
 }
 
 // RemoveDoubleSpends removes all transactions which spend outputs spent by the
@@ -390,8 +392,6 @@ func (mp *TxPool) RemoveTransaction(tx *btcutil.Tx, removeRedeemers bool) {
 func (mp *TxPool) RemoveDoubleSpends(tx *btcutil.Tx) {
 	// Protect concurrent access.
 	mp.mtx.Lock()
-	defer mp.mtx.Unlock()
-
 	for _, txIn := range tx.MsgTx().TxIn {
 		if txRedeemer, ok := mp.outpoints[txIn.PreviousOutPoint]; ok {
 			if !txRedeemer.Hash().IsEqual(tx.Hash()) {
@@ -399,6 +399,7 @@ func (mp *TxPool) RemoveDoubleSpends(tx *btcutil.Tx) {
 			}
 		}
 	}
+	mp.mtx.Unlock()
 }
 
 // addTransaction adds the passed transaction to the memory pool.  It should
@@ -482,9 +483,10 @@ func (mp *TxPool) fetchInputUtxos(tx *btcutil.Tx) (*blockchain.UtxoViewpoint, er
 func (mp *TxPool) FetchTransaction(txHash *chainhash.Hash) (*btcutil.Tx, error) {
 	// Protect concurrent access.
 	mp.mtx.RLock()
-	defer mp.mtx.RUnlock()
+	txDesc, exists := mp.pool[*txHash]
+	mp.mtx.RUnlock()
 
-	if txDesc, exists := mp.pool[*txHash]; exists {
+	if exists {
 		return txDesc.Tx, nil
 	}
 
@@ -761,9 +763,10 @@ func (mp *TxPool) maybeAcceptTransaction(tx *btcutil.Tx, isNew, rateLimit bool)
 func (mp *TxPool) MaybeAcceptTransaction(tx *btcutil.Tx, isNew, rateLimit bool) ([]*chainhash.Hash, error) {
 	// Protect concurrent access.
 	mp.mtx.Lock()
-	defer mp.mtx.Unlock()
+	hashes, err := mp.maybeAcceptTransaction(tx, isNew, rateLimit)
+	mp.mtx.Unlock()
 
-	return mp.maybeAcceptTransaction(tx, isNew, rateLimit)
+	return hashes, err
 }
 
 // processOrphans is the internal function which implements the public
@@ -881,12 +884,12 @@ func (mp *TxPool) ProcessOrphans(hash *chainhash.Hash) []*btcutil.Tx {
 //
 // This function is safe for concurrent access.
 func (mp *TxPool) ProcessTransaction(tx *btcutil.Tx, allowOrphan, rateLimit bool) ([]*btcutil.Tx, error) {
+	log.Tracef("Processing transaction %v", tx.Hash())
+
 	// Protect concurrent access.
 	mp.mtx.Lock()
 	defer mp.mtx.Unlock()
 
-	log.Tracef("Processing transaction %v", tx.Hash())
-
 	// Potentially accept the transaction to the memory pool.
 	missingParents, err := mp.maybeAcceptTransaction(tx, true, rateLimit)
 	if err != nil {
@@ -942,9 +945,10 @@ func (mp *TxPool) ProcessTransaction(tx *btcutil.Tx, allowOrphan, rateLimit bool
 // This function is safe for concurrent access.
 func (mp *TxPool) Count() int {
 	mp.mtx.RLock()
-	defer mp.mtx.RUnlock()
+	count := len(mp.pool)
+	mp.mtx.RUnlock()
 
-	return len(mp.pool)
+	return count
 }
 
 // TxHashes returns a slice of hashes for all of the transactions in the memory
@@ -953,8 +957,6 @@ func (mp *TxPool) Count() int {
 // This function is safe for concurrent access.
 func (mp *TxPool) TxHashes() []*chainhash.Hash {
 	mp.mtx.RLock()
-	defer mp.mtx.RUnlock()
-
 	hashes := make([]*chainhash.Hash, len(mp.pool))
 	i := 0
 	for hash := range mp.pool {
@@ -962,6 +964,7 @@ func (mp *TxPool) TxHashes() []*chainhash.Hash {
 		hashes[i] = &hashCopy
 		i++
 	}
+	mp.mtx.RUnlock()
 
 	return hashes
 }
@@ -972,14 +975,13 @@ func (mp *TxPool) TxHashes() []*chainhash.Hash {
 // This function is safe for concurrent access.
 func (mp *TxPool) TxDescs() []*TxDesc {
 	mp.mtx.RLock()
-	defer mp.mtx.RUnlock()
-
 	descs := make([]*TxDesc, len(mp.pool))
 	i := 0
 	for _, desc := range mp.pool {
 		descs[i] = desc
 		i++
 	}
+	mp.mtx.RUnlock()
 
 	return descs
 }
@@ -991,14 +993,13 @@ func (mp *TxPool) TxDescs() []*TxDesc {
 // concurrent access as required by the interface contract.
 func (mp *TxPool) MiningDescs() []*mining.TxDesc {
 	mp.mtx.RLock()
-	defer mp.mtx.RUnlock()
-
 	descs := make([]*mining.TxDesc, len(mp.pool))
 	i := 0
 	for _, desc := range mp.pool {
 		descs[i] = &desc.TxDesc
 		i++
 	}
+	mp.mtx.RUnlock()
 
 	return descs
 }
diff --git a/peer/mruinvmap.go b/peer/mruinvmap.go
index 89d4a841..ecbfc042 100644
--- a/peer/mruinvmap.go
+++ b/peer/mruinvmap.go
@@ -50,12 +50,10 @@ func (m *mruInventoryMap) String() string {
 // This function is safe for concurrent access.
 func (m *mruInventoryMap) Exists(iv *wire.InvVect) bool {
 	m.invMtx.Lock()
-	defer m.invMtx.Unlock()
+	_, exists := m.invMap[*iv]
+	m.invMtx.Unlock()
 
-	if _, exists := m.invMap[*iv]; exists {
-		return true
-	}
-	return false
+	return exists
 }
 
 // Add adds the passed inventory to the map and handles eviction of the oldest
@@ -109,12 +107,11 @@ func (m *mruInventoryMap) Add(iv *wire.InvVect) {
 // This function is safe for concurrent access.
 func (m *mruInventoryMap) Delete(iv *wire.InvVect) {
 	m.invMtx.Lock()
-	defer m.invMtx.Unlock()
-
 	if node, exists := m.invMap[*iv]; exists {
 		m.invList.Remove(node)
 		delete(m.invMap, *iv)
 	}
+	m.invMtx.Unlock()
 }
 
 // newMruInventoryMap returns a new inventory map that is limited to the number
diff --git a/peer/mrunoncemap.go b/peer/mrunoncemap.go
index 1bf54429..a830e7a2 100644
--- a/peer/mrunoncemap.go
+++ b/peer/mrunoncemap.go
@@ -48,12 +48,10 @@ func (m *mruNonceMap) String() string {
 // This function is safe for concurrent access.
 func (m *mruNonceMap) Exists(nonce uint64) bool {
 	m.mtx.Lock()
-	defer m.mtx.Unlock()
+	_, exists := m.nonceMap[nonce]
+	m.mtx.Unlock()
 
-	if _, exists := m.nonceMap[nonce]; exists {
-		return true
-	}
-	return false
+	return exists
 }
 
 // Add adds the passed nonce to the map and handles eviction of the oldest item
@@ -107,12 +105,11 @@ func (m *mruNonceMap) Add(nonce uint64) {
 // This function is safe for concurrent access.
 func (m *mruNonceMap) Delete(nonce uint64) {
 	m.mtx.Lock()
-	defer m.mtx.Unlock()
-
 	if node, exists := m.nonceMap[nonce]; exists {
 		m.nonceList.Remove(node)
 		delete(m.nonceMap, nonce)
 	}
+	m.mtx.Unlock()
 }
 
 // newMruNonceMap returns a new nonce map that is limited to the number of
diff --git a/peer/peer.go b/peer/peer.go
index f6b86417..50ff1020 100644
--- a/peer/peer.go
+++ b/peer/peer.go
@@ -485,7 +485,6 @@ func (p *Peer) AddKnownInventory(invVect *wire.InvVect) {
 // This function is safe for concurrent access.
 func (p *Peer) StatsSnapshot() *StatsSnap {
 	p.statsMtx.RLock()
-	defer p.statsMtx.RUnlock()
 
 	p.flagsMtx.Lock()
 	id := p.id
@@ -496,7 +495,7 @@ func (p *Peer) StatsSnapshot() *StatsSnap {
 	p.flagsMtx.Unlock()
 
 	// Get a copy of all relevant flags and stats.
-	return &StatsSnap{
+	statsSnap := &StatsSnap{
 		ID:             id,
 		Addr:           addr,
 		UserAgent:      userAgent,
@@ -515,6 +514,9 @@ func (p *Peer) StatsSnapshot() *StatsSnap {
 		LastPingMicros: p.lastPingMicros,
 		LastPingTime:   p.lastPingTime,
 	}
+
+	p.statsMtx.RUnlock()
+	return statsSnap
 }
 
 // ID returns the peer id.
@@ -522,9 +524,10 @@ func (p *Peer) StatsSnapshot() *StatsSnap {
 // This function is safe for concurrent access.
 func (p *Peer) ID() int32 {
 	p.flagsMtx.Lock()
-	defer p.flagsMtx.Unlock()
+	id := p.id
+	p.flagsMtx.Unlock()
 
-	return p.id
+	return id
 }
 
 // NA returns the peer network address.
@@ -532,9 +535,10 @@ func (p *Peer) ID() int32 {
 // This function is safe for concurrent access.
 func (p *Peer) NA() *wire.NetAddress {
 	p.flagsMtx.Lock()
-	defer p.flagsMtx.Unlock()
+	na := p.na
+	p.flagsMtx.Unlock()
 
-	return p.na
+	return na
 }
 
 // Addr returns the peer address.
@@ -558,9 +562,10 @@ func (p *Peer) Inbound() bool {
 // This function is safe for concurrent access.
 func (p *Peer) Services() wire.ServiceFlag {
 	p.flagsMtx.Lock()
-	defer p.flagsMtx.Unlock()
+	services := p.services
+	p.flagsMtx.Unlock()
 
-	return p.services
+	return services
 }
 
 // UserAgent returns the user agent of the remote peer.
@@ -568,9 +573,10 @@ func (p *Peer) Services() wire.ServiceFlag {
 // This function is safe for concurrent access.
 func (p *Peer) UserAgent() string {
 	p.flagsMtx.Lock()
-	defer p.flagsMtx.Unlock()
+	userAgent := p.userAgent
+	p.flagsMtx.Unlock()
 
-	return p.userAgent
+	return userAgent
 }
 
 // LastAnnouncedBlock returns the last announced block of the remote peer.
@@ -578,9 +584,10 @@ func (p *Peer) UserAgent() string {
 // This function is safe for concurrent access.
 func (p *Peer) LastAnnouncedBlock() *chainhash.Hash {
 	p.statsMtx.RLock()
-	defer p.statsMtx.RUnlock()
+	lastAnnouncedBlock := p.lastAnnouncedBlock
+	p.statsMtx.RUnlock()
 
-	return p.lastAnnouncedBlock
+	return lastAnnouncedBlock
 }
 
 // LastPingNonce returns the last ping nonce of the remote peer.
@@ -588,9 +595,10 @@ func (p *Peer) LastAnnouncedBlock() *chainhash.Hash {
 // This function is safe for concurrent access.
 func (p *Peer) LastPingNonce() uint64 {
 	p.statsMtx.RLock()
-	defer p.statsMtx.RUnlock()
+	lastPingNonce := p.lastPingNonce
+	p.statsMtx.RUnlock()
 
-	return p.lastPingNonce
+	return lastPingNonce
 }
 
 // LastPingTime returns the last ping time of the remote peer.
@@ -598,9 +606,10 @@ func (p *Peer) LastPingNonce() uint64 {
 // This function is safe for concurrent access.
 func (p *Peer) LastPingTime() time.Time {
 	p.statsMtx.RLock()
-	defer p.statsMtx.RUnlock()
+	lastPingTime := p.lastPingTime
+	p.statsMtx.RUnlock()
 
-	return p.lastPingTime
+	return lastPingTime
 }
 
 // LastPingMicros returns the last ping micros of the remote peer.
@@ -608,9 +617,10 @@ func (p *Peer) LastPingTime() time.Time {
 // This function is safe for concurrent access.
 func (p *Peer) LastPingMicros() int64 {
 	p.statsMtx.RLock()
-	defer p.statsMtx.RUnlock()
+	lastPingMicros := p.lastPingMicros
+	p.statsMtx.RUnlock()
 
-	return p.lastPingMicros
+	return lastPingMicros
 }
 
 // VersionKnown returns the whether or not the version of a peer is known
@@ -619,9 +629,10 @@ func (p *Peer) LastPingMicros() int64 {
 // This function is safe for concurrent access.
 func (p *Peer) VersionKnown() bool {
 	p.flagsMtx.Lock()
-	defer p.flagsMtx.Unlock()
+	versionKnown := p.versionKnown
+	p.flagsMtx.Unlock()
 
-	return p.versionKnown
+	return versionKnown
 }
 
 // VerAckReceived returns whether or not a verack message was received by the
@@ -630,9 +641,10 @@ func (p *Peer) VersionKnown() bool {
 // This function is safe for concurrent access.
 func (p *Peer) VerAckReceived() bool {
 	p.flagsMtx.Lock()
-	defer p.flagsMtx.Unlock()
+	verAckReceived := p.verAckReceived
+	p.flagsMtx.Unlock()
 
-	return p.verAckReceived
+	return verAckReceived
 }
 
 // ProtocolVersion returns the peer protocol version.
@@ -640,9 +652,10 @@ func (p *Peer) VerAckReceived() bool {
 // This function is safe for concurrent access.
 func (p *Peer) ProtocolVersion() uint32 {
 	p.flagsMtx.Lock()
-	defer p.flagsMtx.Unlock()
+	protocolVersion := p.protocolVersion
+	p.flagsMtx.Unlock()
 
-	return p.protocolVersion
+	return protocolVersion
 }
 
 // LastBlock returns the last block of the peer.
@@ -650,9 +663,10 @@ func (p *Peer) ProtocolVersion() uint32 {
 // This function is safe for concurrent access.
 func (p *Peer) LastBlock() int32 {
 	p.statsMtx.RLock()
-	defer p.statsMtx.RUnlock()
+	lastBlock := p.lastBlock
+	p.statsMtx.RUnlock()
 
-	return p.lastBlock
+	return lastBlock
 }
 
 // LastSend returns the last send time of the peer.
@@ -688,9 +702,10 @@ func (p *Peer) BytesReceived() uint64 {
 // This function is safe for concurrent access.
 func (p *Peer) TimeConnected() time.Time {
 	p.statsMtx.RLock()
-	defer p.statsMtx.RUnlock()
+	timeConnected := p.timeConnected
+	p.statsMtx.RUnlock()
 
-	return p.timeConnected
+	return timeConnected
 }
 
 // TimeOffset returns the number of seconds the local time was offset from the
@@ -700,9 +715,10 @@ func (p *Peer) TimeConnected() time.Time {
 // This function is safe for concurrent access.
 func (p *Peer) TimeOffset() int64 {
 	p.statsMtx.RLock()
-	defer p.statsMtx.RUnlock()
+	timeOffset := p.timeOffset
+	p.statsMtx.RUnlock()
 
-	return p.timeOffset
+	return timeOffset
 }
 
 // StartingHeight returns the last known height the peer reported during the
@@ -711,9 +727,10 @@ func (p *Peer) TimeOffset() int64 {
 // This function is safe for concurrent access.
 func (p *Peer) StartingHeight() int32 {
 	p.statsMtx.RLock()
-	defer p.statsMtx.RUnlock()
+	startingHeight := p.startingHeight
+	p.statsMtx.RUnlock()
 
-	return p.startingHeight
+	return startingHeight
 }
 
 // WantsHeaders returns if the peer wants header messages instead of
@@ -722,9 +739,10 @@ func (p *Peer) StartingHeight() int32 {
 // This function is safe for concurrent access.
 func (p *Peer) WantsHeaders() bool {
 	p.flagsMtx.Lock()
-	defer p.flagsMtx.Unlock()
+	sendHeadersPreferred := p.sendHeadersPreferred
+	p.flagsMtx.Unlock()
 
-	return p.sendHeadersPreferred
+	return sendHeadersPreferred
 }
 
 // localVersionMsg creates a version message that can be used to send to the
@@ -1029,9 +1047,6 @@ func (p *Peer) handlePingMsg(msg *wire.MsgPing) {
 // version > BIP0031Version).  There is no effect for older clients or when a
 // ping was not previously sent.
 func (p *Peer) handlePongMsg(msg *wire.MsgPong) {
-	p.statsMtx.Lock()
-	defer p.statsMtx.Unlock()
-
 	// Arguably we could use a buffered channel here sending data
 	// in a fifo manner whenever we send a ping, or a list keeping track of
 	// the times of each ping. For now we just make a best effort and
@@ -1039,12 +1054,14 @@ func (p *Peer) handlePongMsg(msg *wire.MsgPong) {
 	// and overlapping pings will be ignored. It is unlikely to occur
 	// without large usage of the ping rpc call since we ping infrequently
 	// enough that if they overlap we would have timed out the peer.
-	if p.ProtocolVersion() > wire.BIP0031Version && p.lastPingNonce != 0 &&
-		msg.Nonce == p.lastPingNonce {
-
-		p.lastPingMicros = time.Now().Sub(p.lastPingTime).Nanoseconds()
-		p.lastPingMicros /= 1000 // convert to usec.
-		p.lastPingNonce = 0
+	if p.ProtocolVersion() > wire.BIP0031Version {
+		p.statsMtx.Lock()
+		if p.lastPingNonce != 0 && msg.Nonce == p.lastPingNonce {
+			p.lastPingMicros = time.Now().Sub(p.lastPingTime).Nanoseconds()
+			p.lastPingMicros /= 1000 // convert to usec.
+			p.lastPingNonce = 0
+		}
+		p.statsMtx.Unlock()
 	}
 }
 
diff --git a/rpcwebsocket.go b/rpcwebsocket.go
index 2c6b541c..95fcfe65 100644
--- a/rpcwebsocket.go
+++ b/rpcwebsocket.go
@@ -1352,9 +1352,10 @@ func (c *wsClient) QueueNotification(marshalledJSON []byte) error {
 // Disconnected returns whether or not the websocket client is disconnected.
 func (c *wsClient) Disconnected() bool {
 	c.Lock()
-	defer c.Unlock()
+	isDisconnected := c.disconnected
+	c.Unlock()
 
-	return c.disconnected
+	return isDisconnected
 }
 
 // Disconnect disconnects the websocket client.
diff --git a/server.go b/server.go
index d8fadbaf..fc346c3b 100644
--- a/server.go
+++ b/server.go
@@ -285,9 +285,10 @@ func (sp *serverPeer) setDisableRelayTx(disable bool) {
 // It is safe for concurrent access.
 func (sp *serverPeer) relayTxDisabled() bool {
 	sp.relayMtx.Lock()
-	defer sp.relayMtx.Unlock()
+	isDisabled := sp.disableRelayTx
+	sp.relayMtx.Unlock()
 
-	return sp.disableRelayTx
+	return isDisabled
 }
 
 // pushAddrMsg sends an addr message to the connected peer using the provided