From 440980384c4c854ae9f4f67dd90689d207e9eb6a Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Mon, 5 Feb 2018 14:04:10 -0600 Subject: [PATCH] applied travis suggestions --- fragment.go | 2 +- roaring/assembly_test.go | 8 +++++--- roaring/containers_btree.go | 15 ++++++++------- roaring/containers_slice.go | 2 -- roaring/containers_test.go | 31 +++++++++++++++++++------------ roaring/roaring.go | 2 +- roaring/roaring_test.go | 10 +++++----- 7 files changed, 39 insertions(+), 31 deletions(-) diff --git a/fragment.go b/fragment.go index a18c4de61..d1cd41e27 100644 --- a/fragment.go +++ b/fragment.go @@ -189,7 +189,7 @@ func (f *Fragment) Open() error { func (f *Fragment) openStorage() error { // Create a roaring bitmap to serve as storage for the slice. if f.storage == nil { - f.storage = roaring.NewBitmapBtree() + f.storage = roaring.NewBitmapBTree() } // Open the data file to be mmap'd and used as an ops log. file, err := os.OpenFile(f.path, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666) diff --git a/roaring/assembly_test.go b/roaring/assembly_test.go index a35ae58c5..b3e75c32c 100644 --- a/roaring/assembly_test.go +++ b/roaring/assembly_test.go @@ -42,7 +42,6 @@ func TestBSFQ_CompareGo(t *testing.T) { } */ } -var Result uint64 func BenchmarkBSF(b *testing.B) { for i := 0; i < b.N; i++ { BSFQ(uint64(i)) @@ -61,9 +60,12 @@ func BenchmarkPOPCNTQ(b *testing.B) { } } +// This value prevents the benchmarks from being optimized out +var Result uint64 + func BenchmarkPopcount(b *testing.B) { for i := 0; i < b.N; i++ { - Result=popcount(uint64(i)) + Result = popcount(uint64(i)) } } @@ -77,7 +79,7 @@ func BenchmarkPopcntAsm(b *testing.B) { func BenchmarkPopcntGo(b *testing.B) { // run the Fib function b.N times for n := 0; n < b.N; n++ { - Result=popcntGo(uint64(n)) + Result = popcntGo(uint64(n)) } } diff --git a/roaring/containers_btree.go b/roaring/containers_btree.go index 9bc2d118e..134fb12a2 100644 --- a/roaring/containers_btree.go +++ b/roaring/containers_btree.go @@ -47,13 +47,6 @@ func (btc *BTreeContainers) Put(key uint64, c *container) { btc.tree.Set(key, c) } -type updater struct { - key uint64 - containerType byte - n int - mapped bool -} - func (u updater) update(oldV *container, exists bool) (*container, bool) { // update the existing container if exists { @@ -69,6 +62,14 @@ func (u updater) update(oldV *container, exists bool) (*container, bool) { }, true } +// this struct is added to prevent the closure locals from being escaped out to the heap +type updater struct { + key uint64 + containerType byte + n int + mapped bool +} + func (btc *BTreeContainers) PutContainerValues(key uint64, containerType byte, n int, mapped bool) { a := updater{key, containerType, n, mapped} btc.tree.Put(key, a.update) diff --git a/roaring/containers_slice.go b/roaring/containers_slice.go index a27d9bd06..391ed74e4 100644 --- a/roaring/containers_slice.go +++ b/roaring/containers_slice.go @@ -27,7 +27,6 @@ func (sc *SliceContainers) Put(key uint64, c *container) { if i < 0 { sc.insertAt(key, c, -i-1) } else { - //should this happen? sc.containers[i] = c } @@ -42,7 +41,6 @@ func (sc *SliceContainers) PutContainerValues(key uint64, containerType byte, n c.mapped = mapped sc.insertAt(key, c, -i-1) } else { - //should this happen? c := sc.containers[i] c.containerType = containerType c.n = n diff --git a/roaring/containers_test.go b/roaring/containers_test.go index 02e998151..4199fcbe5 100644 --- a/roaring/containers_test.go +++ b/roaring/containers_test.go @@ -4,10 +4,17 @@ import ( "testing" ) -func TestContainersIterator(t *testing.T) { - //btc := NewBTreeContainers() - btc := NewSliceContainers() - itr, found := btc.Iterator(0) +func TestContainersSliceIterator(t *testing.T) { + btc := NewBTreeContainers() + testContainersIterator(btc, t) +} +func TestContainersBTreeIterator(t *testing.T) { + slc := NewSliceContainers() + testContainersIterator(slc, t) + +} +func testContainersIterator(cs Containers, t *testing.T) { + itr, found := cs.Iterator(0) if found { t.Fatalf("shouldn't have found 0 in empty btc") } @@ -15,10 +22,10 @@ func TestContainersIterator(t *testing.T) { t.Fatal("Next() should be false for empty btc") } - btc.Put(1, &container{n: 1}) - btc.Put(2, &container{n: 2}) + cs.Put(1, &container{n: 1}) + cs.Put(2, &container{n: 2}) - itr, found = btc.Iterator(0) + itr, found = cs.Iterator(0) if found { t.Fatalf("shouldn't have found 0") } @@ -40,11 +47,11 @@ func TestContainersIterator(t *testing.T) { t.Fatalf("itr should be done, but got true") } - btc.Put(3, &container{n: 3}) - btc.Put(5, &container{n: 5}) - btc.Put(6, &container{n: 6}) + cs.Put(3, &container{n: 3}) + cs.Put(5, &container{n: 5}) + cs.Put(6, &container{n: 6}) - itr, found = btc.Iterator(3) + itr, found = cs.Iterator(3) if !itr.Next() { t.Fatalf("3 should be next, but got false") } @@ -61,7 +68,7 @@ func TestContainersIterator(t *testing.T) { t.Fatalf("Wrong k/v, exp: 5,5 got: %v,%v", key, val.n) } - itr, found = btc.Iterator(4) + itr, found = cs.Iterator(4) if found { t.Fatalf("shouldn't have found 4") } diff --git a/roaring/roaring.go b/roaring/roaring.go index bcef1762c..3cc5c1638 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -120,7 +120,7 @@ func NewBitmap(a ...uint64) *Bitmap { return b } -func NewBitmapBtree(a ...uint64) *Bitmap { +func NewBitmapBTree(a ...uint64) *Bitmap { b := &Bitmap{ conts: NewBTreeContainers(), } diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go index 48d29c058..bea8c140b 100644 --- a/roaring/roaring_test.go +++ b/roaring/roaring_test.go @@ -1176,7 +1176,7 @@ const ( func BenchmarkContainerLinear(b *testing.B) { for n := 0; n < b.N; n++ { - bm := roaring.NewBitmapBtree() + bm := roaring.NewBitmapBTree() for row := uint64(1); row < NumRows; row++ { for col := uint64(1); col < NumColums; col++ { bm.Add(row*pilosa.SliceWidth + (col * MaxContainerVal)) @@ -1187,7 +1187,7 @@ func BenchmarkContainerLinear(b *testing.B) { func BenchmarkContainerReverse(b *testing.B) { for n := 0; n < b.N; n++ { - bm := roaring.NewBitmapBtree() + bm := roaring.NewBitmapBTree() for row := NumRows - 1; row >= 1; row-- { for col := NumColums - 1; col >= 1; col-- { bm.Add(row*pilosa.SliceWidth + (col * MaxContainerVal)) @@ -1198,7 +1198,7 @@ func BenchmarkContainerReverse(b *testing.B) { func BenchmarkContainerColumn(b *testing.B) { for n := 0; n < b.N; n++ { - bm := roaring.NewBitmapBtree() + bm := roaring.NewBitmapBTree() for col := uint64(1); col < NumColums; col++ { for row := uint64(1); row < NumRows; row++ { bm.Add(row*pilosa.SliceWidth + (col * MaxContainerVal)) @@ -1210,7 +1210,7 @@ func BenchmarkContainerColumn(b *testing.B) { func BenchmarkContainerOutsideIn(b *testing.B) { middle := NumRows / uint64(2) for n := 0; n < b.N; n++ { - bm := roaring.NewBitmapBtree() + bm := roaring.NewBitmapBTree() for col := uint64(1); col < NumColums; col++ { for row := uint64(1); row < middle; row++ { @@ -1224,7 +1224,7 @@ func BenchmarkContainerOutsideIn(b *testing.B) { func BenchmarkContainerInsideOut(b *testing.B) { middle := NumRows / uint64(2) for n := 0; n < b.N; n++ { - bm := roaring.NewBitmapBtree() + bm := roaring.NewBitmapBTree() for col := uint64(1); col < NumColums; col++ { for row := uint64(1); row <= middle; row++ { bm.Add((middle+row)*pilosa.SliceWidth + (col * MaxContainerVal))