Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: replace binary-tree library from petar/GoLLRB to google/btree. #5335

Merged
merged 7 commits into from
Dec 7, 2017
Merged

*: replace binary-tree library from petar/GoLLRB to google/btree. #5335

merged 7 commits into from
Dec 7, 2017

Conversation

winkyao
Copy link
Contributor

@winkyao winkyao commented Dec 7, 2017

the petar/GoLLRB is not active many years, some cases, TiDB will panic in llrb.flip().

image

@coocood @shenli @tiancaiamao PTAL

I use below benchmark cases:

package testllrb

import (
	"flag"
	"math/rand"
	"testing"

	// "github.com/petar/GoLLRB/llrb"
	llrb "github.com/google/btree"
)

var btreeDegree = flag.Int("degree", 32, "B-Tree degree")

// Int implements the Item interface for integers.
type Int int

// Less returns true if int(a) < int(b).
func (a Int) Less(b llrb.Item) bool {
	return a < b.(Int)
}

// perm returns a random permutation of n Int items in the range [0, n).
func perm(n int) (out []llrb.Item) {
	for _, v := range rand.Perm(n) {
		out = append(out, Int(v))
	}
	return
}

const benchmarkTreeSize = 100000

func BenchmarkInsert(b *testing.B) {
	b.StopTimer()
	insertP := perm(benchmarkTreeSize)
	b.StartTimer()
	i := 0
	for i < b.N {
		tr := llrb.New(*btreeDegree)
		for _, item := range insertP {
			tr.ReplaceOrInsert(item)
			i++
			if i >= b.N {
				return
			}
		}
	}
}

func BenchmarkDelete(b *testing.B) {
	b.StopTimer()
	insertP := perm(benchmarkTreeSize)
	removeP := perm(benchmarkTreeSize)
	b.StartTimer()
	i := 0
	for i < b.N {
		b.StopTimer()
		tr := llrb.New(*btreeDegree)
		for _, v := range insertP {
			tr.ReplaceOrInsert(v)
		}
		b.StartTimer()
		for _, item := range removeP {
			tr.Delete(item)
			i++
			if i >= b.N {
				return
			}
		}
		if tr.Len() > 0 {
			panic(tr.Len())
		}
	}
}

func BenchmarkGet(b *testing.B) {
	b.StopTimer()
	insertP := perm(benchmarkTreeSize)
	removeP := perm(benchmarkTreeSize)
	b.StartTimer()
	i := 0
	for i < b.N {
		b.StopTimer()
		tr := llrb.New(*btreeDegree)
		for _, v := range insertP {
			tr.ReplaceOrInsert(v)
		}
		b.StartTimer()
		for _, item := range removeP {
			tr.Get(item)
			i++
			if i >= b.N {
				return
			}
		}
	}
}

the petar/GoLLRB got:

❯ go test -bench=.
goos: darwin
goarch: amd64
pkg: gowink.cn/test
BenchmarkInsert-8   	 2000000	       738 ns/op
BenchmarkDelete-8   	 1000000	      1114 ns/op
BenchmarkGet-8      	 3000000	       497 ns/op
PASS
ok  	gowink.cn/test	10.146s

and google/bree got:

❯ go test --degree 32 -bench=.
goos: darwin
goarch: amd64
pkg: gowink.cn/test
BenchmarkInsert-8   	 3000000	       471 ns/op
BenchmarkDelete-8   	 3000000	       526 ns/op
BenchmarkGet-8      	 3000000	       475 ns/op
PASS
ok  	gowink.cn/test	10.829s

❯ go test --degree 2 -bench=.
goos: darwin
goarch: amd64
pkg: gowink.cn/test
BenchmarkInsert-8   	 1000000	      1309 ns/op
BenchmarkDelete-8   	 1000000	      1316 ns/op
BenchmarkGet-8      	 2000000	       997 ns/op
PASS
ok  	gowink.cn/test	12.086s

when google/btree's degree set to 32, is more effective.

ref https://opensource.googleblog.com/2013/01/c-containers-that-save-memory-and-time.html

@winkyao winkyao added the WIP label Dec 7, 2017
@coocood
Copy link
Member

coocood commented Dec 7, 2017

@winkyao
We should remove github.com/petar/GoLLRB in glide.yml.

@coocood
Copy link
Member

coocood commented Dec 7, 2017

@winkyao
Well, btree depends on llrb.

@winkyao winkyao removed the WIP label Dec 7, 2017
@winkyao winkyao changed the title _vendor: replace llrb library from petar/GoLLRB to google/btree. *: replace llrb library from petar/GoLLRB to google/btree. Dec 7, 2017
@winkyao winkyao changed the title *: replace llrb library from petar/GoLLRB to google/btree. *: replace binary-tree library from petar/GoLLRB to google/btree. Dec 7, 2017
@winkyao
Copy link
Contributor Author

winkyao commented Dec 7, 2017

/run-all-tests

1 similar comment
@tiancaiamao
Copy link
Contributor

/run-all-tests

@shenli
Copy link
Member

shenli commented Dec 7, 2017

Any benchmark result?

@coocood
Copy link
Member

coocood commented Dec 7, 2017

LGTM

@winkyao
Copy link
Contributor Author

winkyao commented Dec 7, 2017

@shenli on my first comment

@tiancaiamao
Copy link
Contributor

LGTM

@winkyao winkyao added all-tests-passed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 7, 2017
@coocood coocood merged commit 3674523 into pingcap:master Dec 7, 2017
@winkyao winkyao deleted the replace_llrb_library branch December 7, 2017 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants