Merge pull request #418 from neena/neena/faster-iandnot-bm

faster iandnot between bitmap containers and run containers

benchmark_test.go

@@ -1176,55 +1176,59 @@ func BenchmarkAndNot(b *testing.B) {
 	}
 
 	for _, inPlace := range []bool{true, false} {
-		for _, leftGen := range []generator{makeRunContainer, makeArrayContainer, makeBitmapContainer} {
-			for _, rightGen := range []generator{makeRunContainer, makeArrayContainer, makeBitmapContainer} {
-				b.Run(fmt.Sprintf("inPlace=%v/left=%s/right=%s", inPlace, leftGen.name, rightGen.name), func(b *testing.B) {
-					b.StopTimer()
-					serializedLefts := make([][]byte, 1000)
-					for i := range serializedLefts {
-						var err error
-						serializedLefts[i], err = leftGen.f().ToBytes()
-						if err != nil {
-							b.Fatal(err)
-						}
-					}
-					serializedRights := make([][]byte, 1000)
-					for i := range serializedRights {
-						var err error
-						serializedRights[i], err = rightGen.f().ToBytes()
-						if err != nil {
-							b.Fatal(err)
-						}
-					}
+		b.Run(fmt.Sprintf("inPlace=%v", inPlace), func(b *testing.B) {
+			for _, leftGen := range []generator{makeRunContainer, makeArrayContainer, makeBitmapContainer} {
+				b.Run(fmt.Sprintf("left=%s", leftGen.name), func(b *testing.B) {
+					for _, rightGen := range []generator{makeRunContainer, makeArrayContainer, makeBitmapContainer} {
+						b.Run(fmt.Sprintf("right=%s", rightGen.name), func(b *testing.B) {
+							b.StopTimer()
+							serializedLefts := make([][]byte, 1000)
+							for i := range serializedLefts {
+								var err error
+								serializedLefts[i], err = leftGen.f().ToBytes()
+								if err != nil {
+									b.Fatal(err)
+								}
+							}
+							serializedRights := make([][]byte, 1000)
+							for i := range serializedRights {
+								var err error
+								serializedRights[i], err = rightGen.f().ToBytes()
+								if err != nil {
+									b.Fatal(err)
+								}
+							}
 
-					lefts := make([]*Bitmap, b.N)
-					for i := range lefts {
-						buf := serializedLefts[i%len(serializedLefts)]
-						lefts[i] = NewBitmap()
-						if _, err := lefts[i].FromBuffer(buf); err != nil {
-							b.Fatal(err)
-						}
-						lefts[i] = lefts[i].Clone()
-					}
-					rights := make([]*Bitmap, b.N)
-					for i := range rights {
-						buf := serializedRights[i%len(serializedRights)]
-						rights[i] = NewBitmap()
-						if _, err := rights[i].FromBuffer(buf); err != nil {
-							b.Fatal(err)
-						}
-						rights[i] = rights[i].Clone()
-					}
-					b.StartTimer()
-					for i := 0; i < b.N; i++ {
-						if inPlace {
-							lefts[i].AndNot(rights[i])
-						} else {
-							_ = AndNot(lefts[i], rights[i])
-						}
+							lefts := make([]*Bitmap, b.N)
+							for i := range lefts {
+								buf := serializedLefts[i%len(serializedLefts)]
+								lefts[i] = NewBitmap()
+								if _, err := lefts[i].FromBuffer(buf); err != nil {
+									b.Fatal(err)
+								}
+								lefts[i] = lefts[i].Clone()
+							}
+							rights := make([]*Bitmap, b.N)
+							for i := range rights {
+								buf := serializedRights[i%len(serializedRights)]
+								rights[i] = NewBitmap()
+								if _, err := rights[i].FromBuffer(buf); err != nil {
+									b.Fatal(err)
+								}
+								rights[i] = rights[i].Clone()
+							}
+							b.StartTimer()
+							for i := 0; i < b.N; i++ {
+								if inPlace {
+									lefts[i].AndNot(rights[i])
+								} else {
+									_ = AndNot(lefts[i], rights[i])
+								}
+							}
+						})
 					}
 				})
 			}
-		}
+		})
 	}
 }

bitmapcontainer.go

@@ -893,8 +893,28 @@ func (bc *bitmapContainer) iandNotArray(ac *arrayContainer) container {
 }
 
 func (bc *bitmapContainer) iandNotRun16(rc *runContainer16) container {
-	rcb := rc.toBitmapContainer()
-	return bc.iandNotBitmapSurely(rcb)
+	if rc.isEmpty() || bc.isEmpty() {
+		// Nothing to do.
+		return bc
+	}
+
+	wordRangeStart := rc.iv[0].start / 64
+	wordRangeEnd := (rc.iv[len(rc.iv)-1].last()) / 64 // inclusive
+
+	cardinalityChange := popcntSlice(bc.bitmap[wordRangeStart : wordRangeEnd+1]) // before cardinality - after cardinality (for word range)
+
+	for _, iv := range rc.iv {
+		resetBitmapRange(bc.bitmap, int(iv.start), int(iv.last())+1)
+	}
+
+	cardinalityChange -= popcntSlice(bc.bitmap[wordRangeStart : wordRangeEnd+1])
+
+	bc.cardinality -= int(cardinalityChange)
+
+	if bc.getCardinality() <= arrayDefaultMaxSize {
+		return bc.toArrayContainer()
+	}
+	return bc
 }
 
 func (bc *bitmapContainer) andNotArray(value2 *arrayContainer) container {

bitmapcontainer_test.go

@@ -1,10 +1,12 @@
 package roaring
 
 import (
+	"math"
 	"math/rand"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // bitmapContainer's numberOfRuns() function should be correct against the runContainer equivalent
@@ -305,3 +307,24 @@ func TestBitmapContainerResetTo(t *testing.T) {
 		assert.True(t, dirty.toEfficientContainer().equals(run))
 	})
 }
+
+func TestBitmapContainerIAndNot(t *testing.T) {
+	var bc container
+	bc = newBitmapContainer()
+	for i := 0; i < arrayDefaultMaxSize; i++ {
+		bc.iadd(uint16(i * 3))
+	}
+	bc.iadd(math.MaxUint16)
+
+	var rc container
+	rc = newRunContainer16Range(0, 1)
+	for i := 0; i < arrayDefaultMaxSize-3; i++ {
+		rc = rc.iaddRange(i*3, i*3+1)
+	}
+	rc.iaddRange(math.MaxUint16-3, math.MaxUint16+1)
+
+	bc = bc.iandNot(rc)
+
+	require.ElementsMatch(t, []uint16{12279, 12282, 12285}, bc.(*arrayContainer).content)
+	require.Equal(t, 3, bc.getCardinality())
+}