From a0f832466c04a44db7cbd0a2ed1f5415b18e398c Mon Sep 17 00:00:00 2001 From: ucwong Date: Mon, 18 Nov 2024 00:04:31 +0800 Subject: [PATCH] tests on the binary iterator --- core/state/snapshot/iterator_binary.go | 99 +++++++++++++++----------- core/state/snapshot/iterator_test.go | 18 ++--- 2 files changed, 68 insertions(+), 49 deletions(-) diff --git a/core/state/snapshot/iterator_binary.go b/core/state/snapshot/iterator_binary.go index db532aa0c3..a29a31072a 100644 --- a/core/state/snapshot/iterator_binary.go +++ b/core/state/snapshot/iterator_binary.go @@ -39,12 +39,12 @@ type binaryIterator struct { // initBinaryAccountIterator creates a simplistic iterator to step over all the // accounts in a slow, but eaily verifiable way. Note this function is used for // initialization, use `newBinaryAccountIterator` as the API. -func (dl *diffLayer) initBinaryAccountIterator() Iterator { +func (dl *diffLayer) initBinaryAccountIterator(seek common.Hash) Iterator { parent, ok := dl.parent.(*diffLayer) if !ok { l := &binaryIterator{ - a: dl.AccountIterator(common.Hash{}), - b: dl.Parent().AccountIterator(common.Hash{}), + a: dl.AccountIterator(seek), + b: dl.Parent().AccountIterator(seek), accountIterator: true, } l.aDone = !l.a.Next() @@ -52,8 +52,8 @@ func (dl *diffLayer) initBinaryAccountIterator() Iterator { return l } l := &binaryIterator{ - a: dl.AccountIterator(common.Hash{}), - b: parent.initBinaryAccountIterator(), + a: dl.AccountIterator(seek), + b: parent.initBinaryAccountIterator(seek), accountIterator: true, } l.aDone = !l.a.Next() @@ -64,12 +64,12 @@ func (dl *diffLayer) initBinaryAccountIterator() Iterator { // initBinaryStorageIterator creates a simplistic iterator to step over all the // storage slots in a slow, but eaily verifiable way. Note this function is used // for initialization, use `newBinaryStorageIterator` as the API. -func (dl *diffLayer) initBinaryStorageIterator(account common.Hash) Iterator { +func (dl *diffLayer) initBinaryStorageIterator(account, seek common.Hash) Iterator { parent, ok := dl.parent.(*diffLayer) if !ok { // If the storage in this layer is already destructed, discard all - // deeper layers but still return an valid single-branch iterator. - a, destructed := dl.StorageIterator(account, common.Hash{}) + // deeper layers but still return a valid single-branch iterator. + a, destructed := dl.StorageIterator(account, seek) if destructed { l := &binaryIterator{ a: a, @@ -81,7 +81,7 @@ func (dl *diffLayer) initBinaryStorageIterator(account common.Hash) Iterator { } // The parent is disk layer, don't need to take care "destructed" // anymore. - b, _ := dl.Parent().StorageIterator(account, common.Hash{}) + b, _ := dl.Parent().StorageIterator(account, seek) l := &binaryIterator{ a: a, b: b, @@ -92,8 +92,8 @@ func (dl *diffLayer) initBinaryStorageIterator(account common.Hash) Iterator { return l } // If the storage in this layer is already destructed, discard all - // deeper layers but still return an valid single-branch iterator. - a, destructed := dl.StorageIterator(account, common.Hash{}) + // deeper layers but still return a valid single-branch iterator. + a, destructed := dl.StorageIterator(account, seek) if destructed { l := &binaryIterator{ a: a, @@ -105,7 +105,7 @@ func (dl *diffLayer) initBinaryStorageIterator(account common.Hash) Iterator { } l := &binaryIterator{ a: a, - b: parent.initBinaryStorageIterator(account), + b: parent.initBinaryStorageIterator(account, seek), account: account, } l.aDone = !l.a.Next() @@ -117,33 +117,50 @@ func (dl *diffLayer) initBinaryStorageIterator(account common.Hash) Iterator { // or an error if iteration failed for some reason (e.g. root being iterated // becomes stale and garbage collected). func (it *binaryIterator) Next() bool { + for { + if !it.next() { + return false + } + if len(it.Account()) != 0 || len(it.Slot()) != 0 { + return true + } + // it.fail might be set if error occurs by calling + // it.Account() or it.Slot(), stop iteration if so. + if it.fail != nil { + return false + } + } +} + +func (it *binaryIterator) next() bool { if it.aDone && it.bDone { return false } -first: - if it.aDone { - it.k = it.b.Hash() + for { + if it.aDone { + it.k = it.b.Hash() + it.bDone = !it.b.Next() + return true + } + if it.bDone { + it.k = it.a.Hash() + it.aDone = !it.a.Next() + return true + } + nextA, nextB := it.a.Hash(), it.b.Hash() + if diff := bytes.Compare(nextA[:], nextB[:]); diff < 0 { + it.aDone = !it.a.Next() + it.k = nextA + return true + } else if diff == 0 { + // Now we need to advance one of them + it.aDone = !it.a.Next() + continue + } it.bDone = !it.b.Next() + it.k = nextB return true } - if it.bDone { - it.k = it.a.Hash() - it.aDone = !it.a.Next() - return true - } - nextA, nextB := it.a.Hash(), it.b.Hash() - if diff := bytes.Compare(nextA[:], nextB[:]); diff < 0 { - it.aDone = !it.a.Next() - it.k = nextA - return true - } else if diff == 0 { - // Now we need to advance one of them - it.aDone = !it.a.Next() - goto first - } - it.bDone = !it.b.Next() - it.k = nextB - return true } // Error returns any failure that occurred during iteration, which might have @@ -195,19 +212,21 @@ func (it *binaryIterator) Slot() []byte { // Release recursively releases all the iterators in the stack. func (it *binaryIterator) Release() { it.a.Release() - it.b.Release() + if it.b != nil { + it.b.Release() + } } // newBinaryAccountIterator creates a simplistic account iterator to step over -// all the accounts in a slow, but eaily verifiable way. -func (dl *diffLayer) newBinaryAccountIterator() AccountIterator { - iter := dl.initBinaryAccountIterator() +// all the accounts in a slow, but easily verifiable way. +func (dl *diffLayer) newBinaryAccountIterator(seek common.Hash) AccountIterator { + iter := dl.initBinaryAccountIterator(seek) return iter.(AccountIterator) } // newBinaryStorageIterator creates a simplistic account iterator to step over -// all the storage slots in a slow, but eaily verifiable way. -func (dl *diffLayer) newBinaryStorageIterator(account common.Hash) StorageIterator { - iter := dl.initBinaryStorageIterator(account) +// all the storage slots in a slow, but easily verifiable way. +func (dl *diffLayer) newBinaryStorageIterator(account, seek common.Hash) StorageIterator { + iter := dl.initBinaryStorageIterator(account, seek) return iter.(StorageIterator) } diff --git a/core/state/snapshot/iterator_test.go b/core/state/snapshot/iterator_test.go index 2227a45353..995b0e181e 100644 --- a/core/state/snapshot/iterator_test.go +++ b/core/state/snapshot/iterator_test.go @@ -177,7 +177,7 @@ func TestAccountIteratorTraversal(t *testing.T) { head := snaps.Snapshot(common.HexToHash("0x04")) verifyIterator(t, 3, head.(snapshot).AccountIterator(common.Hash{})) - verifyIterator(t, 7, head.(*diffLayer).newBinaryAccountIterator()) + verifyIterator(t, 7, head.(*diffLayer).newBinaryAccountIterator(common.Hash{})) it, _ := snaps.AccountIterator(common.HexToHash("0x04"), common.Hash{}) defer it.Release() @@ -289,7 +289,7 @@ func TestAccountIteratorLargeTraversal(t *testing.T) { // Iterate the entire stack and ensure everything is hit only once head := snaps.Snapshot(common.HexToHash("0x80")) verifyIterator(t, 200, head.(snapshot).AccountIterator(common.Hash{})) - verifyIterator(t, 200, head.(*diffLayer).newBinaryAccountIterator()) + verifyIterator(t, 200, head.(*diffLayer).newBinaryAccountIterator(common.Hash{})) it, _ := snaps.AccountIterator(common.HexToHash("0x80"), common.Hash{}) defer it.Release() @@ -472,12 +472,12 @@ func BenchmarkAccountIteratorTraversal(b *testing.B) { // We call this once before the benchmark, so the creation of // sorted accountlists are not included in the results. head := snaps.Snapshot(common.HexToHash("0x65")) - head.(*diffLayer).newBinaryAccountIterator() + head.(*diffLayer).newBinaryAccountIterator(common.Hash{}) b.Run("binary iterator keys", func(b *testing.B) { for i := 0; i < b.N; i++ { got := 0 - it := head.(*diffLayer).newBinaryAccountIterator() + it := head.(*diffLayer).newBinaryAccountIterator(common.Hash{}) for it.Next() { got++ } @@ -489,7 +489,7 @@ func BenchmarkAccountIteratorTraversal(b *testing.B) { b.Run("binary iterator values", func(b *testing.B) { for i := 0; i < b.N; i++ { got := 0 - it := head.(*diffLayer).newBinaryAccountIterator() + it := head.(*diffLayer).newBinaryAccountIterator(common.Hash{}) for it.Next() { got++ head.(*diffLayer).accountRLP(it.Hash(), 0) @@ -569,12 +569,12 @@ func BenchmarkAccountIteratorLargeBaselayer(b *testing.B) { // We call this once before the benchmark, so the creation of // sorted accountlists are not included in the results. head := snaps.Snapshot(common.HexToHash("0x65")) - head.(*diffLayer).newBinaryAccountIterator() + head.(*diffLayer).newBinaryAccountIterator(common.Hash{}) b.Run("binary iterator (keys)", func(b *testing.B) { for i := 0; i < b.N; i++ { got := 0 - it := head.(*diffLayer).newBinaryAccountIterator() + it := head.(*diffLayer).newBinaryAccountIterator(common.Hash{}) for it.Next() { got++ } @@ -586,7 +586,7 @@ func BenchmarkAccountIteratorLargeBaselayer(b *testing.B) { b.Run("binary iterator (values)", func(b *testing.B) { for i := 0; i < b.N; i++ { got := 0 - it := head.(*diffLayer).newBinaryAccountIterator() + it := head.(*diffLayer).newBinaryAccountIterator(common.Hash{}) for it.Next() { got++ v := it.Hash() @@ -631,7 +631,7 @@ func BenchmarkAccountIteratorLargeBaselayer(b *testing.B) { /* func BenchmarkBinaryAccountIteration(b *testing.B) { benchmarkAccountIteration(b, func(snap snapshot) AccountIterator { - return snap.(*diffLayer).newBinaryAccountIterator() + return snap.(*diffLayer).newBinaryAccountIterator(common.Hash{}) }) }