From bb80d1ed2d505edcecfff189c03c7f3bbc1c988c Mon Sep 17 00:00:00 2001 From: Travis Date: Wed, 22 Jan 2020 11:59:32 -0600 Subject: [PATCH] tidy comments and remove pilosa dependency --- roaring/roaring.go | 104 ++++++++++++++++++------------- roaring/roaring_internal_test.go | 16 ++--- roaring/roaring_test.go | 28 +++++---- 3 files changed, 83 insertions(+), 65 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index ed1b0bbe7..126833b2d 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -25,7 +25,6 @@ import ( "sort" "unsafe" - "github.com/pilosa/pilosa" "github.com/pkg/errors" ) @@ -834,16 +833,18 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) { // Bitmap 4 |___X_______________________| | |___X_______________________| | |___X_______________________| // _ func (b *Bitmap) unionInPlace(others ...*Bitmap) { + const staticSize = 20 var ( requiredSliceSize = len(others) - // To avoid having to allocate a slice everytime, if the number of bitmaps - // being unioned is small enough we can just use this stack-allocated array. - staticHandledIters = [20]handledIter{} + // To avoid having to allocate a slice every time, if the number of bitmaps + // being unioned is small enough (i.e. smaller than staticSize), we can just + // use this stack-allocated array. + staticHandledIters = [staticSize]handledIter{} bitmapIters handledIters target = b ) - if requiredSliceSize <= 20 { + if requiredSliceSize <= staticSize { bitmapIters = staticHandledIters[:0] } else { bitmapIters = make(handledIters, 0, requiredSliceSize) @@ -5454,18 +5455,29 @@ type containerUnionSummaryStats struct { hasMaxRange bool } +// DifferenceInPlace returns the bitwise difference of b and others, modifying +// b in place. func (b *Bitmap) DifferenceInPlace(others ...*Bitmap) { + bSize := b.Size() + + // If b doesn't have any containers then return early. + if bSize == 0 { + return + } + + const staticSize = 20 var ( requiredSliceSize = len(others) - // To avoid having to allocate a slice everytime, if the number of bitmaps - // being unioned is small enough we can just use this stack-allocated array. - staticHandledIters = [20]handledIter{} - bitmapIters handledIters - target = b - removeContinerKeys = [pilosa.ShardWidth / (1 << 16)]uint64{} //i need a better guess for this + // To avoid having to allocate a slice every time, if the number of bitmaps + // being differenced is small enough (i.e. smaller than staticSize), we can + // just use this stack-allocated array. + staticHandledIters = [staticSize]handledIter{} + bitmapIters handledIters + target = b + removeContainerKeys = make([]uint64, bSize) ) - if requiredSliceSize <= 20 { + if requiredSliceSize <= staticSize { bitmapIters = staticHandledIters[:0] } else { bitmapIters = make(handledIters, 0, requiredSliceSize) @@ -5494,9 +5506,9 @@ func (b *Bitmap) DifferenceInPlace(others ...*Bitmap) { } iKey, iContainer := iIter.iter.Value() if targetKey == iKey { - curContainer.differenceInPlaceO(iContainer) + curContainer.differenceInPlace(iContainer) if curContainer.N() == 0 { //according to comments N = 1-count, so N should == 1 if 0 elements - removeContinerKeys[n] = iKey + removeContainerKeys[n] = iKey n++ break } @@ -5508,35 +5520,13 @@ func (b *Bitmap) DifferenceInPlace(others ...*Bitmap) { } for i := 0; i < n; i++ { - b.Containers.Remove(removeContinerKeys[i]) + b.Containers.Remove(removeContainerKeys[i]) } target.Containers.Repair() } func (c *Container) differenceInPlace(other *Container) { - if other.isArray() { - for _, v := range other.array() { - c.remove(v) - } - } else if other.isBitmap() { - for i, word := range other.bitmap() { - for word != 0 { - t := word & -word - c.remove(uint16((i*64 + int(popcount(t-1))))) - word ^= t - } - } - } else if other.isRun() { - for _, r := range other.runs() { - for v := int(r.start); v <= int(r.last); v++ { - c.remove(uint16(v)) - } - } - } -} - -func (c *Container) differenceInPlaceO(other *Container) { if other.isArray() { if c.isArray() { differenceArrayArrayInPlace(c, other) @@ -5564,10 +5554,13 @@ func (c *Container) differenceInPlaceO(other *Container) { } } -func differenceArrayArrayInPlace(output, other *Container) { +func differenceArrayArrayInPlace(c, other *Container) { statsHit("differenceInPlace/ArrayArray") - aa, ab := output.array(), other.array() + aa, ab := c.array(), other.array() na, nb := len(aa), len(ab) + if na == 0 || nb == 0 { + return + } n := 0 for i, j := 0, 0; i < na; { va := aa[i] @@ -5590,13 +5583,17 @@ func differenceArrayArrayInPlace(output, other *Container) { } } aa = aa[:n] - output.setArray(aa) + c.setArray(aa) } + func differenceArrayBitmapInPlace(c, other *Container) { statsHit("differenceInPlace/ArrayBitmap") aa := c.array() n := 0 bitmap := other.bitmap() + if len(aa) == 0 || len(bitmap) == 0 { + return + } for _, va := range aa { bmidx := va / 64 bidx := va % 64 @@ -5659,9 +5656,13 @@ func differenceArrayRunInPlace(c, other *Container) { func differenceBitmapArrayInPlace(c, other *Container) { statsHit("differenceInPlace/BitmapArray") bitmap := c.bitmap() + ab := other.array() + if len(bitmap) == 0 || len(ab) == 0 { + return + } n := c.N() - for _, v := range other.array() { + for _, v := range ab { if c.bitmapContains(v) { bitmap[v/64] &^= (uint64(1) << uint(v%64)) n-- @@ -5677,10 +5678,15 @@ func differenceBitmapBitmapInPlace(c, other *Container) { statsHit("differenceInPlace/BitmapBitmap") // local variables added to prevent BCE checks in loop // see https://go101.org/article/bounds-check-elimination.html + a := c.bitmap() + b := other.bitmap() + if len(a) == 0 || len(b) == 0 { + return + } var ( - ab = c.bitmap()[:bitmapN] - bb = other.bitmap()[:bitmapN] + ab = a[:bitmapN] + bb = b[:bitmapN] n int32 ) @@ -5693,16 +5699,21 @@ func differenceBitmapBitmapInPlace(c, other *Container) { c.bitmapToArray() // Will this work? } } + func differenceBitmapRunInPlace(c, other *Container) { statsHit("differenceInPlace/BitmapRun") + if len(c.bitmap()) == 0 { + return + } for _, run := range other.runs() { c.bitmapZeroRange(uint64(run.start), uint64(run.last)+1) } } + func differenceRunArrayInPlace(c, other *Container) { statsHit("differenceInPlace/RunArray") ra, ab := c.runs(), other.array() - if len(ab) == 0 { + if len(ra) == 0 || len(ab) == 0 { return } runs := make([]interval16, 0, len(ra)) @@ -5755,9 +5766,13 @@ RUNLOOP: } c.optimize() } + func differenceRunBitmapInPlace(c, other *Container) { statsHit("differenceInPlace/RunBitmap") ra := c.runs() + if len(ra) == 0 || len(other.bitmap()) == 0 { + return + } // If a is full, difference is the flip of b. if len(ra) > 0 && ra[0].start == 0 && ra[0].last == 65535 { clone := other.Clone() @@ -5820,6 +5835,7 @@ func differenceRunBitmapInPlace(c, other *Container) { c.runToBitmap() } } + func differenceRunRunInPlace(c, other *Container) { statsHit("differenceInPlace/RunRun") diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index 48b0a4b0a..f8476459f 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -2709,6 +2709,7 @@ func unionInPlaceWrapper(a, b *Container) *Container { out.UnionInPlace(B) return out.Containers.Get(0) } + func differenceInPlaceWrapper(a, b *Container) *Container { out := NewBitmap() out.Containers.Put(0, a.Clone()) @@ -3388,23 +3389,20 @@ func TestContainerCombinations(t *testing.T) { {differenceInPlaceWrapper, "oddBitsSet", "firstBitUnset", "empty"}, {differenceInPlaceWrapper, "oddBitsSet", "lastBitUnset", "lastBitSet"}, {differenceInPlaceWrapper, "oddBitsSet", "innerBitsSet", "lastBitSet"}, - //{difference, "oddBitsSet", "outerBitsSet", ""}, + //{differenceInPlaceWrapper, "oddBitsSet", "outerBitsSet", ""}, {differenceInPlaceWrapper, "oddBitsSet", "oddBitsSet", "empty"}, {differenceInPlaceWrapper, "oddBitsSet", "evenBitsSet", "oddBitsSet"}, // {differenceInPlaceWrapper, "evenBitsSet", "empty", "evenBitsSet"}, {differenceInPlaceWrapper, "evenBitsSet", "full", "empty"}, - //{difference, "evenBitsSet", "firstBitSet", ""}, + //{differenceInPlaceWrapper, "evenBitsSet", "firstBitSet", ""}, {differenceInPlaceWrapper, "evenBitsSet", "lastBitSet", "evenBitsSet"}, {differenceInPlaceWrapper, "evenBitsSet", "firstBitUnset", "firstBitSet"}, {differenceInPlaceWrapper, "evenBitsSet", "lastBitUnset", "empty"}, {differenceInPlaceWrapper, "evenBitsSet", "innerBitsSet", "firstBitSet"}, - //{difference, "evenBitsSet", "outerBitsSet", ""}, + //{differenceInPlaceWrapper, "evenBitsSet", "outerBitsSet", ""}, {differenceInPlaceWrapper, "evenBitsSet", "oddBitsSet", "evenBitsSet"}, {differenceInPlaceWrapper, "evenBitsSet", "evenBitsSet", "empty"}, - /* - */ - } for _, testOp := range testOps { for _, x := range containerTypes { @@ -4019,13 +4017,11 @@ func TestBitmapAny(t *testing.T) { } } -func TestStuff(t *testing.T) { +func TestDifferenceInPlace_N(t *testing.T) { a := doContainer(containerRun, runFull()) b := doContainer(containerBitmap, bitmapFull()) - // b := doContainer(containerBitmap, bitmapLastBitSet()) r := differenceInPlaceWrapper(a, b) if r.N() != 0 { - t.Error("fail") + t.Error("expected difference of containers to have n=0") } - } diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go index 3d1d70734..54ea5704a 100644 --- a/roaring/roaring_test.go +++ b/roaring/roaring_test.go @@ -1740,40 +1740,46 @@ func BenchmarkUnionBulk(b *testing.B) { } } -func TestBitmap_differenceInPlace(t *testing.T) { - bm := testBM() - //the array +func TestBitmap_DifferenceInPlace(t *testing.T) { + // array arraybm := roaring.NewSliceBitmap() - bitmapbm := roaring.NewSliceBitmap() - smallrunbm := roaring.NewSliceBitmap() - largerunbm := roaring.NewSliceBitmap() - for i := uint64(0); i < 1024; i += 4 { _, _ = arraybm.Add((1 << 16) + i) } arraybm.Optimize() + // bitmap + bitmapbm := roaring.NewSliceBitmap() for i := uint64(0); i < 16384; i += 2 { _, _ = bitmapbm.Add((2 << 16) + i) } bitmapbm.Optimize() - //small run + + // small run + smallrunbm := roaring.NewSliceBitmap() for i := uint64(0); i < 1024; i++ { _, _ = smallrunbm.Add((3 << 16) + i) } smallrunbm.Optimize() + + // large run + largerunbm := roaring.NewSliceBitmap() for i := uint64(0); i < 65535; i++ { _, _ = largerunbm.Add((4 << 16) + i) } - smallrunbm.Optimize() + largerunbm.Optimize() + + // test the difference in place + bm := testBM() bm.DifferenceInPlace(arraybm, bitmapbm, smallrunbm, largerunbm) if bm.Count() != 0 { - t.Fatalf("expected bitmap to be empty, it wasn't. %d", bm.Count()) + t.Fatalf("expected bitmap count to be 0, but got: %d", bm.Count()) } + bm = testBM() bm.DifferenceInPlace(bitmapbm, smallrunbm, largerunbm) if bm.Count() != 256 { - t.Fatalf("expected bitmap to have 256, it wasn't. %d", bm.Count()) + t.Fatalf("expected bitmap count to be 256, but got: %d", bm.Count()) } }