From 60a834ac476c09061a661dd39e908abe94b2ed5b Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 3 Mar 2020 19:27:05 +0100 Subject: [PATCH 1/9] memdb: use btree for storage --- backend_test.go | 72 ++++++++++++++ common_test.go | 4 +- go.mod | 1 + go.sum | 2 + mem_batch.go | 2 +- mem_db.go | 260 +++++++++++++++++++++++++++++------------------- 6 files changed, 237 insertions(+), 104 deletions(-) diff --git a/backend_test.go b/backend_test.go index 9bba5f290..5ddfb50ee 100644 --- a/backend_test.go +++ b/backend_test.go @@ -204,6 +204,7 @@ func TestDBIterator(t *testing.T) { for dbType := range backends { t.Run(fmt.Sprintf("%v", dbType), func(t *testing.T) { testDBIterator(t, dbType) + testDBIterator_blankKey(t, dbType) }) } } @@ -311,6 +312,18 @@ func testDBIterator(t *testing.T, backend BackendType) { verifyIterator(t, ritr, []int64(nil), "reverse iterator from 7 (ex) to 6") + ritr, err = db.ReverseIterator(int642Bytes(10), nil) + require.NoError(t, err) + verifyIterator(t, ritr, []int64(nil), "reverse iterator to 10") + + ritr, err = db.ReverseIterator(int642Bytes(6), nil) + require.NoError(t, err) + verifyIterator(t, ritr, []int64{9, 8, 7}, "reverse iterator to 6") + + ritr, err = db.ReverseIterator(int642Bytes(5), nil) + require.NoError(t, err) + verifyIterator(t, ritr, []int64{9, 8, 7, 5}, "reverse iterator to 5") + // verifyIterator(t, db.Iterator(int642Bytes(0), int642Bytes(1)), []int64{0}, "forward iterator from 0 to 1") ritr, err = db.ReverseIterator(int642Bytes(8), int642Bytes(9)) @@ -329,7 +342,56 @@ func testDBIterator(t *testing.T, backend BackendType) { require.NoError(t, err) verifyIterator(t, ritr, []int64(nil), "reverse iterator from 2 (ex) to 4") +} + +func testDBIterator_blankKey(t *testing.T, backend BackendType) { + name := fmt.Sprintf("test_%x", randStr(12)) + dir := os.TempDir() + db := NewDB(name, backend, dir) + defer cleanupDBDir(dir, name) + + err := db.Set([]byte(""), []byte{0}) + require.NoError(t, err) + err = db.Set([]byte("a"), []byte{1}) + require.NoError(t, err) + err = db.Set([]byte("b"), []byte{2}) + require.NoError(t, err) + + value, err := db.Get([]byte("")) + require.NoError(t, err) + assert.Equal(t, []byte{0}, value) + + i, err := db.Iterator(nil, nil) + require.NoError(t, err) + verifyIteratorStrings(t, i, []string{"", "a", "b"}, "forward") + i, err = db.Iterator([]byte(""), nil) + require.NoError(t, err) + verifyIteratorStrings(t, i, []string{"", "a", "b"}, "forward from blank") + + i, err = db.Iterator([]byte("a"), nil) + require.NoError(t, err) + verifyIteratorStrings(t, i, []string{"a", "b"}, "forward from a") + + i, err = db.Iterator([]byte(""), []byte("b")) + require.NoError(t, err) + verifyIteratorStrings(t, i, []string{"", "a"}, "forward from blank to b") + + i, err = db.ReverseIterator(nil, nil) + require.NoError(t, err) + verifyIteratorStrings(t, i, []string{"b", "a", ""}, "reverse") + + i, err = db.ReverseIterator([]byte(""), nil) + require.NoError(t, err) + verifyIteratorStrings(t, i, []string{"b", "a", ""}, "reverse to blank") + + i, err = db.ReverseIterator([]byte(""), []byte("a")) + require.NoError(t, err) + verifyIteratorStrings(t, i, []string{""}, "reverse to blank from a") + + i, err = db.ReverseIterator([]byte("a"), nil) + require.NoError(t, err) + verifyIteratorStrings(t, i, []string{"b", "a"}, "reverse to a") } func verifyIterator(t *testing.T, itr Iterator, expected []int64, msg string) { @@ -341,3 +403,13 @@ func verifyIterator(t *testing.T, itr Iterator, expected []int64, msg string) { } assert.Equal(t, expected, list, msg) } + +func verifyIteratorStrings(t *testing.T, itr Iterator, expected []string, msg string) { + var list []string + for itr.Valid() { + key := itr.Key() + list = append(list, string(key)) + itr.Next() + } + assert.Equal(t, expected, list, msg) +} diff --git a/common_test.go b/common_test.go index d17c49e00..2e598dd64 100644 --- a/common_test.go +++ b/common_test.go @@ -80,7 +80,7 @@ func newTempDB(t *testing.T, backend BackendType) (db DB, dbDir string) { // NOTE: not actually goroutine safe. // If you want something goroutine safe, maybe you just want a MemDB. type mockDB struct { - mtx sync.Mutex + mtx sync.RWMutex calls map[string]int } @@ -90,7 +90,7 @@ func newMockDB() *mockDB { } } -func (mdb *mockDB) Mutex() *sync.Mutex { +func (mdb *mockDB) Mutex() *sync.RWMutex { return &(mdb.mtx) } diff --git a/go.mod b/go.mod index 6d3062630..56e87e583 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/facebookgo/stack v0.0.0-20160209184415-751773369052 // indirect github.com/facebookgo/subset v0.0.0-20150612182917-8dac2c3c4870 // indirect github.com/gogo/protobuf v1.3.1 + github.com/google/btree v1.0.0 github.com/jmhodges/levigo v1.0.0 github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.5.1 diff --git a/go.sum b/go.sum index 3c3d2e5ce..5b70eeea6 100644 --- a/go.sum +++ b/go.sum @@ -28,6 +28,8 @@ github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/snappy v0.0.1 h1:Qgr9rKW7uDUkrbSmQeiDsGa8SjGyCOGtuasMWwvp2P4= github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= +github.com/google/btree v1.0.0 h1:0udJVsspx3VBr5FwtLhQQtuAsVc79tTq0ocGIPAU6qo= +github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI= diff --git a/mem_batch.go b/mem_batch.go index a00baaf7b..d374e59e8 100644 --- a/mem_batch.go +++ b/mem_batch.go @@ -3,7 +3,7 @@ package db import "sync" type atomicSetDeleter interface { - Mutex() *sync.Mutex + Mutex() *sync.RWMutex SetNoLock(key, value []byte) SetNoLockSync(key, value []byte) DeleteNoLock(key []byte) diff --git a/mem_db.go b/mem_db.go index a88e764ef..0f28dab62 100644 --- a/mem_db.go +++ b/mem_db.go @@ -1,11 +1,36 @@ package db import ( + "bytes" + "context" "fmt" - "sort" "sync" + + "github.com/google/btree" ) +type item struct { + key []byte + value []byte +} + +// Less implements btree.Item. +func (i *item) Less(other btree.Item) bool { + // this considers nil == []byte{}, but that's ok since we handle nil endpoints + // in iterators specially anyway + return bytes.Compare(i.key, other.(*item).key) == -1 +} + +// newKey creates a new key item +func newKey(key []byte) *item { + return &item{key: key} +} + +// newPair creates a new pair item +func newPair(key, value []byte) *item { + return &item{key: key, value: value} +} + func init() { registerDBCreator(MemDBBackend, func(name, dir string) (DB, error) { return NewMemDB(), nil @@ -15,40 +40,42 @@ func init() { var _ DB = (*MemDB)(nil) type MemDB struct { - mtx sync.Mutex - db map[string][]byte + mtx sync.RWMutex + btree *btree.BTree } func NewMemDB() *MemDB { database := &MemDB{ - db: make(map[string][]byte), + btree: btree.New(32), } return database } // Implements atomicSetDeleter. -func (db *MemDB) Mutex() *sync.Mutex { +func (db *MemDB) Mutex() *sync.RWMutex { return &(db.mtx) } // Implements DB. func (db *MemDB) Get(key []byte) ([]byte, error) { - db.mtx.Lock() - defer db.mtx.Unlock() + db.mtx.RLock() + defer db.mtx.RUnlock() key = nonNilBytes(key) - value := db.db[string(key)] - return value, nil + i := db.btree.Get(newKey(key)) + if i != nil { + return i.(*item).value, nil + } + return nil, nil } // Implements DB. func (db *MemDB) Has(key []byte) (bool, error) { - db.mtx.Lock() - defer db.mtx.Unlock() + db.mtx.RLock() + defer db.mtx.RUnlock() key = nonNilBytes(key) - _, ok := db.db[string(key)] - return ok, nil + return db.btree.Has(newKey(key)), nil } // Implements DB. @@ -79,7 +106,7 @@ func (db *MemDB) SetNoLockSync(key []byte, value []byte) { key = nonNilBytes(key) value = nonNilBytes(value) - db.db[string(key)] = value + db.btree.ReplaceOrInsert(newPair(key, value)) } // Implements DB. @@ -109,7 +136,7 @@ func (db *MemDB) DeleteNoLock(key []byte) { func (db *MemDB) DeleteNoLockSync(key []byte) { key = nonNilBytes(key) - delete(db.db, string(key)) + db.btree.Delete(newKey(key)) } // Implements DB. @@ -127,28 +154,27 @@ func (db *MemDB) Print() error { db.mtx.Lock() defer db.mtx.Unlock() - for key, value := range db.db { - fmt.Printf("[%X]:\t[%X]\n", []byte(key), value) - } + db.btree.Ascend(func(i btree.Item) bool { + item := i.(*item) + fmt.Printf("[%X]:\t[%X]\n", item.key, item.value) + return true + }) return nil } // Implements DB. func (db *MemDB) Stats() map[string]string { - db.mtx.Lock() - defer db.mtx.Unlock() + db.mtx.RLock() + defer db.mtx.RUnlock() stats := make(map[string]string) stats["database.type"] = "memDB" - stats["database.size"] = fmt.Sprintf("%d", len(db.db)) + stats["database.size"] = fmt.Sprintf("%d", db.btree.Len()) return stats } // Implements DB. func (db *MemDB) NewBatch() Batch { - db.mtx.Lock() - defer db.mtx.Unlock() - return &memBatch{db, nil} } @@ -157,113 +183,145 @@ func (db *MemDB) NewBatch() Batch { // Implements DB. func (db *MemDB) Iterator(start, end []byte) (Iterator, error) { - db.mtx.Lock() - defer db.mtx.Unlock() + db.mtx.RLock() + defer db.mtx.RUnlock() - keys := db.getSortedKeys(start, end, false) - return newMemDBIterator(db, keys, start, end), nil + return newMemDBIterator(db.btree, start, end, false), nil } // Implements DB. func (db *MemDB) ReverseIterator(start, end []byte) (Iterator, error) { - db.mtx.Lock() - defer db.mtx.Unlock() + db.mtx.RLock() + defer db.mtx.RUnlock() - keys := db.getSortedKeys(start, end, true) - return newMemDBIterator(db, keys, start, end), nil + return newMemDBIterator(db.btree, start, end, true), nil } -// We need a copy of all of the keys. -// Not the best, but probably not a bottleneck depending. type memDBIterator struct { - db DB - cur int - keys []string - start []byte - end []byte + ch <-chan *item + cancel context.CancelFunc + item *item + start []byte + end []byte } var _ Iterator = (*memDBIterator)(nil) -// Keys is expected to be in reverse order for reverse iterators. -func newMemDBIterator(db DB, keys []string, start, end []byte) *memDBIterator { - return &memDBIterator{ - db: db, - cur: 0, - keys: keys, - start: start, - end: end, +func newMemDBIterator(bt *btree.BTree, start []byte, end []byte, reverse bool) *memDBIterator { + ctx, cancel := context.WithCancel(context.Background()) + ch := make(chan *item) + iter := &memDBIterator{ + ch: ch, + cancel: cancel, + start: start, + end: end, } -} -// Implements Iterator. -func (itr *memDBIterator) Domain() ([]byte, []byte) { - return itr.start, itr.end -} + go func() { + // Because we use [start, end) for reverse ranges, while btree uses (start, end], we need + // the following variables to handle some reverse iteration conditions ourselves. + var ( + skipEqual []byte + abortLessThan []byte + ) + visitor := func(i btree.Item) bool { + item := i.(*item) + if skipEqual != nil && bytes.Compare(item.key, skipEqual) == 0 { + skipEqual = nil + return true + } + if abortLessThan != nil && bytes.Compare(item.key, abortLessThan) == -1 { + return false + } + select { + case <-ctx.Done(): + return false + case ch <- item: + return true + } + } + s := newKey(start) + e := newKey(end) + switch { + case start == nil && end == nil && !reverse: + bt.Ascend(visitor) + case start == nil && end == nil && reverse: + bt.Descend(visitor) + case end == nil && !reverse: + // must handle this specially, since nil is considered less than anything else + bt.AscendGreaterOrEqual(s, visitor) + case !reverse: + bt.AscendRange(s, e, visitor) + case end == nil: + // abort after start, since we use [start, end) while btree uses (start, end] + abortLessThan = s.key + bt.Descend(visitor) + default: + // skip end and abort after start, since we use [start, end) while btree uses (start, end] + skipEqual = e.key + abortLessThan = s.key + bt.DescendLessOrEqual(e, visitor) + } + close(ch) + }() + + // prime the iterator with the first value, if any + if item, ok := <-ch; ok { + iter.item = item + } else { + iter.item = nil + } -// Implements Iterator. -func (itr *memDBIterator) Valid() bool { - return 0 <= itr.cur && itr.cur < len(itr.keys) + return iter } -// Implements Iterator. -func (itr *memDBIterator) Next() { - itr.assertIsValid() - itr.cur++ +// Close implements Iterator. +func (i *memDBIterator) Close() { + i.cancel() + for range i.ch { // drain channel + } } -// Implements Iterator. -func (itr *memDBIterator) Key() []byte { - itr.assertIsValid() - return []byte(itr.keys[itr.cur]) +// Domain implements Iterator. +func (i *memDBIterator) Domain() ([]byte, []byte) { + return i.start, i.end } -// Implements Iterator. -func (itr *memDBIterator) Value() []byte { - itr.assertIsValid() - key := []byte(itr.keys[itr.cur]) - bytes, err := itr.db.Get(key) - if err != nil { - return nil - } - return bytes +// Valid implements Iterator. +func (i *memDBIterator) Valid() bool { + return i.item != nil } -func (itr *memDBIterator) Error() error { - return nil +// Next implements Iterator. +func (i *memDBIterator) Next() { + item, ok := <-i.ch + switch { + case ok: + i.item = item + case i.item == nil: + panic("called Next() on invalid iterator") + default: + i.item = nil + } } -// Implements Iterator. -func (itr *memDBIterator) Close() { - itr.keys = nil - itr.db = nil +// Error implements Iterator. +func (i *memDBIterator) Error() error { + return nil // famous last words } -func (itr *memDBIterator) assertIsValid() { - if !itr.Valid() { - panic("memDBIterator is invalid") +// Key implements Iterator. +func (i *memDBIterator) Key() []byte { + if i.item == nil { + panic("called Key() on invalid iterator") } + return i.item.key } -//---------------------------------------- -// Misc. - -func (db *MemDB) getSortedKeys(start, end []byte, reverse bool) []string { - keys := []string{} - for key := range db.db { - inDomain := IsKeyInDomain([]byte(key), start, end) - if inDomain { - keys = append(keys, key) - } - } - sort.Strings(keys) - if reverse { - nkeys := len(keys) - for i := 0; i < nkeys/2; i++ { - temp := keys[i] - keys[i] = keys[nkeys-i-1] - keys[nkeys-i-1] = temp - } +// Value implements Iterator. +func (i *memDBIterator) Value() []byte { + if i.item == nil { + panic("called Value() on invalid iterator") } - return keys + return i.item.value } From f253ebd00e134f303e12c3345eafaa0887d9da45 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 3 Mar 2020 19:35:38 +0100 Subject: [PATCH 2/9] Appease linter --- backend_test.go | 4 ++-- mem_db.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend_test.go b/backend_test.go index 5ddfb50ee..1d530ecb0 100644 --- a/backend_test.go +++ b/backend_test.go @@ -204,7 +204,7 @@ func TestDBIterator(t *testing.T) { for dbType := range backends { t.Run(fmt.Sprintf("%v", dbType), func(t *testing.T) { testDBIterator(t, dbType) - testDBIterator_blankKey(t, dbType) + testDBIteratorBlankKey(t, dbType) }) } } @@ -344,7 +344,7 @@ func testDBIterator(t *testing.T, backend BackendType) { []int64(nil), "reverse iterator from 2 (ex) to 4") } -func testDBIterator_blankKey(t *testing.T, backend BackendType) { +func testDBIteratorBlankKey(t *testing.T, backend BackendType) { name := fmt.Sprintf("test_%x", randStr(12)) dir := os.TempDir() db := NewDB(name, backend, dir) diff --git a/mem_db.go b/mem_db.go index 0f28dab62..10c318dd7 100644 --- a/mem_db.go +++ b/mem_db.go @@ -226,7 +226,7 @@ func newMemDBIterator(bt *btree.BTree, start []byte, end []byte, reverse bool) * ) visitor := func(i btree.Item) bool { item := i.(*item) - if skipEqual != nil && bytes.Compare(item.key, skipEqual) == 0 { + if skipEqual != nil && bytes.Equal(item.key, skipEqual) { skipEqual = nil return true } From 5fe3042ffd0176cacda03faca306d9085a6c8567 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 3 Mar 2020 20:23:30 +0100 Subject: [PATCH 3/9] Minor tweaks --- mem_db.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mem_db.go b/mem_db.go index 10c318dd7..f81b48c24 100644 --- a/mem_db.go +++ b/mem_db.go @@ -209,7 +209,7 @@ var _ Iterator = (*memDBIterator)(nil) func newMemDBIterator(bt *btree.BTree, start []byte, end []byte, reverse bool) *memDBIterator { ctx, cancel := context.WithCancel(context.Background()) - ch := make(chan *item) + ch := make(chan *item, 64) iter := &memDBIterator{ ch: ch, cancel: cancel, @@ -268,8 +268,6 @@ func newMemDBIterator(bt *btree.BTree, start []byte, end []byte, reverse bool) * // prime the iterator with the first value, if any if item, ok := <-ch; ok { iter.item = item - } else { - iter.item = nil } return iter @@ -280,6 +278,7 @@ func (i *memDBIterator) Close() { i.cancel() for range i.ch { // drain channel } + i.item = nil } // Domain implements Iterator. From 3fa3c74c199bed986fffdcfa743a4077087563fb Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 3 Mar 2020 21:11:21 +0100 Subject: [PATCH 4/9] Added range scan benchmark --- common_test.go | 44 ++++++++++++++++++++++++++++++++++++++++---- mem_db_test.go | 21 +++++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/common_test.go b/common_test.go index 2e598dd64..57c8d4fdb 100644 --- a/common_test.go +++ b/common_test.go @@ -201,6 +201,38 @@ func (mockIterator) Error() error { func (mockIterator) Close() { } +func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) { + b.StopTimer() + + rangeSize := int64(10000) + if dbSize < rangeSize { + b.Errorf("db size %v cannot be less than range size %v", dbSize, rangeSize) + } + + for i := int64(0); i < dbSize; i++ { + bytes := int642Bytes(i) + err := db.Set(bytes, bytes) + if err != nil { + // for performance + require.NoError(b, err) + } + } + b.StartTimer() + + for i := 0; i < b.N; i++ { + start := rand.Int63n(dbSize - rangeSize) + end := start + rangeSize + iter, err := db.Iterator(int642Bytes(start), int642Bytes(end)) + require.NoError(b, err) + count := 0 + for ; iter.Valid(); iter.Next() { + count++ + } + iter.Close() + require.EqualValues(b, rangeSize, count) + } +} + func benchmarkRandomReadsWrites(b *testing.B, db DB) { b.StopTimer() @@ -217,23 +249,27 @@ func benchmarkRandomReadsWrites(b *testing.B, db DB) { for i := 0; i < b.N; i++ { // Write something { - idx := int64(rand.Int()) % numItems // nolint:gosec testing file, so accepting weak random number generator + idx := rand.Int63n(numItems) // nolint:gosec testing file, so accepting weak random number generator internal[idx]++ val := internal[idx] idxBytes := int642Bytes(idx) valBytes := int642Bytes(val) //fmt.Printf("Set %X -> %X\n", idxBytes, valBytes) err := db.Set(idxBytes, valBytes) - b.Error(err) + if err != nil { + require.NoError(b, err) + } } // Read something { - idx := int64(rand.Int()) % numItems // nolint:gosec testing file, so accepting weak random number generator + idx := rand.Int63n(numItems) // nolint:gosec testing file, so accepting weak random number generator valExp := internal[idx] idxBytes := int642Bytes(idx) valBytes, err := db.Get(idxBytes) - b.Error(err) + if err != nil { + require.NoError(b, err) + } //fmt.Printf("Get %X -> %X\n", idxBytes, valBytes) if valExp == 0 { if !bytes.Equal(valBytes, nil) { diff --git a/mem_db_test.go b/mem_db_test.go index 7f6468ee6..ee2eab9a7 100644 --- a/mem_db_test.go +++ b/mem_db_test.go @@ -32,3 +32,24 @@ func TestMemDB_Iterator(t *testing.T) { itr.Next() assert.False(t, itr.Valid()) } + +func BenchmarkMemDBRangeScans1M(b *testing.B) { + db := NewMemDB() + defer db.Close() + + benchmarkRangeScans(b, db, int64(1e6)) +} + +func BenchmarkMemDBRangeScans10M(b *testing.B) { + db := NewMemDB() + defer db.Close() + + benchmarkRangeScans(b, db, int64(10e6)) +} + +func BenchmarkMemDBRandomReadsWrites(b *testing.B) { + db := NewMemDB() + defer db.Close() + + benchmarkRandomReadsWrites(b, db) +} From f919587f42fd40fee8209901a70f51dc737c6bbc Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Wed, 4 Mar 2020 10:44:48 +0100 Subject: [PATCH 5/9] Added constants for MemDB parameters --- mem_db.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/mem_db.go b/mem_db.go index f81b48c24..c60815448 100644 --- a/mem_db.go +++ b/mem_db.go @@ -9,6 +9,17 @@ import ( "github.com/google/btree" ) +const ( + // The approximate number of items and children per B-tree node. Tuned with benchmarks. + bTreeDegree = 32 + + // Size of the channel buffer between traversal goroutine and iterator. Using an unbuffered + // channel causes two context switches per item sent, while buffering allows more work per + // context switch. Tuned with benchmarks. + chBufferSize = 64 +) + +// item is a btree.Item with byte slices as keys and values type item struct { key []byte value []byte @@ -46,7 +57,7 @@ type MemDB struct { func NewMemDB() *MemDB { database := &MemDB{ - btree: btree.New(32), + btree: btree.New(bTreeDegree), } return database } @@ -209,7 +220,7 @@ var _ Iterator = (*memDBIterator)(nil) func newMemDBIterator(bt *btree.BTree, start []byte, end []byte, reverse bool) *memDBIterator { ctx, cancel := context.WithCancel(context.Background()) - ch := make(chan *item, 64) + ch := make(chan *item, chBufferSize) iter := &memDBIterator{ ch: ch, cancel: cancel, From 3a9fb9ad2bd00981a30c1bc7a1741c54cbd87f84 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Wed, 4 Mar 2020 10:46:31 +0100 Subject: [PATCH 6/9] Added performance comments in tests --- common_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/common_test.go b/common_test.go index 57c8d4fdb..3ec99fc95 100644 --- a/common_test.go +++ b/common_test.go @@ -213,7 +213,8 @@ func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) { bytes := int642Bytes(i) err := db.Set(bytes, bytes) if err != nil { - // for performance + // require.NoError() is very expensive (according to profiler), so we do a cheap if as + // well since this is a tight loop. require.NoError(b, err) } } @@ -257,6 +258,8 @@ func benchmarkRandomReadsWrites(b *testing.B, db DB) { //fmt.Printf("Set %X -> %X\n", idxBytes, valBytes) err := db.Set(idxBytes, valBytes) if err != nil { + // require.NoError() is very expensive (according to profiler), so we do a cheap if + // as well since this is a tight loop. require.NoError(b, err) } } @@ -268,6 +271,8 @@ func benchmarkRandomReadsWrites(b *testing.B, db DB) { idxBytes := int642Bytes(idx) valBytes, err := db.Get(idxBytes) if err != nil { + // require.NoError() is very expensive (according to profiler), so we do a cheap if + // as well since this is a tight loop. require.NoError(b, err) } //fmt.Printf("Get %X -> %X\n", idxBytes, valBytes) From 2b2bff6ce202a4f5a527f8fb73e5f1f9e78ec045 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Wed, 4 Mar 2020 10:47:33 +0100 Subject: [PATCH 7/9] Use a Mutex rather than RWMutex for strict backwards compatibility --- common_test.go | 4 ++-- mem_batch.go | 2 +- mem_db.go | 24 ++++++++++++------------ 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/common_test.go b/common_test.go index 3ec99fc95..13706352c 100644 --- a/common_test.go +++ b/common_test.go @@ -80,7 +80,7 @@ func newTempDB(t *testing.T, backend BackendType) (db DB, dbDir string) { // NOTE: not actually goroutine safe. // If you want something goroutine safe, maybe you just want a MemDB. type mockDB struct { - mtx sync.RWMutex + mtx sync.Mutex calls map[string]int } @@ -90,7 +90,7 @@ func newMockDB() *mockDB { } } -func (mdb *mockDB) Mutex() *sync.RWMutex { +func (mdb *mockDB) Mutex() *sync.Mutex { return &(mdb.mtx) } diff --git a/mem_batch.go b/mem_batch.go index d374e59e8..a00baaf7b 100644 --- a/mem_batch.go +++ b/mem_batch.go @@ -3,7 +3,7 @@ package db import "sync" type atomicSetDeleter interface { - Mutex() *sync.RWMutex + Mutex() *sync.Mutex SetNoLock(key, value []byte) SetNoLockSync(key, value []byte) DeleteNoLock(key []byte) diff --git a/mem_db.go b/mem_db.go index c60815448..286145810 100644 --- a/mem_db.go +++ b/mem_db.go @@ -51,7 +51,7 @@ func init() { var _ DB = (*MemDB)(nil) type MemDB struct { - mtx sync.RWMutex + mtx sync.Mutex btree *btree.BTree } @@ -63,14 +63,14 @@ func NewMemDB() *MemDB { } // Implements atomicSetDeleter. -func (db *MemDB) Mutex() *sync.RWMutex { +func (db *MemDB) Mutex() *sync.Mutex { return &(db.mtx) } // Implements DB. func (db *MemDB) Get(key []byte) ([]byte, error) { - db.mtx.RLock() - defer db.mtx.RUnlock() + db.mtx.Lock() + defer db.mtx.Unlock() key = nonNilBytes(key) i := db.btree.Get(newKey(key)) @@ -82,8 +82,8 @@ func (db *MemDB) Get(key []byte) ([]byte, error) { // Implements DB. func (db *MemDB) Has(key []byte) (bool, error) { - db.mtx.RLock() - defer db.mtx.RUnlock() + db.mtx.Lock() + defer db.mtx.Unlock() key = nonNilBytes(key) return db.btree.Has(newKey(key)), nil @@ -175,8 +175,8 @@ func (db *MemDB) Print() error { // Implements DB. func (db *MemDB) Stats() map[string]string { - db.mtx.RLock() - defer db.mtx.RUnlock() + db.mtx.Lock() + defer db.mtx.Unlock() stats := make(map[string]string) stats["database.type"] = "memDB" @@ -194,16 +194,16 @@ func (db *MemDB) NewBatch() Batch { // Implements DB. func (db *MemDB) Iterator(start, end []byte) (Iterator, error) { - db.mtx.RLock() - defer db.mtx.RUnlock() + db.mtx.Lock() + defer db.mtx.Unlock() return newMemDBIterator(db.btree, start, end, false), nil } // Implements DB. func (db *MemDB) ReverseIterator(start, end []byte) (Iterator, error) { - db.mtx.RLock() - defer db.mtx.RUnlock() + db.mtx.Lock() + defer db.mtx.Unlock() return newMemDBIterator(db.btree, start, end, true), nil } From c71e17359d28f4270527a48c4ddf69bd9e7c151f Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Wed, 4 Mar 2020 12:33:40 +0100 Subject: [PATCH 8/9] Use b.Fatal() instead of require.NoError() --- common_test.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/common_test.go b/common_test.go index 13706352c..895acb198 100644 --- a/common_test.go +++ b/common_test.go @@ -213,9 +213,8 @@ func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) { bytes := int642Bytes(i) err := db.Set(bytes, bytes) if err != nil { - // require.NoError() is very expensive (according to profiler), so we do a cheap if as - // well since this is a tight loop. - require.NoError(b, err) + // require.NoError() is very expensive (according to profiler), so check manually + b.Fatal(b, err) } } b.StartTimer() @@ -258,9 +257,8 @@ func benchmarkRandomReadsWrites(b *testing.B, db DB) { //fmt.Printf("Set %X -> %X\n", idxBytes, valBytes) err := db.Set(idxBytes, valBytes) if err != nil { - // require.NoError() is very expensive (according to profiler), so we do a cheap if - // as well since this is a tight loop. - require.NoError(b, err) + // require.NoError() is very expensive (according to profiler), so check manually + b.Fatal(b, err) } } @@ -271,9 +269,8 @@ func benchmarkRandomReadsWrites(b *testing.B, db DB) { idxBytes := int642Bytes(idx) valBytes, err := db.Get(idxBytes) if err != nil { - // require.NoError() is very expensive (according to profiler), so we do a cheap if - // as well since this is a tight loop. - require.NoError(b, err) + // require.NoError() is very expensive (according to profiler), so check manually + b.Fatal(b, err) } //fmt.Printf("Get %X -> %X\n", idxBytes, valBytes) if valExp == 0 { From 471cafd24cd87053b0e60626b3dce6ebb38a9f10 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Mon, 9 Mar 2020 13:18:37 +0100 Subject: [PATCH 9/9] Updated changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b303a9c90..c44adc479 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Improvements + +- [memdb] [\#53](https://github.com/tendermint/tm-db/pull/53) Use a B-tree for storage, which significantly improves range scan performance + ## 0.4.1 **2020-2-26**