From b01a68d74bcf1c1d300f6b23c4061a15630b4a48 Mon Sep 17 00:00:00 2001 From: Ryan Schneider Date: Tue, 22 May 2018 07:48:43 -0700 Subject: [PATCH] core: Remove redundant `txLookup.Find` and improve comments on txLookup methods. --- core/tx_list.go | 6 +++--- core/tx_pool.go | 30 +++++++++++++++--------------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/core/tx_list.go b/core/tx_list.go index 90514fffed4c..287dda4c33bf 100644 --- a/core/tx_list.go +++ b/core/tx_list.go @@ -444,7 +444,7 @@ func (l *txPricedList) Cap(threshold *big.Int, local *accountSet) types.Transact for len(*l.items) > 0 { // Discard stale transactions if found during cleanup tx := heap.Pop(l.items).(*types.Transaction) - if _, ok := l.all.Find(tx.Hash()); !ok { + if l.all.Get(tx.Hash()) == nil { l.stales-- continue } @@ -476,7 +476,7 @@ func (l *txPricedList) Underpriced(tx *types.Transaction, local *accountSet) boo // Discard stale price points if found at the heap start for len(*l.items) > 0 { head := []*types.Transaction(*l.items)[0] - if _, ok := l.all.Find(head.Hash()); !ok { + if l.all.Get(head.Hash()) == nil { l.stales-- heap.Pop(l.items) continue @@ -501,7 +501,7 @@ func (l *txPricedList) Discard(count int, local *accountSet) types.Transactions for len(*l.items) > 0 && count > 0 { // Discard stale transactions if found during cleanup tx := heap.Pop(l.items).(*types.Transaction) - if _, ok := l.all.Find(tx.Hash()); !ok { + if l.all.Get(tx.Hash()) == nil { l.stales-- continue } diff --git a/core/tx_pool.go b/core/tx_pool.go index 1b08673109d4..de0c1b5298d0 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -1160,19 +1160,28 @@ func (as *accountSet) add(addr common.Address) { as.accounts[addr] = struct{}{} } -// txLookup is used to track all transactions to allow lookups without contention +// txLookup is used internally by TxPool to track transactions while allowing lookup without +// mutex contention. +// +// Note, although this type is properly protected against concurrent access, it +// is **not** a type that should ever be mutated or even exposed outside of the +// transaction pool, since its internal state is tightly coupled with the pools +// internal mechanisms. The sole purpose of the type is to permit out-of-bound +// peeking into the pool in TxPool.Get without having to acquire the widely scoped +// TxPool.mu mutex. type txLookup struct { all map[common.Hash]*types.Transaction lock sync.RWMutex } +// newTxLookup returns a new txLookup structure. func newTxLookup() *txLookup { return &txLookup{ all: make(map[common.Hash]*types.Transaction), } } -// calls f on each key and value present in the map +// Range calls f on each key and value present in the map. func (t *txLookup) Range(f func(hash common.Hash, tx *types.Transaction) bool) { t.lock.RLock() defer t.lock.RUnlock() @@ -1184,7 +1193,7 @@ func (t *txLookup) Range(f func(hash common.Hash, tx *types.Transaction) bool) { } } -// returns a transaction if it exists in the lookup, or nil if not found +// Get returns a transaction if it exists in the lookup, or nil if not found. func (t *txLookup) Get(hash common.Hash) *types.Transaction { t.lock.RLock() defer t.lock.RUnlock() @@ -1192,16 +1201,7 @@ func (t *txLookup) Get(hash common.Hash) *types.Transaction { return t.all[hash] } -// returns a transaction if it exists in the lookup, and a bool indicating if it was found -func (t *txLookup) Find(hash common.Hash) (*types.Transaction, bool) { - t.lock.RLock() - defer t.lock.RUnlock() - - value, ok := t.all[hash] - return value, ok -} - -// returns the current number of items in the lookup +// Count returns the current number of items in the lookup. func (t *txLookup) Count() int { t.lock.RLock() defer t.lock.RUnlock() @@ -1209,7 +1209,7 @@ func (t *txLookup) Count() int { return len(t.all) } -// add a transaction to the lookup +// Add adds a transaction to the lookup. func (t *txLookup) Add(tx *types.Transaction) { t.lock.Lock() defer t.lock.Unlock() @@ -1217,7 +1217,7 @@ func (t *txLookup) Add(tx *types.Transaction) { t.all[tx.Hash()] = tx } -// remove a transaction from the lookup +// Remove removes a transaction from the lookup. func (t *txLookup) Remove(hash common.Hash) { t.lock.Lock() defer t.lock.Unlock()