Skip to content

Commit

Permalink
Minor optimizations (#1017)
Browse files Browse the repository at this point in the history
- Precalculate hash for bloom filter test.
- Remove assertTrue from y.CompareKeys and y.ParseKeys.
  • Loading branch information
Ibrahim Jarif authored Sep 6, 2019
1 parent 7e99a81 commit a1ff348
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 13 deletions.
6 changes: 4 additions & 2 deletions iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

"github.com/dgraph-io/badger/options"
"github.com/dgraph-io/badger/table"
"github.com/dgryski/go-farm"

"github.com/dgraph-io/badger/y"
)
Expand Down Expand Up @@ -361,7 +362,7 @@ func (opt *IteratorOptions) pickTable(t table.TableInterface) bool {
}
// Bloom filter lookup would only work if opt.Prefix does NOT have the read
// timestamp as part of the key.
if opt.prefixIsKey && t.DoesNotHave(opt.Prefix) {
if opt.prefixIsKey && t.DoesNotHave(farm.Fingerprint64(opt.Prefix)) {
return false
}
return true
Expand Down Expand Up @@ -394,6 +395,7 @@ func (opt *IteratorOptions) pickTables(all []*table.Table) []*table.Table {
}

var out []*table.Table
hash := farm.Fingerprint64(opt.Prefix)
for _, t := range filtered {
// When we encounter the first table whose smallest key is higher than
// opt.Prefix, we can stop.
Expand All @@ -402,7 +404,7 @@ func (opt *IteratorOptions) pickTables(all []*table.Table) []*table.Table {
}
// opt.Prefix is actually the key. So, we can run bloom filter checks
// as well.
if t.DoesNotHave(opt.Prefix) {
if t.DoesNotHave(hash) {
continue
}
out = append(out, t)
Expand Down
6 changes: 3 additions & 3 deletions iterator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ type tableMock struct {
left, right []byte
}

func (tm *tableMock) Smallest() []byte { return tm.left }
func (tm *tableMock) Biggest() []byte { return tm.right }
func (tm *tableMock) DoesNotHave(key []byte) bool { return false }
func (tm *tableMock) Smallest() []byte { return tm.left }
func (tm *tableMock) Biggest() []byte { return tm.right }
func (tm *tableMock) DoesNotHave(hash uint64) bool { return false }

func TestPickTables(t *testing.T) {
opt := DefaultIteratorOptions
Expand Down
5 changes: 4 additions & 1 deletion level_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"sort"
"sync"

"github.com/dgryski/go-farm"

"github.com/dgraph-io/badger/table"
"github.com/dgraph-io/badger/y"
"github.com/pkg/errors"
Expand Down Expand Up @@ -231,9 +233,10 @@ func (s *levelHandler) get(key []byte) (y.ValueStruct, error) {
tables, decr := s.getTableForKey(key)
keyNoTs := y.ParseKey(key)

hash := farm.Fingerprint64(keyNoTs)
var maxVs y.ValueStruct
for _, th := range tables {
if th.DoesNotHave(keyNoTs) {
if th.DoesNotHave(hash) {
y.NumLSMBloomHits.Add(s.strLevel, 1)
continue
}
Expand Down
9 changes: 4 additions & 5 deletions table/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"sync"
"sync/atomic"

"github.com/dgryski/go-farm"
"github.com/golang/protobuf/proto"
"github.com/pkg/errors"

Expand Down Expand Up @@ -62,7 +61,7 @@ type Options struct {
type TableInterface interface {
Smallest() []byte
Biggest() []byte
DoesNotHave(key []byte) bool
DoesNotHave(hash uint64) bool
}

// Table represents a loaded table file with the info we have about it
Expand Down Expand Up @@ -348,9 +347,9 @@ func (t *Table) Filename() string { return t.fd.Name() }
// ID is the table's ID number (used to make the file name).
func (t *Table) ID() uint64 { return t.id }

// DoesNotHave returns true if (but not "only if") the table does not have the key. It does a
// bloom filter lookup.
func (t *Table) DoesNotHave(key []byte) bool { return !t.bf.Has(farm.Fingerprint64(key)) }
// DoesNotHave returns true if (but not "only if") the table does not have the key hash.
// It does a bloom filter lookup.
func (t *Table) DoesNotHave(hash uint64) bool { return !t.bf.Has(hash) }

// VerifyChecksum verifies checksum for all blocks of table. This function is called by
// OpenTable() function. This function is also called inside levelsController.VerifyChecksum().
Expand Down
2 changes: 0 additions & 2 deletions y/y.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ func ParseTs(key []byte) uint64 {
// a<timestamp> would be sorted higher than aa<timestamp> if we use bytes.compare
// All keys should have timestamp.
func CompareKeys(key1, key2 []byte) int {
AssertTrue(len(key1) > 8 && len(key2) > 8)
if cmp := bytes.Compare(key1[:len(key1)-8], key2[:len(key2)-8]); cmp != 0 {
return cmp
}
Expand All @@ -141,7 +140,6 @@ func ParseKey(key []byte) []byte {
return nil
}

AssertTrue(len(key) > 8)
return key[:len(key)-8]
}

Expand Down

0 comments on commit a1ff348

Please sign in to comment.