Merge PR #2143: Speedup IAVL iterator by removing defers when unneeded.

PENDING.md

@@ -63,6 +63,7 @@ IMPROVEMENTS
 * SDK
     * [tools] Make get_vendor_deps deletes `.vendor-new` directories, in case scratch files are present.
     * [cli] \#1632 Add integration tests to ensure `basecoind init && basecoind` start sequences run successfully for both `democoin` and `basecoin` examples.
+    * [store] Speedup IAVL iteration, and consequently everything that requires IAVL iteration. [#2143](https://github.com/cosmos/cosmos-sdk/issues/2143)
     * [simulation] Make timestamps randomized [#2153](https://github.com/cosmos/cosmos-sdk/pull/2153)
 
 * Tendermint

store/iavlstore.go

@@ -332,39 +332,42 @@ func (iter *iavlIterator) Domain() (start, end []byte) {
 func (iter *iavlIterator) Valid() bool {
 	iter.waitInit()
 	iter.mtx.Lock()
-	defer iter.mtx.Unlock()
 
-	return !iter.invalid
+	validity := !iter.invalid
+	iter.mtx.Unlock()
+	return validity
 }
 
 // Implements Iterator.
 func (iter *iavlIterator) Next() {
 	iter.waitInit()
 	iter.mtx.Lock()
-	defer iter.mtx.Unlock()
-	iter.assertIsValid()
+	iter.assertIsValid(true)
 
 	iter.receiveNext()
+	iter.mtx.Unlock()
 }
 
 // Implements Iterator.
 func (iter *iavlIterator) Key() []byte {
 	iter.waitInit()
 	iter.mtx.Lock()
-	defer iter.mtx.Unlock()
-	iter.assertIsValid()
+	iter.assertIsValid(true)
 
-	return iter.key
+	key := iter.key
+	iter.mtx.Unlock()
+	return key
 }
 
 // Implements Iterator.
 func (iter *iavlIterator) Value() []byte {
 	iter.waitInit()
 	iter.mtx.Lock()
-	defer iter.mtx.Unlock()
-	iter.assertIsValid()
+	iter.assertIsValid(true)
 
-	return iter.value
+	val := iter.value
+	iter.mtx.Unlock()
+	return val
 }
 
 // Implements Iterator.
@@ -375,14 +378,14 @@ func (iter *iavlIterator) Close() {
 //----------------------------------------
 
 func (iter *iavlIterator) setNext(key, value []byte) {
-	iter.assertIsValid()
+	iter.assertIsValid(false)
 
 	iter.key = key
 	iter.value = value
 }
 
 func (iter *iavlIterator) setInvalid() {
-	iter.assertIsValid()
+	iter.assertIsValid(false)
 
 	iter.invalid = true
 }
@@ -400,8 +403,14 @@ func (iter *iavlIterator) receiveNext() {
 	}
 }
 
-func (iter *iavlIterator) assertIsValid() {
+// assertIsValid panics if the iterator is invalid. If unlockMutex is true,
+// it also unlocks the mutex before panicing, to prevent deadlocks in code that
+// recovers from panics
+func (iter *iavlIterator) assertIsValid(unlockMutex bool) {
 	if iter.invalid {
+		if unlockMutex {
+			iter.mtx.Unlock()
+		}
 		panic("invalid iterator")
 	}
 }

store/iavlstore_test.go

@@ -464,3 +464,26 @@ func TestIAVLStoreQuery(t *testing.T) {
 	require.Equal(t, uint32(sdk.CodeOK), qres.Code)
 	require.Equal(t, v1, qres.Value)
 }
+
+func BenchmarkIAVLIteratorNext(b *testing.B) {
+	db := dbm.NewMemDB()
+	treeSize := 1000
+	tree := iavl.NewVersionedTree(db, cacheSize)
+	for i := 0; i < treeSize; i++ {
+		key := cmn.RandBytes(4)
+		value := cmn.RandBytes(50)
+		tree.Set(key, value)
+	}
+	iavlStore := newIAVLStore(tree, numRecent, storeEvery)
+	iterators := make([]Iterator, b.N/treeSize)
+	for i := 0; i < len(iterators); i++ {
+		iterators[i] = iavlStore.Iterator([]byte{0}, []byte{255, 255, 255, 255, 255})
+	}
+	b.ResetTimer()
+	for i := 0; i < len(iterators); i++ {
+		iter := iterators[i]
+		for j := 0; j < treeSize; j++ {
+			iter.Next()
+		}
+	}
+}