From fd01d3cc6c92f87d1018b0b54f8b49aade3224fa Mon Sep 17 00:00:00 2001 From: Soheil Hassas Yeganeh Date: Thu, 14 Jul 2016 21:54:23 -0700 Subject: [PATCH 1/2] Fix race in patricia tree This commit fixes a major issue added in 703b087a. There are still wins on allocation though. benchmark old ns/op new ns/op delta BenchmarkCMuxConnHTTP1-4 783 836 +6.77% BenchmarkCMuxConnHTTP2-4 895 806 -9.94% BenchmarkCMuxConnHTTP1n2-4 1000 1026 +2.60% BenchmarkCMuxConnHTTP2n1-4 916 961 +4.91% benchmark old allocs new allocs delta BenchmarkCMuxConnHTTP1-4 3 4 +33.33% BenchmarkCMuxConnHTTP2-4 4 4 +0.00% BenchmarkCMuxConnHTTP1n2-4 4 5 +25.00% BenchmarkCMuxConnHTTP2n1-4 4 5 +25.00% benchmark old bytes new bytes delta BenchmarkCMuxConnHTTP1-4 272 280 +2.94% BenchmarkCMuxConnHTTP2-4 304 304 +0.00% BenchmarkCMuxConnHTTP1n2-4 304 312 +2.63% BenchmarkCMuxConnHTTP2n1-4 304 312 +2.63% --- patricia.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/patricia.go b/patricia.go index 0099e38..a38cae0 100644 --- a/patricia.go +++ b/patricia.go @@ -22,8 +22,8 @@ import ( // patriciaTree is a simple patricia tree that handles []byte instead of string // and cannot be changed after instantiation. type patriciaTree struct { - root *ptNode - buf []byte // preallocated buffer to read data while matching + root *ptNode + maxDepth int // max depth of the tree. } func newPatriciaTree(bs ...[]byte) *patriciaTree { @@ -34,8 +34,8 @@ func newPatriciaTree(bs ...[]byte) *patriciaTree { } } return &patriciaTree{ - root: newNode(bs), - buf: make([]byte, max+1), + root: newNode(bs), + maxDepth: max + 1, } } @@ -48,13 +48,15 @@ func newPatriciaTreeString(strs ...string) *patriciaTree { } func (t *patriciaTree) matchPrefix(r io.Reader) bool { - n, _ := io.ReadFull(r, t.buf) - return t.root.match(t.buf[:n], true) + buf := make([]byte, t.maxDepth) + n, _ := io.ReadFull(r, buf) + return t.root.match(buf[:n], true) } func (t *patriciaTree) match(r io.Reader) bool { - n, _ := io.ReadFull(r, t.buf) - return t.root.match(t.buf[:n], false) + buf := make([]byte, t.maxDepth) + n, _ := io.ReadFull(r, buf) + return t.root.match(buf[:n], false) } type ptNode struct { From df31d4863664f41a65bf1be4f8d5e08b499f956d Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Tue, 12 Jul 2016 12:13:54 -0400 Subject: [PATCH 2/2] Parallelize benchmarks --- .travis.yml | 3 ++- bench_test.go | 50 ++++++++++++++++++++++++++++---------------------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/.travis.yml b/.travis.yml index c21a725..6be7119 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,4 +24,5 @@ before_script: - if [[ $TRAVIS_GO_VERSION == 1.6* ]]; then go tool vet --shadow .; fi script: - - go test -v ./... + - go test -bench . -v ./... + - go test -race -bench . -v ./... diff --git a/bench_test.go b/bench_test.go index b1013e3..782b07b 100644 --- a/bench_test.go +++ b/bench_test.go @@ -62,12 +62,14 @@ func BenchmarkCMuxConnHTTP1(b *testing.B) { wg.Add(b.N) b.ResetTimer() - for i := 0; i < b.N; i++ { - c := &mockConn{ - r: bytes.NewReader(benchHTTP1Payload), + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + wg.Add(1) + m.serve(&mockConn{ + r: bytes.NewReader(benchHTTP1Payload), + }, donec, &wg) } - m.serve(c, donec, &wg) - } + }) } func BenchmarkCMuxConnHTTP2(b *testing.B) { @@ -80,12 +82,14 @@ func BenchmarkCMuxConnHTTP2(b *testing.B) { wg.Add(b.N) b.ResetTimer() - for i := 0; i < b.N; i++ { - c := &mockConn{ - r: bytes.NewReader(benchHTTP2Payload), + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + wg.Add(1) + m.serve(&mockConn{ + r: bytes.NewReader(benchHTTP2Payload), + }, donec, &wg) } - m.serve(c, donec, &wg) - } + }) } func BenchmarkCMuxConnHTTP1n2(b *testing.B) { @@ -98,15 +102,16 @@ func BenchmarkCMuxConnHTTP1n2(b *testing.B) { donec := make(chan struct{}) var wg sync.WaitGroup - wg.Add(b.N) b.ResetTimer() - for i := 0; i < b.N; i++ { - c := &mockConn{ - r: bytes.NewReader(benchHTTP2Payload), + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + wg.Add(1) + m.serve(&mockConn{ + r: bytes.NewReader(benchHTTP2Payload), + }, donec, &wg) } - m.serve(c, donec, &wg) - } + }) } func BenchmarkCMuxConnHTTP2n1(b *testing.B) { @@ -119,13 +124,14 @@ func BenchmarkCMuxConnHTTP2n1(b *testing.B) { donec := make(chan struct{}) var wg sync.WaitGroup - wg.Add(b.N) b.ResetTimer() - for i := 0; i < b.N; i++ { - c := &mockConn{ - r: bytes.NewReader(benchHTTP1Payload), + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + wg.Add(1) + m.serve(&mockConn{ + r: bytes.NewReader(benchHTTP1Payload), + }, donec, &wg) } - m.serve(c, donec, &wg) - } + }) }