diff --git a/lite/helpers.go b/lite/helpers.go index 9c015a08..d985882d 100644 --- a/lite/helpers.go +++ b/lite/helpers.go @@ -77,7 +77,7 @@ func (v ValKeys) signHeader(header *types.Header, first, last int) *types.Commit vset := v.ToValidators(1, 0) // fill in the votes we want - for i := first; i < last; i++ { + for i := first; i < last && i < len(v); i++ { vote := makeVote(header, vset, v[i]) votes[vote.ValidatorIndex] = vote } diff --git a/lite/memprovider.go b/lite/memprovider.go index ed7cd772..ac0d8321 100644 --- a/lite/memprovider.go +++ b/lite/memprovider.go @@ -14,6 +14,8 @@ type memStoreProvider struct { // btree would be more efficient for larger sets byHeight fullCommits byHash map[string]FullCommit + + sorted bool } // fullCommits just exists to allow easy sorting @@ -52,25 +54,78 @@ func (m *memStoreProvider) StoreCommit(fc FullCommit) error { defer m.mtx.Unlock() m.byHash[key] = fc m.byHeight = append(m.byHeight, fc) - sort.Sort(m.byHeight) + m.sorted = false return nil } // GetByHeight returns the FullCommit for height h or an error if the commit is not found. func (m *memStoreProvider) GetByHeight(h int64) (FullCommit, error) { + // By heuristics, GetByHeight with linearsearch is fast enough + // for about 50 keys but after that, it needs binary search. + // See https://github.com/tendermint/tendermint/pull/1043#issue-285188242 m.mtx.RLock() - defer m.mtx.RUnlock() + n := len(m.byHeight) + m.mtx.RUnlock() + if n <= 50 { + return m.getByHeightLinearSearch(h) + } + return m.getByHeightBinarySearch(h) +} + +func (m *memStoreProvider) sortByHeightIfNecessaryLocked() { + if !m.sorted { + sort.Sort(m.byHeight) + m.sorted = true + } +} + +func (m *memStoreProvider) getByHeightLinearSearch(h int64) (FullCommit, error) { + m.mtx.Lock() + defer m.mtx.Unlock() + m.sortByHeightIfNecessaryLocked() // search from highest to lowest for i := len(m.byHeight) - 1; i >= 0; i-- { - fc := m.byHeight[i] - if fc.Height() <= h { + if fc := m.byHeight[i]; fc.Height() <= h { return fc, nil } } return FullCommit{}, liteErr.ErrCommitNotFound() } +func (m *memStoreProvider) getByHeightBinarySearch(h int64) (FullCommit, error) { + m.mtx.Lock() + defer m.mtx.Unlock() + m.sortByHeightIfNecessaryLocked() + low, high := 0, len(m.byHeight)-1 + var mid int + var hmid int64 + var midFC FullCommit + // Our goal is to either find: + // * item ByHeight with the query + // * greatest height with a height <= query + for low <= high { + mid = int(uint(low+high) >> 1) // Avoid an overflow + midFC = m.byHeight[mid] + hmid = midFC.Height() + switch { + case hmid == h: + return midFC, nil + case hmid < h: + low = mid + 1 + case hmid > h: + high = mid - 1 + } + } + + if high >= 0 { + if highFC := m.byHeight[high]; highFC.Height() < h { + return highFC, nil + } + } + return FullCommit{}, liteErr.ErrCommitNotFound() +} + // GetByHash returns the FullCommit for the hash or an error if the commit is not found. func (m *memStoreProvider) GetByHash(hash []byte) (FullCommit, error) { m.mtx.RLock() @@ -85,12 +140,13 @@ func (m *memStoreProvider) GetByHash(hash []byte) (FullCommit, error) { // LatestCommit returns the latest FullCommit or an error if no commits exist. func (m *memStoreProvider) LatestCommit() (FullCommit, error) { - m.mtx.RLock() - defer m.mtx.RUnlock() + m.mtx.Lock() + defer m.mtx.Unlock() l := len(m.byHeight) if l == 0 { return FullCommit{}, liteErr.ErrCommitNotFound() } + m.sortByHeightIfNecessaryLocked() return m.byHeight[l-1], nil } diff --git a/lite/performance_test.go b/lite/performance_test.go index 28c73bb0..8cd522cb 100644 --- a/lite/performance_test.go +++ b/lite/performance_test.go @@ -1,33 +1,129 @@ -package lite_test +package lite import ( "fmt" + "math/rand" + "sync" "testing" - "github.com/tendermint/tendermint/lite" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + liteErr "github.com/tendermint/tendermint/lite/errors" ) +func TestMemStoreProvidergetByHeightBinaryAndLinearSameResult(t *testing.T) { + p := NewMemStoreProvider().(*memStoreProvider) + + // Store a bunch of commits at specific heights + // and then ensure that: + // * getByHeightLinearSearch + // * getByHeightBinarySearch + // both return the exact same result + + // 1. Non-existent height commits + nonExistent := []int64{-1000, -1, 0, 1, 10, 11, 17, 31, 67, 1000, 1e9} + ensureNonExistentCommitsAtHeight(t, "getByHeightLinearSearch", p.getByHeightLinearSearch, nonExistent) + ensureNonExistentCommitsAtHeight(t, "getByHeightBinarySearch", p.getByHeightBinarySearch, nonExistent) + + // 2. Save some known height commits + knownHeights := []int64{0, 1, 7, 9, 12, 13, 18, 44, 23, 16, 1024, 100, 199, 1e9} + createAndStoreCommits(t, p, knownHeights) + + // 3. Now check if those heights are retrieved + ensureExistentCommitsAtHeight(t, "getByHeightLinearSearch", p.getByHeightLinearSearch, knownHeights) + ensureExistentCommitsAtHeight(t, "getByHeightBinarySearch", p.getByHeightBinarySearch, knownHeights) + + // 4. And now for the height probing to ensure that any height + // requested returns a fullCommit of height <= requestedHeight. + comparegetByHeightAlgorithms(t, p, 0, 0) + comparegetByHeightAlgorithms(t, p, 1, 1) + comparegetByHeightAlgorithms(t, p, 2, 1) + comparegetByHeightAlgorithms(t, p, 5, 1) + comparegetByHeightAlgorithms(t, p, 7, 7) + comparegetByHeightAlgorithms(t, p, 10, 9) + comparegetByHeightAlgorithms(t, p, 12, 12) + comparegetByHeightAlgorithms(t, p, 14, 13) + comparegetByHeightAlgorithms(t, p, 19, 18) + comparegetByHeightAlgorithms(t, p, 43, 23) + comparegetByHeightAlgorithms(t, p, 45, 44) + comparegetByHeightAlgorithms(t, p, 1025, 1024) + comparegetByHeightAlgorithms(t, p, 101, 100) + comparegetByHeightAlgorithms(t, p, 1e3, 199) + comparegetByHeightAlgorithms(t, p, 1e4, 1024) + comparegetByHeightAlgorithms(t, p, 1e9, 1e9) + comparegetByHeightAlgorithms(t, p, 1e9+1, 1e9) +} + +func createAndStoreCommits(t *testing.T, p Provider, heights []int64) { + chainID := "cache-best-height-binary-and-linear" + appHash := []byte("0xdeadbeef") + keys := GenValKeys(len(heights) / 2) + + for _, h := range heights { + vals := keys.ToValidators(10, int64(len(heights)/2)) + fc := keys.GenFullCommit(chainID, h, nil, vals, appHash, []byte("params"), []byte("results"), 0, 5) + err := p.StoreCommit(fc) + require.NoError(t, err, "StoreCommit height=%d", h) + } +} + +func comparegetByHeightAlgorithms(t *testing.T, p *memStoreProvider, ask, expect int64) { + algos := map[string]func(int64) (FullCommit, error){ + "getHeightByLinearSearch": p.getByHeightLinearSearch, + "getHeightByBinarySearch": p.getByHeightBinarySearch, + } + + for algo, fn := range algos { + fc, err := fn(ask) + // t.Logf("%s got=%v want=%d", algo, expect, fc.Height()) + require.Nil(t, err, "%s: %+v", algo, err) + if assert.Equal(t, expect, fc.Height()) { + err = p.StoreCommit(fc) + require.Nil(t, err, "%s: %+v", algo, err) + } + } +} + +var blankFullCommit FullCommit + +func ensureNonExistentCommitsAtHeight(t *testing.T, prefix string, fn func(int64) (FullCommit, error), data []int64) { + for i, qh := range data { + fc, err := fn(qh) + assert.NotNil(t, err, "#%d: %s: height=%d should return non-nil error", i, prefix, qh) + assert.Equal(t, fc, blankFullCommit, "#%d: %s: height=%d\ngot =%+v\nwant=%+v", i, prefix, qh, fc, blankFullCommit) + } +} + +func ensureExistentCommitsAtHeight(t *testing.T, prefix string, fn func(int64) (FullCommit, error), data []int64) { + for i, qh := range data { + fc, err := fn(qh) + assert.Nil(t, err, "#%d: %s: height=%d should not return an error: %v", i, prefix, qh, err) + assert.NotEqual(t, fc, blankFullCommit, "#%d: %s: height=%d got a blankCommit", i, prefix, qh) + } +} + func BenchmarkGenCommit20(b *testing.B) { - keys := lite.GenValKeys(20) + keys := GenValKeys(20) benchmarkGenCommit(b, keys) } func BenchmarkGenCommit100(b *testing.B) { - keys := lite.GenValKeys(100) + keys := GenValKeys(100) benchmarkGenCommit(b, keys) } func BenchmarkGenCommitSec20(b *testing.B) { - keys := lite.GenSecpValKeys(20) + keys := GenSecpValKeys(20) benchmarkGenCommit(b, keys) } func BenchmarkGenCommitSec100(b *testing.B) { - keys := lite.GenSecpValKeys(100) + keys := GenSecpValKeys(100) benchmarkGenCommit(b, keys) } -func benchmarkGenCommit(b *testing.B, keys lite.ValKeys) { +func benchmarkGenCommit(b *testing.B, keys ValKeys) { chainID := fmt.Sprintf("bench-%d", len(keys)) vals := keys.ToValidators(20, 10) for i := 0; i < b.N; i++ { @@ -40,7 +136,7 @@ func benchmarkGenCommit(b *testing.B, keys lite.ValKeys) { // this benchmarks generating one key func BenchmarkGenValKeys(b *testing.B) { - keys := lite.GenValKeys(20) + keys := GenValKeys(20) for i := 0; i < b.N; i++ { keys = keys.Extend(1) } @@ -48,7 +144,7 @@ func BenchmarkGenValKeys(b *testing.B) { // this benchmarks generating one key func BenchmarkGenSecpValKeys(b *testing.B) { - keys := lite.GenSecpValKeys(20) + keys := GenSecpValKeys(20) for i := 0; i < b.N; i++ { keys = keys.Extend(1) } @@ -64,7 +160,7 @@ func BenchmarkToValidators100(b *testing.B) { // this benchmarks constructing the validator set (.PubKey() * nodes) func benchmarkToValidators(b *testing.B, nodes int) { - keys := lite.GenValKeys(nodes) + keys := GenValKeys(nodes) for i := 1; i <= b.N; i++ { keys.ToValidators(int64(2*i), int64(i)) } @@ -76,36 +172,36 @@ func BenchmarkToValidatorsSec100(b *testing.B) { // this benchmarks constructing the validator set (.PubKey() * nodes) func benchmarkToValidatorsSec(b *testing.B, nodes int) { - keys := lite.GenSecpValKeys(nodes) + keys := GenSecpValKeys(nodes) for i := 1; i <= b.N; i++ { keys.ToValidators(int64(2*i), int64(i)) } } func BenchmarkCertifyCommit20(b *testing.B) { - keys := lite.GenValKeys(20) + keys := GenValKeys(20) benchmarkCertifyCommit(b, keys) } func BenchmarkCertifyCommit100(b *testing.B) { - keys := lite.GenValKeys(100) + keys := GenValKeys(100) benchmarkCertifyCommit(b, keys) } func BenchmarkCertifyCommitSec20(b *testing.B) { - keys := lite.GenSecpValKeys(20) + keys := GenSecpValKeys(20) benchmarkCertifyCommit(b, keys) } func BenchmarkCertifyCommitSec100(b *testing.B) { - keys := lite.GenSecpValKeys(100) + keys := GenSecpValKeys(100) benchmarkCertifyCommit(b, keys) } -func benchmarkCertifyCommit(b *testing.B, keys lite.ValKeys) { +func benchmarkCertifyCommit(b *testing.B, keys ValKeys) { chainID := "bench-certify" vals := keys.ToValidators(20, 10) - cert := lite.NewStaticCertifier(chainID, vals) + cert := NewStaticCertifier(chainID, vals) check := keys.GenCommit(chainID, 123, nil, vals, []byte("foo"), []byte("params"), []byte("res"), 0, len(keys)) for i := 0; i < b.N; i++ { err := cert.Certify(check) @@ -115,3 +211,155 @@ func benchmarkCertifyCommit(b *testing.B, keys lite.ValKeys) { } } + +type algo bool + +const ( + linearSearch = true + binarySearch = false +) + +// Lazy load the commits +var fcs5, fcs50, fcs100, fcs500, fcs1000 []FullCommit +var h5, h50, h100, h500, h1000 []int64 +var commitsOnce sync.Once + +func lazyGenerateFullCommits(b *testing.B) { + b.Logf("Generating FullCommits") + commitsOnce.Do(func() { + fcs5, h5 = genFullCommits(nil, nil, 5) + b.Logf("Generated 5 FullCommits") + fcs50, h50 = genFullCommits(fcs5, h5, 50) + b.Logf("Generated 50 FullCommits") + fcs100, h100 = genFullCommits(fcs50, h50, 100) + b.Logf("Generated 100 FullCommits") + fcs500, h500 = genFullCommits(fcs100, h100, 500) + b.Logf("Generated 500 FullCommits") + fcs1000, h1000 = genFullCommits(fcs500, h500, 1000) + b.Logf("Generated 1000 FullCommits") + }) +} + +func BenchmarkMemStoreProviderGetByHeightLinearSearch5(b *testing.B) { + benchmarkMemStoreProvidergetByHeight(b, fcs5, h5, linearSearch) +} + +func BenchmarkMemStoreProviderGetByHeightLinearSearch50(b *testing.B) { + benchmarkMemStoreProvidergetByHeight(b, fcs50, h50, linearSearch) +} + +func BenchmarkMemStoreProviderGetByHeightLinearSearch100(b *testing.B) { + benchmarkMemStoreProvidergetByHeight(b, fcs100, h100, linearSearch) +} + +func BenchmarkMemStoreProviderGetByHeightLinearSearch500(b *testing.B) { + benchmarkMemStoreProvidergetByHeight(b, fcs500, h500, linearSearch) +} + +func BenchmarkMemStoreProviderGetByHeightLinearSearch1000(b *testing.B) { + benchmarkMemStoreProvidergetByHeight(b, fcs1000, h1000, linearSearch) +} + +func BenchmarkMemStoreProviderGetByHeightBinarySearch5(b *testing.B) { + benchmarkMemStoreProvidergetByHeight(b, fcs5, h5, binarySearch) +} + +func BenchmarkMemStoreProviderGetByHeightBinarySearch50(b *testing.B) { + benchmarkMemStoreProvidergetByHeight(b, fcs50, h50, binarySearch) +} + +func BenchmarkMemStoreProviderGetByHeightBinarySearch100(b *testing.B) { + benchmarkMemStoreProvidergetByHeight(b, fcs100, h100, binarySearch) +} + +func BenchmarkMemStoreProviderGetByHeightBinarySearch500(b *testing.B) { + benchmarkMemStoreProvidergetByHeight(b, fcs500, h500, binarySearch) +} + +func BenchmarkMemStoreProviderGetByHeightBinarySearch1000(b *testing.B) { + benchmarkMemStoreProvidergetByHeight(b, fcs1000, h1000, binarySearch) +} + +var rng = rand.New(rand.NewSource(10)) + +func benchmarkMemStoreProvidergetByHeight(b *testing.B, fcs []FullCommit, fHeights []int64, algo algo) { + lazyGenerateFullCommits(b) + + b.StopTimer() + mp := NewMemStoreProvider() + for i, fc := range fcs { + if err := mp.StoreCommit(fc); err != nil { + b.Fatalf("FullCommit #%d: err: %v", i, err) + } + } + qHeights := make([]int64, len(fHeights)) + copy(qHeights, fHeights) + // Append some non-existent heights to trigger the worst cases. + qHeights = append(qHeights, 19, -100, -10000, 1e7, -17, 31, -1e9) + + memP := mp.(*memStoreProvider) + searchFn := memP.getByHeightLinearSearch + if algo == binarySearch { // nolint + searchFn = memP.getByHeightBinarySearch + } + + hPerm := rng.Perm(len(qHeights)) + b.StartTimer() + b.ResetTimer() + for i := 0; i < b.N; i++ { + for _, j := range hPerm { + h := qHeights[j] + if _, err := searchFn(h); err != nil { + } + } + } + b.ReportAllocs() +} + +func genFullCommits(prevFC []FullCommit, prevH []int64, want int) ([]FullCommit, []int64) { + fcs := make([]FullCommit, len(prevFC)) + copy(fcs, prevFC) + heights := make([]int64, len(prevH)) + copy(heights, prevH) + + appHash := []byte("benchmarks") + chainID := "benchmarks-gen-full-commits" + n := want + keys := GenValKeys(2 + (n / 3)) + for i := 0; i < n; i++ { + vals := keys.ToValidators(10, int64(n/2)) + h := int64(20 + 10*i) + fcs = append(fcs, keys.GenFullCommit(chainID, h, nil, vals, appHash, []byte("params"), []byte("results"), 0, 5)) + heights = append(heights, h) + } + return fcs, heights +} + +func TestMemStoreProviderLatestCommitAlwaysUsesSorted(t *testing.T) { + p := NewMemStoreProvider().(*memStoreProvider) + // 1. With no commits yet stored, it should return ErrCommitNotFound + got, err := p.LatestCommit() + require.Equal(t, err.Error(), liteErr.ErrCommitNotFound().Error(), "should return ErrCommitNotFound()") + require.Equal(t, got, blankFullCommit, "With no fullcommits, it should return a blank FullCommit") + + // 2. Generate some full commits now and we'll add them unsorted. + genAndStoreCommitsOfHeight(t, p, 27, 100, 1, 12, 1000, 17, 91) + fc, err := p.LatestCommit() + require.Nil(t, err, "with commits saved no error expected") + require.NotEqual(t, fc, blankFullCommit, "with commits saved no blank FullCommit") + require.Equal(t, fc.Height(), int64(1000), "the latest commit i.e. the largest expected") +} + +func genAndStoreCommitsOfHeight(t *testing.T, p Provider, heights ...int64) { + n := len(heights) + appHash := []byte("tests") + chainID := "tests-gen-full-commits" + keys := GenValKeys(2 + (n / 3)) + for i := 0; i < n; i++ { + h := heights[i] + vals := keys.ToValidators(10, int64(n/2)) + fc := keys.GenFullCommit(chainID, h, nil, vals, appHash, []byte("params"), []byte("results"), 0, 5) + err := p.StoreCommit(fc) + require.NoError(t, err, "StoreCommit height=%d", h) + } +} diff --git a/lite/provider_test.go b/lite/provider_test.go index b2529b55..77b5b1a8 100644 --- a/lite/provider_test.go +++ b/lite/provider_test.go @@ -103,10 +103,10 @@ func checkProvider(t *testing.T, p lite.Provider, chainID, app string) { // this will make a get height, and if it is good, set the data as well func checkGetHeight(t *testing.T, p lite.Provider, ask, expect int64) { fc, err := p.GetByHeight(ask) - require.Nil(t, err, "%+v", err) + require.Nil(t, err, "GetByHeight") if assert.Equal(t, expect, fc.Height()) { err = p.StoreCommit(fc) - require.Nil(t, err, "%+v", err) + require.Nil(t, err, "StoreCommit") } }