From 82a9ef2059386483d364f7cdbbd2a040ef6a0e7a Mon Sep 17 00:00:00 2001
From: Yuce Tekol <yucetekol@gmail.com>
Date: Wed, 21 Nov 2018 16:35:50 +0300
Subject: [PATCH 01/61] Added /internal/translate/keys endpoint

---
 http/handler.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/http/handler.go b/http/handler.go
index 330acae0d..68719fdfe 100644
--- a/http/handler.go
+++ b/http/handler.go
@@ -24,8 +24,8 @@ import (
 	"io/ioutil"
 	"net"
 	"net/http"
-	_ "net/http/pprof"
-	"net/url" // Imported for its side-effect of registering pprof endpoints with the server.
+	_ "net/http/pprof" // Imported for its side-effect of registering pprof endpoints with the server.
+	"net/url"
 	"reflect"
 	"runtime/debug"
 	"strconv"

From b77c8a630b741ee2222ddd71bdca1f4018da7587 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Wed, 28 Nov 2018 14:46:15 -0500
Subject: [PATCH 02/61] Add in place union

---
 roaring/roaring.go | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 9c4274df9..8cc856bfc 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -396,9 +396,24 @@ func (b *Bitmap) Intersect(other *Bitmap) *Bitmap {
 }
 
 // Union returns the bitwise union of b and other.
-func (b *Bitmap) Union(other *Bitmap) *Bitmap {
+func (b *Bitmap) Union(others ...*Bitmap) *Bitmap {
 	output := NewBitmap()
+	output.UnionInPlace(others...)
+	return output
+}
 
+// UnionInPlace returns the bitwise union of b and other, modifying
+// b in place.
+func (b *Bitmap) UnionInPlace(others ...*Bitmap) {
+	for _, other := range others {
+		b.unionIntoTarget(other, b)
+	}
+}
+
+// unionIntoTarget stores the union of b and other into target. b and other will
+// be left unchanged, but target will be modified in place. Used to share
+// the union logic between the copy-on-write and in-place functions.
+func (b *Bitmap) unionIntoTarget(other *Bitmap, target *Bitmap) {
 	iiter, _ := b.Containers.Iterator(0)
 	jiter, _ := other.Containers.Iterator(0)
 	i, j := iiter.Next(), jiter.Next()
@@ -406,21 +421,20 @@ func (b *Bitmap) Union(other *Bitmap) *Bitmap {
 	kj, cj := jiter.Value()
 	for i || j {
 		if i && (!j || ki < kj) {
-			output.Containers.Put(ki, ci.Clone())
+			target.Containers.Put(ki, ci.Clone())
 			i = iiter.Next()
 			ki, ci = iiter.Value()
 		} else if j && (!i || ki > kj) {
-			output.Containers.Put(kj, cj.Clone())
+			target.Containers.Put(kj, cj.Clone())
 			j = jiter.Next()
 			kj, cj = jiter.Value()
 		} else { // ki == kj
-			output.Containers.Put(ki, union(ci, cj))
+			target.Containers.Put(ki, union(ci, cj))
 			i, j = iiter.Next(), jiter.Next()
 			ki, ci = iiter.Value()
 			kj, cj = jiter.Value()
 		}
 	}
-	return output
 }
 
 // Difference returns the difference of b and other.

From d3606e274dd84363859011dcf43dbb043556dacb Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Wed, 28 Nov 2018 15:07:56 -0500
Subject: [PATCH 03/61] fix bug

---
 roaring/roaring.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 8cc856bfc..99f7a06fa 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -398,6 +398,7 @@ func (b *Bitmap) Intersect(other *Bitmap) *Bitmap {
 // Union returns the bitwise union of b and other.
 func (b *Bitmap) Union(others ...*Bitmap) *Bitmap {
 	output := NewBitmap()
+	output.UnionInPlace(b)
 	output.UnionInPlace(others...)
 	return output
 }

From d2da91fdde0638b49db541879e3da9355deda750 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Wed, 28 Nov 2018 15:20:36 -0500
Subject: [PATCH 04/61] add test

---
 roaring/roaring_test.go | 83 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go
index f5c016ea3..f554d1df3 100644
--- a/roaring/roaring_test.go
+++ b/roaring/roaring_test.go
@@ -396,7 +396,44 @@ func TestBitmap_Union1(t *testing.T) {
 	if n := result.Count(); n != 75007 {
 		t.Fatalf("unexpected n: %d", n)
 	}
+}
 
+func TestBitmap_UnionInPlace1(t *testing.T) {
+	var (
+		bm0    = roaring.NewFileBitmap(0, 2683177)
+		bm1    = roaring.NewFileBitmap()
+		result = roaring.NewBitmap()
+	)
+	for i := uint64(628); i < 2683301; i++ {
+		bm1.Add(i)
+	}
+	bm1.Add(4000000)
+
+	result.UnionInPlace(bm0, bm1)
+	if n := result.Count(); n != 2682675 {
+		t.Fatalf("unexpected n: %d", n)
+	}
+
+	bm := testBM()
+	result = roaring.NewBitmap()
+	result.UnionInPlace(bm, bm0)
+	if n := result.Count(); n != 75009 {
+		t.Fatalf("unexpected n: %d", n)
+	}
+
+	result = roaring.NewBitmap()
+	result.UnionInPlace(bm, bm)
+	if n := result.Count(); n != 75007 {
+		t.Fatalf("unexpected n: %d", n)
+	}
+
+	// Make sure the bitmaps weren't mutated.
+	if n := bm0.Count(); n != 2 {
+		t.Fatalf("unexpected n: %d", n)
+	}
+	if n := bm1.Count(); n != 2682674 {
+		t.Fatalf("unexpected n: %d", n)
+	}
 }
 
 func TestBitmap_Intersection_Empty(t *testing.T) {
@@ -544,11 +581,35 @@ func TestBitmap_Union(t *testing.T) {
 	bm0 := roaring.NewFileBitmap(0, 1000001, 1000002, 1000003)
 	bm1 := roaring.NewFileBitmap(0, 50000, 1000001, 1000002)
 	result := bm0.Union(bm1)
+
 	if n := result.Count(); n != 5 {
 		t.Fatalf("unexpected n: %d", n)
 	}
 }
 
+func TestBitmap_UnionInPlace(t *testing.T) {
+	var (
+		bm0    = roaring.NewFileBitmap(0, 1000001, 1000002, 1000003)
+		bm1    = roaring.NewFileBitmap(0, 50000, 1000001, 1000002)
+		result = roaring.NewBitmap()
+	)
+	result.UnionInPlace(bm0, bm1)
+
+	// Make sure the union worked.
+	if n := result.Count(); n != 5 {
+		t.Fatalf("unexpected n: %d", n)
+	}
+
+	// Make sure the other bitmaps weren't mutated.
+	if n := bm0.Count(); n != 4 {
+		t.Fatalf("unexpected n: %d", n)
+	}
+	if n := bm1.Count(); n != 4 {
+		t.Fatalf("unexpected n: %d", n)
+	}
+
+}
+
 func TestBitmap_Xor(t *testing.T) {
 	bm0 := testBM()
 	bm1 := roaring.NewFileBitmap(0, 1, 2, 3)
@@ -1350,3 +1411,25 @@ func BenchmarkSliceDescending(b *testing.B) {
 		}
 	}
 }
+
+func BenchmarkUnion(b *testing.B) {
+	// a1, a2, b, r1, r2 *roaring.Bitmap
+	data := getBenchData(b)
+	for n := 0; n < b.N; n++ {
+		data.a1.
+			Union(data.a2).
+			Union(data.b).
+			Union(data.r1).
+			Union(data.r2)
+	}
+}
+
+// func BenchmarkUnionBulk(b *testing.B) {
+// 	// a1, a2, b, r1, r2 *roaring.Bitmap
+// 	data := getBenchData(b)
+// 	yolo := roaring.NewBitmap()
+// 	for n := 0; n < b.N; n++ {
+// 		yolo.
+// 			BulkUnion(data.a1, data.a2, data.b, data.r1, data.r2)
+// 	}
+// }

From 2e1f60ac426f2fac9388b2dbf61107b2676ed669 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Wed, 28 Nov 2018 18:40:32 -0500
Subject: [PATCH 05/61] horrible wip

---
 roaring/roaring.go | 252 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 232 insertions(+), 20 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 99f7a06fa..bc75b5119 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -411,31 +411,176 @@ func (b *Bitmap) UnionInPlace(others ...*Bitmap) {
 	}
 }
 
+type wrapperIter struct {
+	iter    ContainerIterator
+	hasNext bool
+	handled bool
+}
+
 // unionIntoTarget stores the union of b and other into target. b and other will
 // be left unchanged, but target will be modified in place. Used to share
 // the union logic between the copy-on-write and in-place functions.
-func (b *Bitmap) unionIntoTarget(other *Bitmap, target *Bitmap) {
-	iiter, _ := b.Containers.Iterator(0)
-	jiter, _ := other.Containers.Iterator(0)
-	i, j := iiter.Next(), jiter.Next()
-	ki, ci := iiter.Value()
-	kj, cj := jiter.Value()
-	for i || j {
-		if i && (!j || ki < kj) {
-			target.Containers.Put(ki, ci.Clone())
-			i = iiter.Next()
-			ki, ci = iiter.Value()
-		} else if j && (!i || ki > kj) {
-			target.Containers.Put(kj, cj.Clone())
-			j = jiter.Next()
-			kj, cj = jiter.Value()
-		} else { // ki == kj
-			target.Containers.Put(ki, union(ci, cj))
-			i, j = iiter.Next(), jiter.Next()
-			ki, ci = iiter.Value()
-			kj, cj = jiter.Value()
+func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
+	otherIters := make([]wrapperIter, 0, len(others))
+	for _, other := range others {
+		otherIter, _ := other.Containers.Iterator(0)
+		otherIters = append(otherIters, wrapperIter{
+			iter: otherIter,
+		})
+	}
+
+	// Loop until we've exhausted every iter.
+	for {
+		hasNext := false
+		for i, otherIter := range otherIters {
+			next := otherIter.iter.Next()
+			otherIters[i].hasNext = next
+			otherIters[i].handled = false
+			if next {
+				hasNext = true
+			}
+		}
+
+		if !hasNext {
+			// None of the iters had any more values, we're done.
+			break
+		}
+
+		// Loop until every iters current value has been handled.
+		for {
+			for i, iIter := range otherIters {
+				if !iIter.hasNext || iIter.handled {
+					continue
+				}
+
+				// Can store key-level statistics here
+				iKey, iContainer := iIter.iter.Value()
+				n := iContainer.n
+				needsUnion := false
+				hasMaxRange := iContainer.n == maxContainerVal+1
+				for _, jIter := range otherIters[i:] {
+					if hasMaxRange {
+						continue
+					}
+
+					// Calculate key-level statistics here
+					jKey, jContainer := jIter.iter.Value()
+
+					if iKey == jKey {
+						needsUnion = true
+						n += jContainer.n
+						if !hasMaxRange {
+							hasMaxRange = jContainer.n == maxContainerVal+1
+						}
+					}
+				}
+
+				if !needsUnion {
+					// TODO: Don't clone if sealed
+					target.Containers.Put(iKey, iContainer.Clone())
+					otherIters[i].handled = true
+					continue
+				}
+
+				// Need to union
+				if hasMaxRange {
+					// Use the max range
+					container := &Container{
+						runs:          []interval16{{start: 0, last: maxContainerVal}},
+						containerType: containerRun,
+						n:             maxContainerVal + 1,
+					}
+					target.Containers.Put(iKey, container)
+				} else {
+					// TODO: Implement this
+					// if n < ArrayMaxSize {
+					// 	// Use an array
+					// 	// container := &Container{
+					// 	// 	containerType: containerArray,
+					// 	// 	array:         make([]uint16, 0, n),
+					// 	// }
+					// }
+					// else {
+					// Use a bitmap
+					buf := make([]uint64, bitmapN)
+					ob := buf[:bitmapN]
+					output := &Container{
+						bitmap:        ob,
+						n:             n,
+						containerType: containerBitmap,
+					}
+
+					for _, jIter := range otherIters {
+						jKey, jContainer := jIter.iter.Value()
+
+						if iKey == jKey {
+						}
+					}
+				}
+			}
 		}
 	}
+	// iiter, _ := b.Containers.Iterator(0)
+	// i := iiter.Next()
+	// for i {
+	// 	for _, otherIter := range otherIters {
+	// 		jiter := otherIter
+	// 		j := jiter.Next()
+	// 		if !j {
+	// 			continue
+	// 		}
+
+	// 		ki, ci := iiter.Value()
+	// 		kj, cj := jiter.Value()
+
+	// 		if i && (!j || ki < kj) {
+	// 			// TODO: Don't clone if sealed
+	// 			target.Containers.Put(ki, ci.Clone())
+	// 			i = iiter.Next()
+	// 			ki, ci = iiter.Value()
+	// 		} else if j && (!i || ki > kj) {
+	// 			// TODO: Don't clone if sealed
+	// 			target.Containers.Put(kj, cj.Clone())
+	// 			j = jiter.Next()
+	// 			kj, cj = jiter.Value()
+	// 		} else { // ki == kj
+	// 			// TODO: unionIntoTarget?
+	// 			target.Containers.Put(ki, union(ci, cj))
+	// 			i, j = iiter.Next(), jiter.Next()
+	// 			ki, ci = iiter.Value()
+	// 			kj, cj = jiter.Value()
+	// 		}
+
+	// 	}
+	// 	}
+	// }
+
+	// for _, otherIter := range otherIters {
+	// 	jiter := otherIter
+
+	// 	i, j := iiter.Next(), jiter.Next()
+	// 	ki, ci := iiter.Value()
+	// 	kj, cj := jiter.Value()
+	// 	for i || j {
+	// 		if i && (!j || ki < kj) {
+	// 			// TODO: Don't clone if sealed
+	// 			target.Containers.Put(ki, ci.Clone())
+	// 			i = iiter.Next()
+	// 			ki, ci = iiter.Value()
+	// 		} else if j && (!i || ki > kj) {
+	// 			// TODO: Don't clone if sealed
+	// 			target.Containers.Put(kj, cj.Clone())
+	// 			j = jiter.Next()
+	// 			kj, cj = jiter.Value()
+	// 		} else { // ki == kj
+	// 			// TODO: unionIntoTarget?
+	// 			target.Containers.Put(ki, union(ci, cj))
+	// 			i, j = iiter.Next(), jiter.Next()
+	// 			ki, ci = iiter.Value()
+	// 			kj, cj = jiter.Value()
+	// 		}
+	// 	}
+	// }
 }
 
 // Difference returns the difference of b and other.
@@ -2289,6 +2434,38 @@ func unionArrayArray(a, b *Container) *Container {
 	return output
 }
 
+// func unionArrayArrayInPlace(a, b *Container) *Container {
+// 	statsHit("union/ArrayArray")
+// 	output := a
+// 	na, nb := len(a.array), len(b.array)
+// 	for i, j := 0, 0; ; {
+// 		if i >= na && j >= nb {
+// 			break
+// 		} else if i < na && j >= nb {
+// 			output.add(a.array[i])
+// 			i++
+// 			continue
+// 		} else if i >= na && j < nb {
+// 			output.add(b.array[j])
+// 			j++
+// 			continue
+// 		}
+
+// 		va, vb := a.array[i], b.array[j]
+// 		if va < vb {
+// 			output.add(va)
+// 			i++
+// 		} else if va > vb {
+// 			output.add(vb)
+// 			j++
+// 		} else {
+// 			output.add(va)
+// 			i, j = i+1, j+1
+// 		}
+// 	}
+// 	return output
+// }
+
 // unionArrayRun optimistically assumes that the result will be a run container,
 // and converts to a bitmap or array container afterwards if necessary.
 func unionArrayRun(a, b *Container) *Container {
@@ -2399,6 +2576,13 @@ func unionBitmapRun(a, b *Container) *Container {
 	return output
 }
 
+func unionBitmapRunInPlace(a, b *Container) {
+	statsHit("union/BitmapRun")
+	for j := 0; j < len(b.runs); j++ {
+		a.bitmapSetRange(uint64(b.runs[j].start), uint64(b.runs[j].last)+1)
+	}
+}
+
 const maxBitmap = 0xFFFFFFFFFFFFFFFF
 
 // sets all bits in [i, j) (c must be a bitmap container)
@@ -2518,6 +2702,15 @@ func unionArrayBitmap(a, b *Container) *Container {
 	return output
 }
 
+func unionBitmapArrayInPlace(a, b *Container) {
+	for _, v := range b.array {
+		if !a.bitmapContains(v) {
+			a.bitmap[v/64] |= (1 << uint64(v%64))
+			a.n++
+		}
+	}
+}
+
 func unionBitmapBitmap(a, b *Container) *Container {
 	// local variables added to prevent BCE checks in loop
 	// see https://go101.org/article/bounds-check-elimination.html
@@ -2544,6 +2737,25 @@ func unionBitmapBitmap(a, b *Container) *Container {
 	return output
 }
 
+func unionBitmapBitmapInPlace(a, b *Container) {
+	// local variables added to prevent BCE checks in loop
+	// see https://go101.org/article/bounds-check-elimination.html
+
+	var (
+		ab = a.bitmap[:bitmapN]
+		bb = b.bitmap[:bitmapN]
+
+		n int32
+	)
+
+	for i := 0; i < bitmapN; i++ {
+		ab[i] = ab[i] | bb[i]
+		n += int32(popcount(ab[i]))
+	}
+
+	a.n = n
+}
+
 func difference(a, b *Container) *Container {
 	if a.isArray() {
 		if b.isArray() {

From af3cb91a40c6b217afc8084cd164f2fff4c03d5d Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Wed, 28 Nov 2018 18:42:38 -0500
Subject: [PATCH 06/61] first

---
 roaring/roaring.go | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index bc75b5119..b8b5734c5 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -504,7 +504,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 					// Use a bitmap
 					buf := make([]uint64, bitmapN)
 					ob := buf[:bitmapN]
-					output := &Container{
+					container := &Container{
 						bitmap:        ob,
 						n:             n,
 						containerType: containerBitmap,
@@ -514,6 +514,13 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 						jKey, jContainer := jIter.iter.Value()
 
 						if iKey == jKey {
+							if jContainer.isArray() {
+								unionBitmapArrayInPlace(container, jContainer)
+							} else if jContainer.isRun() {
+								unionBitmapRunInPlace(container, jContainer)
+							} else {
+								unionBitmapBitmapInPlace(container, jContainer)
+							}
 						}
 					}
 				}

From 30946a037267664570c267f8abe72308e19725b1 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Wed, 28 Nov 2018 20:15:31 -0500
Subject: [PATCH 07/61] working

---
 roaring/roaring.go      | 280 ++++++++++++++++++----------------------
 roaring/roaring_test.go |   4 +
 2 files changed, 133 insertions(+), 151 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index b8b5734c5..7f88cb651 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -406,9 +406,10 @@ func (b *Bitmap) Union(others ...*Bitmap) *Bitmap {
 // UnionInPlace returns the bitwise union of b and other, modifying
 // b in place.
 func (b *Bitmap) UnionInPlace(others ...*Bitmap) {
-	for _, other := range others {
-		b.unionIntoTarget(other, b)
-	}
+	b.unionIntoTarget(b, others...)
+	// for _, other := range others {
+	// 	b.unionIntoTarget(other, b)
+	// }
 }
 
 type wrapperIter struct {
@@ -421,17 +422,135 @@ type wrapperIter struct {
 // be left unchanged, but target will be modified in place. Used to share
 // the union logic between the copy-on-write and in-place functions.
 func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
-	otherIters := make([]wrapperIter, 0, len(others))
-	for _, other := range others {
-		otherIter, _ := other.Containers.Iterator(0)
+	fmt.Println("huh: ", len(others))
+	otherIters := make([]wrapperIter, 0, len(others)+1)
+	bIter, _ := b.Containers.Iterator(0)
+	next := bIter.Next()
+	if next {
+		fmt.Println("Appending self")
 		otherIters = append(otherIters, wrapperIter{
-			iter: otherIter,
+			iter:    bIter,
+			hasNext: true,
+			handled: false,
 		})
 	}
 
+	for _, other := range others {
+		otherIter, _ := other.Containers.Iterator(0)
+		next := otherIter.Next()
+		if next {
+			otherIters = append(otherIters, wrapperIter{
+				iter:    otherIter,
+				hasNext: true,
+				handled: false,
+			})
+		}
+	}
+
 	// Loop until we've exhausted every iter.
-	for {
-		hasNext := false
+	hasNext := true
+	fmt.Println(len(otherIters))
+	for hasNext {
+		// Loop until every iters current value has been handled.
+		// for {
+		for i, iIter := range otherIters {
+			fmt.Println("copter")
+			if !iIter.hasNext || iIter.handled {
+				continue
+			}
+
+			fmt.Println("here?")
+			// Can store key-level statistics here
+			iKey, iContainer := iIter.iter.Value()
+			fmt.Println("iKey: ", iKey)
+			fmt.Println("iContainer: ", iContainer)
+			n := iContainer.n
+			needsUnion := false
+			hasMaxRange := iContainer.n == maxContainerVal+1
+			for _, jIter := range otherIters[i:] {
+				if hasMaxRange {
+					continue
+				}
+
+				// Calculate key-level statistics here
+				jKey, jContainer := jIter.iter.Value()
+				fmt.Println("jContainer.n: ", jContainer.n)
+
+				if iKey == jKey {
+					needsUnion = true
+					n += jContainer.n
+					if !hasMaxRange {
+						hasMaxRange = jContainer.n == maxContainerVal+1
+					}
+				}
+			}
+
+			if !needsUnion {
+				fmt.Println("Cloning")
+				// TODO: Don't clone if sealed
+				target.Containers.Put(iKey, iContainer.Clone())
+				otherIters[i].handled = true
+				continue
+			}
+
+			// Need to union
+			fmt.Println("ikey: ", iKey)
+			if hasMaxRange {
+				panic("maxRange")
+				fmt.Println("maxRange")
+				// Use the max range
+				container := &Container{
+					runs:          []interval16{{start: 0, last: maxContainerVal}},
+					containerType: containerRun,
+					n:             maxContainerVal + 1,
+				}
+				target.Containers.Put(iKey, container)
+			} else {
+				// TODO: Implement this
+				// if n < ArrayMaxSize {
+				// 	// Use an array
+				// 	// container := &Container{
+				// 	// 	containerType: containerArray,
+				// 	// 	array:         make([]uint16, 0, n),
+				// 	// }
+				// }
+				// else {
+				// Use a bitmap
+				fmt.Println("bitmap")
+				buf := make([]uint64, bitmapN)
+				ob := buf[:bitmapN]
+				container := &Container{
+					bitmap:        ob,
+					n:             0,
+					containerType: containerBitmap,
+				}
+
+				for _, jIter := range otherIters {
+					jKey, jContainer := jIter.iter.Value()
+
+					if iKey == jKey {
+						if jContainer.isArray() {
+							// panic("unionBitmapArrayInPlace")
+							fmt.Println("array into bitmap")
+							unionBitmapArrayInPlace(container, jContainer)
+							fmt.Println("After union array: ", container.n)
+						} else if jContainer.isRun() {
+							// panic("unionBitmapRunInPlace")
+							unionBitmapRunInPlace(container, jContainer)
+						} else {
+							fmt.Println("bitmap into bitmap")
+							// panic("unionBitmapBitmapInPlace")
+							unionBitmapBitmapInPlace(container, jContainer)
+							fmt.Println("After union bitmap: ", container.n)
+						}
+					}
+				}
+				target.Containers.Put(iKey, container)
+			}
+		}
+		// }
+
+		hasNext = false
 		for i, otherIter := range otherIters {
 			next := otherIter.iter.Next()
 			otherIters[i].hasNext = next
@@ -445,149 +564,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 			// None of the iters had any more values, we're done.
 			break
 		}
-
-		// Loop until every iters current value has been handled.
-		for {
-			for i, iIter := range otherIters {
-				if !iIter.hasNext || iIter.handled {
-					continue
-				}
-
-				// Can store key-level statistics here
-				iKey, iContainer := iIter.iter.Value()
-				n := iContainer.n
-				needsUnion := false
-				hasMaxRange := iContainer.n == maxContainerVal+1
-				for _, jIter := range otherIters[i:] {
-					if hasMaxRange {
-						continue
-					}
-
-					// Calculate key-level statistics here
-					jKey, jContainer := jIter.iter.Value()
-
-					if iKey == jKey {
-						needsUnion = true
-						n += jContainer.n
-						if !hasMaxRange {
-							hasMaxRange = jContainer.n == maxContainerVal+1
-						}
-					}
-				}
-
-				if !needsUnion {
-					// TODO: Don't clone if sealed
-					target.Containers.Put(iKey, iContainer.Clone())
-					otherIters[i].handled = true
-					continue
-				}
-
-				// Need to union
-				if hasMaxRange {
-					// Use the max range
-					container := &Container{
-						runs:          []interval16{{start: 0, last: maxContainerVal}},
-						containerType: containerRun,
-						n:             maxContainerVal + 1,
-					}
-					target.Containers.Put(iKey, container)
-				} else {
-					// TODO: Implement this
-					// if n < ArrayMaxSize {
-					// 	// Use an array
-					// 	// container := &Container{
-					// 	// 	containerType: containerArray,
-					// 	// 	array:         make([]uint16, 0, n),
-					// 	// }
-					// }
-					// else {
-					// Use a bitmap
-					buf := make([]uint64, bitmapN)
-					ob := buf[:bitmapN]
-					container := &Container{
-						bitmap:        ob,
-						n:             n,
-						containerType: containerBitmap,
-					}
-
-					for _, jIter := range otherIters {
-						jKey, jContainer := jIter.iter.Value()
-
-						if iKey == jKey {
-							if jContainer.isArray() {
-								unionBitmapArrayInPlace(container, jContainer)
-							} else if jContainer.isRun() {
-								unionBitmapRunInPlace(container, jContainer)
-							} else {
-								unionBitmapBitmapInPlace(container, jContainer)
-							}
-						}
-					}
-				}
-			}
-		}
 	}
-	// iiter, _ := b.Containers.Iterator(0)
-	// i := iiter.Next()
-	// for i {
-	// 	for _, otherIter := range otherIters {
-	// 		jiter := otherIter
-	// 		j := jiter.Next()
-	// 		if !j {
-	// 			continue
-	// 		}
-
-	// 		ki, ci := iiter.Value()
-	// 		kj, cj := jiter.Value()
-
-	// 		if i && (!j || ki < kj) {
-	// 			// TODO: Don't clone if sealed
-	// 			target.Containers.Put(ki, ci.Clone())
-	// 			i = iiter.Next()
-	// 			ki, ci = iiter.Value()
-	// 		} else if j && (!i || ki > kj) {
-	// 			// TODO: Don't clone if sealed
-	// 			target.Containers.Put(kj, cj.Clone())
-	// 			j = jiter.Next()
-	// 			kj, cj = jiter.Value()
-	// 		} else { // ki == kj
-	// 			// TODO: unionIntoTarget?
-	// 			target.Containers.Put(ki, union(ci, cj))
-	// 			i, j = iiter.Next(), jiter.Next()
-	// 			ki, ci = iiter.Value()
-	// 			kj, cj = jiter.Value()
-	// 		}
-
-	// 	}
-	// 	}
-	// }
-
-	// for _, otherIter := range otherIters {
-	// 	jiter := otherIter
-
-	// 	i, j := iiter.Next(), jiter.Next()
-	// 	ki, ci := iiter.Value()
-	// 	kj, cj := jiter.Value()
-	// 	for i || j {
-	// 		if i && (!j || ki < kj) {
-	// 			// TODO: Don't clone if sealed
-	// 			target.Containers.Put(ki, ci.Clone())
-	// 			i = iiter.Next()
-	// 			ki, ci = iiter.Value()
-	// 		} else if j && (!i || ki > kj) {
-	// 			// TODO: Don't clone if sealed
-	// 			target.Containers.Put(kj, cj.Clone())
-	// 			j = jiter.Next()
-	// 			kj, cj = jiter.Value()
-	// 		} else { // ki == kj
-	// 			// TODO: unionIntoTarget?
-	// 			target.Containers.Put(ki, union(ci, cj))
-	// 			i, j = iiter.Next(), jiter.Next()
-	// 			ki, ci = iiter.Value()
-	// 			kj, cj = jiter.Value()
-	// 		}
-	// 	}
-	// }
 }
 
 // Difference returns the difference of b and other.
@@ -2714,6 +2691,7 @@ func unionBitmapArrayInPlace(a, b *Container) {
 		if !a.bitmapContains(v) {
 			a.bitmap[v/64] |= (1 << uint64(v%64))
 			a.n++
+			fmt.Println("added: ", v)
 		}
 	}
 }
diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go
index f554d1df3..18d4858ae 100644
--- a/roaring/roaring_test.go
+++ b/roaring/roaring_test.go
@@ -411,10 +411,14 @@ func TestBitmap_UnionInPlace1(t *testing.T) {
 
 	result.UnionInPlace(bm0, bm1)
 	if n := result.Count(); n != 2682675 {
+		// for _, val := range result.Slice() {
+		// 	fmt.Println(val)
+		// }
 		t.Fatalf("unexpected n: %d", n)
 	}
 
 	bm := testBM()
+	fmt.Println("bm.Count(): ", bm.Count())
 	result = roaring.NewBitmap()
 	result.UnionInPlace(bm, bm0)
 	if n := result.Count(); n != 75009 {

From e8e4369f763c6c18404829884086d9d10931a707 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Wed, 28 Nov 2018 20:18:18 -0500
Subject: [PATCH 08/61] all passing

---
 roaring/roaring.go      | 22 ----------------------
 roaring/roaring_test.go |  4 ----
 2 files changed, 26 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 7f88cb651..0e288d25e 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -422,12 +422,10 @@ type wrapperIter struct {
 // be left unchanged, but target will be modified in place. Used to share
 // the union logic between the copy-on-write and in-place functions.
 func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
-	fmt.Println("huh: ", len(others))
 	otherIters := make([]wrapperIter, 0, len(others)+1)
 	bIter, _ := b.Containers.Iterator(0)
 	next := bIter.Next()
 	if next {
-		fmt.Println("Appending self")
 		otherIters = append(otherIters, wrapperIter{
 			iter:    bIter,
 			hasNext: true,
@@ -449,21 +447,16 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 
 	// Loop until we've exhausted every iter.
 	hasNext := true
-	fmt.Println(len(otherIters))
 	for hasNext {
 		// Loop until every iters current value has been handled.
 		// for {
 		for i, iIter := range otherIters {
-			fmt.Println("copter")
 			if !iIter.hasNext || iIter.handled {
 				continue
 			}
 
-			fmt.Println("here?")
 			// Can store key-level statistics here
 			iKey, iContainer := iIter.iter.Value()
-			fmt.Println("iKey: ", iKey)
-			fmt.Println("iContainer: ", iContainer)
 			n := iContainer.n
 			needsUnion := false
 			hasMaxRange := iContainer.n == maxContainerVal+1
@@ -474,7 +467,6 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 
 				// Calculate key-level statistics here
 				jKey, jContainer := jIter.iter.Value()
-				fmt.Println("jContainer.n: ", jContainer.n)
 
 				if iKey == jKey {
 					needsUnion = true
@@ -486,7 +478,6 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 			}
 
 			if !needsUnion {
-				fmt.Println("Cloning")
 				// TODO: Don't clone if sealed
 				target.Containers.Put(iKey, iContainer.Clone())
 				otherIters[i].handled = true
@@ -494,10 +485,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 			}
 
 			// Need to union
-			fmt.Println("ikey: ", iKey)
 			if hasMaxRange {
-				panic("maxRange")
-				fmt.Println("maxRange")
 				// Use the max range
 				container := &Container{
 					runs:          []interval16{{start: 0, last: maxContainerVal}},
@@ -516,7 +504,6 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				// }
 				// else {
 				// Use a bitmap
-				fmt.Println("bitmap")
 				buf := make([]uint64, bitmapN)
 				ob := buf[:bitmapN]
 				container := &Container{
@@ -530,25 +517,17 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 
 					if iKey == jKey {
 						if jContainer.isArray() {
-							// panic("unionBitmapArrayInPlace")
-							fmt.Println("array into bitmap")
 							unionBitmapArrayInPlace(container, jContainer)
-							fmt.Println("After union array: ", container.n)
 						} else if jContainer.isRun() {
-							// panic("unionBitmapRunInPlace")
 							unionBitmapRunInPlace(container, jContainer)
 						} else {
-							fmt.Println("bitmap into bitmap")
-							// panic("unionBitmapBitmapInPlace")
 							unionBitmapBitmapInPlace(container, jContainer)
-							fmt.Println("After union bitmap: ", container.n)
 						}
 					}
 				}
 				target.Containers.Put(iKey, container)
 			}
 		}
-		// }
 
 		hasNext = false
 		for i, otherIter := range otherIters {
@@ -2691,7 +2670,6 @@ func unionBitmapArrayInPlace(a, b *Container) {
 		if !a.bitmapContains(v) {
 			a.bitmap[v/64] |= (1 << uint64(v%64))
 			a.n++
-			fmt.Println("added: ", v)
 		}
 	}
 }
diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go
index 18d4858ae..f554d1df3 100644
--- a/roaring/roaring_test.go
+++ b/roaring/roaring_test.go
@@ -411,14 +411,10 @@ func TestBitmap_UnionInPlace1(t *testing.T) {
 
 	result.UnionInPlace(bm0, bm1)
 	if n := result.Count(); n != 2682675 {
-		// for _, val := range result.Slice() {
-		// 	fmt.Println(val)
-		// }
 		t.Fatalf("unexpected n: %d", n)
 	}
 
 	bm := testBM()
-	fmt.Println("bm.Count(): ", bm.Count())
 	result = roaring.NewBitmap()
 	result.UnionInPlace(bm, bm0)
 	if n := result.Count(); n != 75009 {

From 339a78d88d2e518faef9794723f0296680b13ae2 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Wed, 28 Nov 2018 21:34:44 -0500
Subject: [PATCH 09/61] wokring

---
 roaring/roaring.go      | 18 +++++++++---------
 roaring/roaring_test.go | 18 +++++++++---------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 0e288d25e..7685f7a94 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -407,9 +407,6 @@ func (b *Bitmap) Union(others ...*Bitmap) *Bitmap {
 // b in place.
 func (b *Bitmap) UnionInPlace(others ...*Bitmap) {
 	b.unionIntoTarget(b, others...)
-	// for _, other := range others {
-	// 	b.unionIntoTarget(other, b)
-	// }
 }
 
 type wrapperIter struct {
@@ -504,12 +501,15 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				// }
 				// else {
 				// Use a bitmap
-				buf := make([]uint64, bitmapN)
-				ob := buf[:bitmapN]
-				container := &Container{
-					bitmap:        ob,
-					n:             0,
-					containerType: containerBitmap,
+				container := target.Containers.Get(iKey)
+				if container == nil {
+					buf := make([]uint64, bitmapN)
+					ob := buf[:bitmapN]
+					container = &Container{
+						bitmap:        ob,
+						n:             0,
+						containerType: containerBitmap,
+					}
 				}
 
 				for _, jIter := range otherIters {
diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go
index f554d1df3..8e389c586 100644
--- a/roaring/roaring_test.go
+++ b/roaring/roaring_test.go
@@ -1424,12 +1424,12 @@ func BenchmarkUnion(b *testing.B) {
 	}
 }
 
-// func BenchmarkUnionBulk(b *testing.B) {
-// 	// a1, a2, b, r1, r2 *roaring.Bitmap
-// 	data := getBenchData(b)
-// 	yolo := roaring.NewBitmap()
-// 	for n := 0; n < b.N; n++ {
-// 		yolo.
-// 			BulkUnion(data.a1, data.a2, data.b, data.r1, data.r2)
-// 	}
-// }
+func BenchmarkUnionBulk(b *testing.B) {
+	// a1, a2, b, r1, r2 *roaring.Bitmap
+	data := getBenchData(b)
+	yolo := roaring.NewBitmap()
+	for n := 0; n < b.N; n++ {
+		yolo.
+			UnionInPlace(data.a1, data.a2, data.b, data.r1, data.r2)
+	}
+}

From b47292aa26842c4c5d44ec0d52dc56a9f1ec35a8 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Wed, 28 Nov 2018 21:43:28 -0500
Subject: [PATCH 10/61] switch to bitmap repairs for inplace algo

---
 roaring/roaring.go | 45 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 7685f7a94..3085b22bd 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -544,6 +544,19 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 			break
 		}
 	}
+
+	// Repair bitmaps after the fact
+	iter, _ := target.Containers.Iterator(0)
+	for iter.Next() {
+		_, container := iter.Value()
+		if container.isBitmap() {
+			n := int32(0)
+			for i := 0; i < bitmapN; i++ {
+				n += int32(popcount(container.bitmap[i]))
+			}
+			container.n = n
+		}
+	}
 }
 
 // Difference returns the difference of b and other.
@@ -2539,10 +2552,12 @@ func unionBitmapRun(a, b *Container) *Container {
 	return output
 }
 
+// unions the run b into the bitmap a, mutating a in place. The n value of
+// a will need to be repaired after the fact.
 func unionBitmapRunInPlace(a, b *Container) {
 	statsHit("union/BitmapRun")
 	for j := 0; j < len(b.runs); j++ {
-		a.bitmapSetRange(uint64(b.runs[j].start), uint64(b.runs[j].last)+1)
+		a.bitmapSetRangeIgnoreN(uint64(b.runs[j].start), uint64(b.runs[j].last)+1)
 	}
 }
 
@@ -2571,6 +2586,25 @@ func (c *Container) bitmapSetRange(i, j uint64) {
 	}
 }
 
+// sets all bits in [i, j) (c must be a bitmap container) without updating
+// the value of n, meaning it will need to be repaired after the fact.
+func (c *Container) bitmapSetRangeIgnoreN(i, j uint64) {
+	x := i >> 6
+	y := (j - 1) >> 6
+	var X uint64 = maxBitmap << (i % 64)
+	var Y uint64 = maxBitmap >> (63 - ((j - 1) % 64))
+
+	if x == y {
+		c.bitmap[x] |= (X & Y)
+	} else {
+		c.bitmap[x] |= X
+		for i := x + 1; i < y; i++ {
+			c.bitmap[i] = maxBitmap
+		}
+		c.bitmap[y] |= Y
+	}
+}
+
 // xor's all bits in [i, j) with all true (c must be a bitmap container).
 func (c *Container) bitmapXorRange(i, j uint64) {
 	x := i >> 6
@@ -2665,6 +2699,8 @@ func unionArrayBitmap(a, b *Container) *Container {
 	return output
 }
 
+// unions array b into bitmap a, mutating a in place. The n value
+// of a will need to be repaired after the fact.
 func unionBitmapArrayInPlace(a, b *Container) {
 	for _, v := range b.array {
 		if !a.bitmapContains(v) {
@@ -2700,6 +2736,8 @@ func unionBitmapBitmap(a, b *Container) *Container {
 	return output
 }
 
+// unions bitmap b into bitmap a, mutating a in place. The n value of
+// a will need to be repaired after the fact.
 func unionBitmapBitmapInPlace(a, b *Container) {
 	// local variables added to prevent BCE checks in loop
 	// see https://go101.org/article/bounds-check-elimination.html
@@ -2707,16 +2745,11 @@ func unionBitmapBitmapInPlace(a, b *Container) {
 	var (
 		ab = a.bitmap[:bitmapN]
 		bb = b.bitmap[:bitmapN]
-
-		n int32
 	)
 
 	for i := 0; i < bitmapN; i++ {
 		ab[i] = ab[i] | bb[i]
-		n += int32(popcount(ab[i]))
 	}
-
-	a.n = n
 }
 
 func difference(a, b *Container) *Container {

From eb5ad7bf49a494e25d9f3aca38d4e0ce6e969749 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Wed, 28 Nov 2018 21:44:37 -0500
Subject: [PATCH 11/61] dont keep n in sync with bitmaprun in place

---
 roaring/roaring.go | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 3085b22bd..98285d543 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -2703,10 +2703,7 @@ func unionArrayBitmap(a, b *Container) *Container {
 // of a will need to be repaired after the fact.
 func unionBitmapArrayInPlace(a, b *Container) {
 	for _, v := range b.array {
-		if !a.bitmapContains(v) {
-			a.bitmap[v/64] |= (1 << uint64(v%64))
-			a.n++
-		}
+		a.bitmap[v/64] |= (1 << uint64(v%64))
 	}
 }
 

From 3eb060c4c681497b85c7ca38749ba71c6bc80076 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 00:15:57 -0500
Subject: [PATCH 12/61] fix bug

---
 roaring/roaring.go | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 98285d543..f152a5656 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -27,6 +27,8 @@ import (
 	"github.com/pkg/errors"
 )
 
+func statshit() {}
+
 const (
 	// magicNumber is an identifier, in bytes 0-1 of the file.
 	magicNumber = uint32(12348)
@@ -419,6 +421,15 @@ type wrapperIter struct {
 // be left unchanged, but target will be modified in place. Used to share
 // the union logic between the copy-on-write and in-place functions.
 func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
+	numArrayIntoBitmap := 0
+	numRunIntoBitmap := 0
+	numBitmapIntoBitmap := 0
+	// defer func() {
+	// 	fmt.Println("numArrayIntoBitmap: ", numArrayIntoBitmap)
+	// 	fmt.Println("numRunIntoBitmap: ", numRunIntoBitmap)
+	// 	fmt.Println("numBitmapIntoBitmap: ", numBitmapIntoBitmap)
+	// }()
+
 	otherIters := make([]wrapperIter, 0, len(others)+1)
 	bIter, _ := b.Containers.Iterator(0)
 	next := bIter.Next()
@@ -490,6 +501,12 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 					n:             maxContainerVal + 1,
 				}
 				target.Containers.Put(iKey, container)
+				for j, jIter := range otherIters {
+					jKey, _ := jIter.iter.Value()
+					if iKey == jKey {
+						otherIters[j].handled = true
+					}
+				}
 			} else {
 				// TODO: Implement this
 				// if n < ArrayMaxSize {
@@ -512,17 +529,21 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 					}
 				}
 
-				for _, jIter := range otherIters {
+				for j, jIter := range otherIters {
 					jKey, jContainer := jIter.iter.Value()
 
 					if iKey == jKey {
 						if jContainer.isArray() {
+							numArrayIntoBitmap++
 							unionBitmapArrayInPlace(container, jContainer)
 						} else if jContainer.isRun() {
+							numRunIntoBitmap++
 							unionBitmapRunInPlace(container, jContainer)
 						} else {
+							numBitmapIntoBitmap++
 							unionBitmapBitmapInPlace(container, jContainer)
 						}
+						otherIters[j].handled = true
 					}
 				}
 				target.Containers.Put(iKey, container)
@@ -2703,7 +2724,9 @@ func unionArrayBitmap(a, b *Container) *Container {
 // of a will need to be repaired after the fact.
 func unionBitmapArrayInPlace(a, b *Container) {
 	for _, v := range b.array {
-		a.bitmap[v/64] |= (1 << uint64(v%64))
+		// a.bitmap[v>>6] |= (1 << uint64(v%64))
+		i := v >> 6
+		a.bitmap[i] = a.bitmap[i] | (uint64(1) << (v % 64))
 	}
 }
 

From 50f119df4e21779495057ef43ef2b9451c2a8387 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 10:02:24 -0500
Subject: [PATCH 13/61] Allocate bitmap if needed (existing wrong type

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index f152a5656..9b40399f0 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -519,7 +519,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				// else {
 				// Use a bitmap
 				container := target.Containers.Get(iKey)
-				if container == nil {
+				if container == nil || container.containerType != containerBitmap {
 					buf := make([]uint64, bitmapN)
 					ob := buf[:bitmapN]
 					container = &Container{

From d78e2fc87c896d32927a873d3ce5a80dd6be90f7 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 11:31:54 -0500
Subject: [PATCH 14/61] Move repair logic to helpers

---
 roaring/containers.go |  8 ++++++++
 roaring/roaring.go    | 23 ++++++++++++-----------
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/roaring/containers.go b/roaring/containers.go
index 3fe0814cc..9862b894c 100644
--- a/roaring/containers.go
+++ b/roaring/containers.go
@@ -156,6 +156,14 @@ func (sc *sliceContainers) Iterator(key uint64) (citer ContainerIterator, found
 	return &sliceIterator{e: sc, i: i}, found
 }
 
+func (sc *sliceContainers) Repair() {
+	for _, c := range sc.containers {
+		if c.isBitmap() {
+			c.bitmapRepair()
+		}
+	}
+}
+
 type sliceIterator struct {
 	e     *sliceContainers
 	i     int
diff --git a/roaring/roaring.go b/roaring/roaring.go
index 9b40399f0..60476d89f 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -100,6 +100,9 @@ type Containers interface {
 
 	Count() uint64
 
+	// Repair will n values after in-place operations.
+	Repair()
+
 	//Reset will clear the containers collection to allow for recycling during snapshot
 	Reset()
 }
@@ -567,17 +570,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 	}
 
 	// Repair bitmaps after the fact
-	iter, _ := target.Containers.Iterator(0)
-	for iter.Next() {
-		_, container := iter.Value()
-		if container.isBitmap() {
-			n := int32(0)
-			for i := 0; i < bitmapN; i++ {
-				n += int32(popcount(container.bitmap[i]))
-			}
-			container.n = n
-		}
-	}
+	target.Containers.Repair()
 }
 
 // Difference returns the difference of b and other.
@@ -1981,6 +1974,14 @@ func (c *Container) check() error {
 	return a
 }
 
+func (c *Container) bitmapRepair() {
+	n := int32(0)
+	for i := 0; i < bitmapN; i++ {
+		n += int32(popcount(c.bitmap[i]))
+	}
+	c.n = n
+}
+
 // containerInfo represents a point-in-time snapshot of container stats.
 type containerInfo struct {
 	Key     uint64         // container key

From fccc7070060720cb8174ea635a95a80d1a41bede Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 11:32:44 -0500
Subject: [PATCH 15/61] remove debug code

---
 roaring/roaring.go | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 60476d89f..b2b6fdf39 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -424,15 +424,6 @@ type wrapperIter struct {
 // be left unchanged, but target will be modified in place. Used to share
 // the union logic between the copy-on-write and in-place functions.
 func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
-	numArrayIntoBitmap := 0
-	numRunIntoBitmap := 0
-	numBitmapIntoBitmap := 0
-	// defer func() {
-	// 	fmt.Println("numArrayIntoBitmap: ", numArrayIntoBitmap)
-	// 	fmt.Println("numRunIntoBitmap: ", numRunIntoBitmap)
-	// 	fmt.Println("numBitmapIntoBitmap: ", numBitmapIntoBitmap)
-	// }()
-
 	otherIters := make([]wrapperIter, 0, len(others)+1)
 	bIter, _ := b.Containers.Iterator(0)
 	next := bIter.Next()
@@ -537,13 +528,10 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 
 					if iKey == jKey {
 						if jContainer.isArray() {
-							numArrayIntoBitmap++
 							unionBitmapArrayInPlace(container, jContainer)
 						} else if jContainer.isRun() {
-							numRunIntoBitmap++
 							unionBitmapRunInPlace(container, jContainer)
 						} else {
-							numBitmapIntoBitmap++
 							unionBitmapBitmapInPlace(container, jContainer)
 						}
 						otherIters[j].handled = true

From 13dbe22b18163d1e4d3759b4c06b3f80b8c24234 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 13:53:10 -0500
Subject: [PATCH 16/61] Move next logic into helper

---
 roaring/roaring.go | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index b2b6fdf39..bbafbae69 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -420,11 +420,28 @@ type wrapperIter struct {
 	handled bool
 }
 
+type wrappedIters []wrapperIter
+
+func (w wrappedIters) next() bool {
+	hasNext := false
+
+	for i, wrapped := range w {
+		next := wrapped.iter.Next()
+		w[i].hasNext = next
+		w[i].handled = false
+		if next {
+			hasNext = true
+		}
+	}
+
+	return hasNext
+}
+
 // unionIntoTarget stores the union of b and other into target. b and other will
 // be left unchanged, but target will be modified in place. Used to share
 // the union logic between the copy-on-write and in-place functions.
 func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
-	otherIters := make([]wrapperIter, 0, len(others)+1)
+	otherIters := make(wrappedIters, 0, len(others)+1)
 	bIter, _ := b.Containers.Iterator(0)
 	next := bIter.Next()
 	if next {
@@ -541,15 +558,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 			}
 		}
 
-		hasNext = false
-		for i, otherIter := range otherIters {
-			next := otherIter.iter.Next()
-			otherIters[i].hasNext = next
-			otherIters[i].handled = false
-			if next {
-				hasNext = true
-			}
-		}
+		hasNext = otherIters.next()
 
 		if !hasNext {
 			// None of the iters had any more values, we're done.

From 5b50ecfd8e62b299f0829d6d38cfe41280068bc2 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 13:53:38 -0500
Subject: [PATCH 17/61] delete unused code

---
 roaring/roaring.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index bbafbae69..77419c2db 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -468,7 +468,6 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 	hasNext := true
 	for hasNext {
 		// Loop until every iters current value has been handled.
-		// for {
 		for i, iIter := range otherIters {
 			if !iIter.hasNext || iIter.handled {
 				continue

From 8e3da346c3bf6e5365d592a06f268627040c97b7 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 14:16:31 -0500
Subject: [PATCH 18/61] Add more comments and add helper method for bulk
 marking handled

---
 roaring/roaring.go | 64 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 49 insertions(+), 15 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 77419c2db..ec0cec4d9 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -437,6 +437,15 @@ func (w wrappedIters) next() bool {
 	return hasNext
 }
 
+func (w wrappedIters) markItersWithCurrentKeyAsHandled(key uint64) {
+	for i, wrapped := range w {
+		currKey, _ := wrapped.iter.Value()
+		if currKey == key {
+			w[i].handled = true
+		}
+	}
+}
+
 // unionIntoTarget stores the union of b and other into target. b and other will
 // be left unchanged, but target will be modified in place. Used to share
 // the union logic between the copy-on-write and in-place functions.
@@ -470,16 +479,40 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 		// Loop until every iters current value has been handled.
 		for i, iIter := range otherIters {
 			if !iIter.hasNext || iIter.handled {
+				// Either we've exhausted this iter (it has no more containers), or
+				// we've already handled the current container by unioning it with
+				// one of the containers we encountered earlier.
 				continue
 			}
 
-			// Can store key-level statistics here
 			iKey, iContainer := iIter.iter.Value()
-			n := iContainer.n
-			needsUnion := false
-			hasMaxRange := iContainer.n == maxContainerVal+1
+
+			// Summary statistics about all the containers in the other bitmaps
+			// that share the same key so we can make smarter union strategy
+			// decisions later.
+			var (
+				// Estimated cardinality of the union of all containers with the same
+				// key as iKey across all bitmaps. This calculation is very rough as
+				// we just sum the cardinality of the container across the different
+				// bitmaps which could result in very inflated values, but it allows
+				// us to avoid allocating expensive bitmaps when unioning many low
+				// density containers.
+				n = iContainer.n
+				// Whether iContainer is the only container across all the bitmaps
+				// with the key iKey. If true, we can skip all the unioning logic
+				// and just clone the container into target.
+				isOnlyContainerWithKey = true
+				// Whether any of the containers are storing every possible value that
+				// they can. If so, we can short-circuit all the unioning logic and use
+				// a RLE container with a single value in it. This is an optimization to
+				// avoid using an expensive bitmap container for bitmaps that have some
+				// extremely dense containers.
+				hasMaxRange = iContainer.n == maxContainerVal+1
+			)
 			for _, jIter := range otherIters[i:] {
 				if hasMaxRange {
+					// If we already know that we're going to use a max range RLE container,
+					// then there is no reason to continue calculating statistics.
 					continue
 				}
 
@@ -487,7 +520,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				jKey, jContainer := jIter.iter.Value()
 
 				if iKey == jKey {
-					needsUnion = true
+					isOnlyContainerWithKey = false
 					n += jContainer.n
 					if !hasMaxRange {
 						hasMaxRange = jContainer.n == maxContainerVal+1
@@ -495,28 +528,29 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				}
 			}
 
-			if !needsUnion {
-				// TODO: Don't clone if sealed
+			if isOnlyContainerWithKey {
+				// TODO(rartoul): We can avoid these clones if we can determine
+				// if the container is coming from an immutable bitmap and we
+				// know that we can mark the target bitmap as immutable as well.
 				target.Containers.Put(iKey, iContainer.Clone())
 				otherIters[i].handled = true
 				continue
 			}
 
-			// Need to union
+			// There was more than one container across the bitmaps with key iKey
+			// so we need to calculate a union.
 			if hasMaxRange {
-				// Use the max range
+				// One (or more) of the containers represented the maximum possible
+				// range that a container can store, so instead of calculating a
+				// union we can generate an RLE container that represents the entire
+				// range.
 				container := &Container{
 					runs:          []interval16{{start: 0, last: maxContainerVal}},
 					containerType: containerRun,
 					n:             maxContainerVal + 1,
 				}
 				target.Containers.Put(iKey, container)
-				for j, jIter := range otherIters {
-					jKey, _ := jIter.iter.Value()
-					if iKey == jKey {
-						otherIters[j].handled = true
-					}
-				}
+				otherIters[i:].markItersWithCurrentKeyAsHandled(iKey)
 			} else {
 				// TODO: Implement this
 				// if n < ArrayMaxSize {

From c02479a5f9063d49b444a0f9dc600416629c72d0 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 14:23:11 -0500
Subject: [PATCH 19/61] more comments and cleanup

---
 roaring/roaring.go | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index ec0cec4d9..457973e8f 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -552,17 +552,18 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				target.Containers.Put(iKey, container)
 				otherIters[i:].markItersWithCurrentKeyAsHandled(iKey)
 			} else {
-				// TODO: Implement this
-				// if n < ArrayMaxSize {
-				// 	// Use an array
-				// 	// container := &Container{
-				// 	// 	containerType: containerArray,
-				// 	// 	array:         make([]uint16, 0, n),
-				// 	// }
-				// }
-				// else {
-				// Use a bitmap
+				// Use a bitmap container for the target bitmap, and union everything
+				// into it.
+				//
+				// TODO(rartoul): Add another conditional case for n < ArrayMaxSize
+				// (or some fraction of that value) to avoid allocating expensive
+				// bitmaps when unioning many low-density array containers, but this
+				// will require writing a union in place algorithm for an array container
+				// that accepts multiple different containers to union into it for
+				// efficiency.
 				container := target.Containers.Get(iKey)
+				// If target already has a bitmap container for iKey then we can reuse that,
+				// otherwise we have to allocate a new one.
 				if container == nil || container.containerType != containerBitmap {
 					buf := make([]uint64, bitmapN)
 					ob := buf[:bitmapN]
@@ -573,6 +574,10 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 					}
 				}
 
+				// Once we've acquire a bitmap container (either by reusing the existing one
+				// or allocating a new one) then the last step is to iterate through all the
+				// other containers to see which ones have the same key, and union all of them
+				// into the target bitmap container.
 				for j, jIter := range otherIters {
 					jKey, jContainer := jIter.iter.Value()
 
@@ -592,11 +597,6 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 		}
 
 		hasNext = otherIters.next()
-
-		if !hasNext {
-			// None of the iters had any more values, we're done.
-			break
-		}
 	}
 
 	// Repair bitmaps after the fact

From defcb40f8c177e4eeedd76e6a779e838e55be659 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 14:25:39 -0500
Subject: [PATCH 20/61] Add comment

---
 roaring/roaring.go | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 457973e8f..e0fa9948c 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -599,7 +599,12 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 		hasNext = otherIters.next()
 	}
 
-	// Repair bitmaps after the fact
+	// Performing the popcount() operation with every union is wasteful because
+	// its likely the value will be invalidated by the next union operation. As
+	// a result, when we're performing all our in-place unions, we don't repair
+	// the value of n (container cardinality), and then at the very end we perform
+	// a "Repair" to recalculate all the container values. That way we never popcount()
+	// an entire bitmap container more than once per bulk union operation.
 	target.Containers.Repair()
 }
 

From 7931bc2c3703ef22adfb601fa0e21820699e84f2 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 14:26:57 -0500
Subject: [PATCH 21/61] Add comment

---
 roaring/roaring.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index e0fa9948c..8d612b5e5 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -592,6 +592,10 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 						otherIters[j].handled = true
 					}
 				}
+
+				// Now that we've calculated a container is that a union of all the containers
+				// with the same key across all the bitmaps, we store it in the list of containers
+				// for the target.
 				target.Containers.Put(iKey, container)
 			}
 		}

From 06e7dbdc757f797c569d7a82c479858ca6814fd6 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 14:30:16 -0500
Subject: [PATCH 22/61] simplify and remove dead code

---
 roaring/roaring.go | 36 +-----------------------------------
 1 file changed, 1 insertion(+), 35 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 8d612b5e5..269f9ec57 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -2471,38 +2471,6 @@ func unionArrayArray(a, b *Container) *Container {
 	return output
 }
 
-// func unionArrayArrayInPlace(a, b *Container) *Container {
-// 	statsHit("union/ArrayArray")
-// 	output := a
-// 	na, nb := len(a.array), len(b.array)
-// 	for i, j := 0, 0; ; {
-// 		if i >= na && j >= nb {
-// 			break
-// 		} else if i < na && j >= nb {
-// 			output.add(a.array[i])
-// 			i++
-// 			continue
-// 		} else if i >= na && j < nb {
-// 			output.add(b.array[j])
-// 			j++
-// 			continue
-// 		}
-
-// 		va, vb := a.array[i], b.array[j]
-// 		if va < vb {
-// 			output.add(va)
-// 			i++
-// 		} else if va > vb {
-// 			output.add(vb)
-// 			j++
-// 		} else {
-// 			output.add(va)
-// 			i, j = i+1, j+1
-// 		}
-// 	}
-// 	return output
-// }
-
 // unionArrayRun optimistically assumes that the result will be a run container,
 // and converts to a bitmap or array container afterwards if necessary.
 func unionArrayRun(a, b *Container) *Container {
@@ -2764,9 +2732,7 @@ func unionArrayBitmap(a, b *Container) *Container {
 // of a will need to be repaired after the fact.
 func unionBitmapArrayInPlace(a, b *Container) {
 	for _, v := range b.array {
-		// a.bitmap[v>>6] |= (1 << uint64(v%64))
-		i := v >> 6
-		a.bitmap[i] = a.bitmap[i] | (uint64(1) << (v % 64))
+		a.bitmap[v>>6] |= (uint64(1) << (v % 64))
 	}
 }
 

From 4cdf2adbe509a669d4edee7ae1abe89ddca9534f Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 15:33:26 -0500
Subject: [PATCH 23/61] fix comment

---
 roaring/roaring.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 269f9ec57..aef03a82c 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -605,8 +605,8 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 
 	// Performing the popcount() operation with every union is wasteful because
 	// its likely the value will be invalidated by the next union operation. As
-	// a result, when we're performing all our in-place unions, we don't repair
-	// the value of n (container cardinality), and then at the very end we perform
+	// a result, when we're performing all our in-place unions we allow the value of
+	// n (container cardinality) to fall out of sync, and then at the very end we perform
 	// a "Repair" to recalculate all the container values. That way we never popcount()
 	// an entire bitmap container more than once per bulk union operation.
 	target.Containers.Repair()

From 49df3fcd305fbd4bcb15476a079ee5015e9e3f7c Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 15:34:29 -0500
Subject: [PATCH 24/61] Dont shadow statshit

---
 roaring/roaring.go | 2 --
 1 file changed, 2 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index aef03a82c..bed3672e5 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -27,8 +27,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-func statshit() {}
-
 const (
 	// magicNumber is an identifier, in bytes 0-1 of the file.
 	magicNumber = uint32(12348)

From 70338be7ca7ecf8ba9d77f680e7483c097d49254 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:25:12 -0500
Subject: [PATCH 25/61] Add crazy comment

---
 roaring/roaring.go | 163 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 130 insertions(+), 33 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index bed3672e5..8efd6e0cf 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -406,47 +406,112 @@ func (b *Bitmap) Union(others ...*Bitmap) *Bitmap {
 	return output
 }
 
-// UnionInPlace returns the bitwise union of b and other, modifying
+// UnionInPlace returns the bitwise union of b and others, modifying
 // b in place.
 func (b *Bitmap) UnionInPlace(others ...*Bitmap) {
 	b.unionIntoTarget(b, others...)
 }
 
-type wrapperIter struct {
-	iter    ContainerIterator
-	hasNext bool
-	handled bool
-}
-
-type wrappedIters []wrapperIter
-
-func (w wrappedIters) next() bool {
-	hasNext := false
-
-	for i, wrapped := range w {
-		next := wrapped.iter.Next()
-		w[i].hasNext = next
-		w[i].handled = false
-		if next {
-			hasNext = true
-		}
-	}
-
-	return hasNext
-}
-
-func (w wrappedIters) markItersWithCurrentKeyAsHandled(key uint64) {
-	for i, wrapped := range w {
-		currKey, _ := wrapped.iter.Value()
-		if currKey == key {
-			w[i].handled = true
-		}
-	}
-}
-
 // unionIntoTarget stores the union of b and other into target. b and other will
 // be left unchanged, but target will be modified in place. Used to share
 // the union logic between the copy-on-write and in-place functions.
+//
+// This function performs an n-way union of n bitmaps. It performs this in an
+// optimized manner looping through all the bitmaps and performing unions one
+// container at a time. As a result, instead of generating many intermediary
+// containers for each union operation for a given container key, only one
+// new container needs to be allocated (or re-used) regardless of how many bitmaps
+// participate in the union. This significantly reduces allocations. In addition,
+// because we perform the unions one container at a time accross all the bitmaps, we
+// can calculate summary statistics that allow us to make more efficient decisions
+// up front. For example, imagine trying to combine a union accross the following three
+// bitsets:
+//
+//     1. Bitmap A: Single array container at key 0 with 400 values in it.
+//     2. Bitmap B: Single array container at key 0 with 500 values in it.
+//     3. Bitmap C: Single array container at key 0 with 3500 values in it.
+//
+// Naive approach:
+//
+//     1. Perform union of bitmap A and B, container by container
+//         a. 400 + 500 < ArrayMaxSize so likely we will choose to allocate a new array
+//            container and then perform a unionArrayArray operation to merge the two
+//            arrays into the new array container.
+//     2. Perform a union of the bitmap generated in the step above with bitmap C.
+//        900 + 3500 > ArrayMaxSize so we will need to upgrade to a bitset container which
+//        we will have to allocate, and then we will have to perform two unions into the
+//        new bitmap container: one for the array container generated in the previous step,
+//        and one for the bitset container in bitmap C.
+//
+// Approach taken by this function:
+//
+//     1. Detect that bitmaps A, B, and C all have containers for key 0.
+//     2. Estimate the resulting cardinality of the union of all their containers to be
+//        400 + 500 + 3500 > ArrayMaxSize and decide upfront to use a bitset for the target
+//        container.
+//     3. Union the containers from bitmaps A, B, and C into the new bitset container directly
+//        using fast bitwise operations.
+//
+// In the naive approach, we had to allocate two containers, whereas in the optimized approach
+// we only had to allocate one container, and we also had to perform less union operations. This
+// example is simplistic, but the impact in terms of CPU cycles and memory allocations achieved
+// by using the optimized alogorithm when working with a large number of large bitmaps is huge.
+//
+// An additional optimization that this function makes is that it recognizes that even when
+// CPU support is present, performing the popcount() operation isn't free. Imagine a scenario
+// where 10 bitset containers are being unioned together one after the next. If every
+// bitset<->bitset union operation needs to keep the containers cardinality up to date, then
+// the algorithm will waste a lot of time performing intermediary popcount() operations that
+// will immediately be invalidated by the next union operation. As a result, we allow the cardinality
+// of containers to degrade when we perform the in-place union operations, and then when the algorithm
+// completes we "repair" all the containers by performing the popcount() operation one time. This means
+// that we only ever have to do O(1) popcount operations per container instead of O(n) where n is the
+// number of containers with the same key that are being unioned together.
+//
+// The algorithm works by iterating through all of the containers in all of the bitmaps concurrently.
+// At every "tick" of the outermost loop, we increment our pointer into the bitmaps list of containers
+// by 1 (if we haven't reached the end of the containers for that bitmap.)
+//
+// We then loop through all of the "current" values of the current container for all of the bitmaps
+// and for each container with a specific key that we encounter, we scan forward to see if any of the
+// other bitmaps have a container for the same key. If so, we calculate some summary statistics and
+// then use that information to make a decision about how to union all of the containers with the same
+// key together, perform the union, and then move on to the next batch of containers that share the same
+// key.
+//
+// We repeat this process until every single bitmaps current container has been "handled". Then  we start the
+// outer loop over again and the process repeats until we've iterated through every container in every bitmap
+// and unioned everything into a single target bitmap.
+//
+// The diagram below shows the iteration state of four different maps as the algorithm progresses. The diagrams should be
+// interpreted from left -> right, top -> bottom. The ^ symbol represents the bitmaps current container iteration position,
+// and the - symbol represents a container that is at the current iteration position, but has been marked as "handled".
+//
+//          ----------------------------      |      ----------------------------      |     ----------------------------
+// Bitmap 1 |___X____________X__________|     |      |___X____________X__________|     |      |___X____________X__________|
+//              ^                             |          _                             |
+//          ----------------------------      |      ----------------------------      |      ----------------------------
+// Bitmap 2 |_______X________X______X___|     |      |_______X_______________X___|     |      |_______X_______________X___|
+//                  ^                         |              ^                         |
+//          ----------------------------      |      ----------------------------      |      ----------------------------
+// Bitmap 3 |_______X___________________|     |      |_______X___________________|     |      |_______X___________________|
+//                  ^                         |              ^                         |
+//          ----------------------------      |      ----------------------------      |      ----------------------------
+// Bitmap 4 |___X_______________________|     |      |___X_______________________|     |      |___X_______________________|
+//              ^                             |          _                             |
+// ------------------------------------------------------------------------------------------------------------------------
+//          ----------------------------      |      ----------------------------      |      ----------------------------
+// Bitmap 1 |___X____________X__________|     |      |___X____________X__________|     |      |___X____________X__________|
+//              _                             |                       ^                |                       _
+//          ----------------------------      |      ----------------------------      |      ----------------------------
+// Bitmap 2 |_______X_______________X___|     |      |_______X_______________X___|     |      |_______X_______________X___|
+//                  _                         |                              ^         |                              ^
+//          ----------------------------      |      ----------------------------      |      ----------------------------
+// Bitmap 3 |_______X___________________|     |      |_______X___________________|     |      |_______X___________________|
+//                  _                         |                                        |
+//          ----------------------------      |      ----------------------------      |      ----------------------------
+// Bitmap 4 |___X_______________________|     |      |___X_______________________|     |      |___X_______________________|
+//              _
 func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 	otherIters := make(wrappedIters, 0, len(others)+1)
 	bIter, _ := b.Containers.Iterator(0)
@@ -3884,3 +3949,35 @@ func readWithRuns(b *Bitmap, data []byte, pos int, keyN uint32) {
 		}
 	}
 }
+
+type wrapperIter struct {
+	iter    ContainerIterator
+	hasNext bool
+	handled bool
+}
+
+type wrappedIters []wrapperIter
+
+func (w wrappedIters) next() bool {
+	hasNext := false
+
+	for i, wrapped := range w {
+		next := wrapped.iter.Next()
+		w[i].hasNext = next
+		w[i].handled = false
+		if next {
+			hasNext = true
+		}
+	}
+
+	return hasNext
+}
+
+func (w wrappedIters) markItersWithCurrentKeyAsHandled(key uint64) {
+	for i, wrapped := range w {
+		currKey, _ := wrapped.iter.Value()
+		if currKey == key {
+			w[i].handled = true
+		}
+	}
+}

From 25eae0204d0728aa5d613db18f3bf007e7162ce6 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:26:44 -0500
Subject: [PATCH 26/61] Update benchmarks

---
 roaring/roaring_test.go | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go
index 8e389c586..72153538f 100644
--- a/roaring/roaring_test.go
+++ b/roaring/roaring_test.go
@@ -1413,7 +1413,6 @@ func BenchmarkSliceDescending(b *testing.B) {
 }
 
 func BenchmarkUnion(b *testing.B) {
-	// a1, a2, b, r1, r2 *roaring.Bitmap
 	data := getBenchData(b)
 	for n := 0; n < b.N; n++ {
 		data.a1.
@@ -1425,11 +1424,10 @@ func BenchmarkUnion(b *testing.B) {
 }
 
 func BenchmarkUnionBulk(b *testing.B) {
-	// a1, a2, b, r1, r2 *roaring.Bitmap
 	data := getBenchData(b)
-	yolo := roaring.NewBitmap()
+	bm := roaring.NewBitmap()
 	for n := 0; n < b.N; n++ {
-		yolo.
+		bm.
 			UnionInPlace(data.a1, data.a2, data.b, data.r1, data.r2)
 	}
 }

From feb19c62b99e0df19bb6411daaca8ba657e3fb0e Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:28:33 -0500
Subject: [PATCH 27/61] Change repair functions to specify they are bitmap only

---
 roaring/containers.go | 2 +-
 roaring/roaring.go    | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/roaring/containers.go b/roaring/containers.go
index 9862b894c..102ffd3bd 100644
--- a/roaring/containers.go
+++ b/roaring/containers.go
@@ -156,7 +156,7 @@ func (sc *sliceContainers) Iterator(key uint64) (citer ContainerIterator, found
 	return &sliceIterator{e: sc, i: i}, found
 }
 
-func (sc *sliceContainers) Repair() {
+func (sc *sliceContainers) RepairBitmaps() {
 	for _, c := range sc.containers {
 		if c.isBitmap() {
 			c.bitmapRepair()
diff --git a/roaring/roaring.go b/roaring/roaring.go
index 8efd6e0cf..c242bf94f 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -98,8 +98,9 @@ type Containers interface {
 
 	Count() uint64
 
-	// Repair will n values after in-place operations.
-	Repair()
+	// RepairBitmaps will repair cardinality(n) values on bitmap containers after
+	// in-place operations.
+	RepairBitmaps()
 
 	//Reset will clear the containers collection to allow for recycling during snapshot
 	Reset()
@@ -672,7 +673,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 	// n (container cardinality) to fall out of sync, and then at the very end we perform
 	// a "Repair" to recalculate all the container values. That way we never popcount()
 	// an entire bitmap container more than once per bulk union operation.
-	target.Containers.Repair()
+	target.Containers.RepairBitmaps()
 }
 
 // Difference returns the difference of b and other.

From 56b3d7d5db91d8a233823776ce503cd8db9a027a Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:31:23 -0500
Subject: [PATCH 28/61] fix comment

---
 roaring/roaring.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index c242bf94f..abd81c59f 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -413,9 +413,9 @@ func (b *Bitmap) UnionInPlace(others ...*Bitmap) {
 	b.unionIntoTarget(b, others...)
 }
 
-// unionIntoTarget stores the union of b and other into target. b and other will
-// be left unchanged, but target will be modified in place. Used to share
-// the union logic between the copy-on-write and in-place functions.
+// unionIntoTarget stores the union of b and others into target. b and others will
+// be left unchanged (unless one of them is also target), but target will be modified
+// in place.
 //
 // This function performs an n-way union of n bitmaps. It performs this in an
 // optimized manner looping through all the bitmaps and performing unions one

From f97ff4b5a0fe2a0a5d011b6f38cf8afd6dd34012 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:34:17 -0500
Subject: [PATCH 29/61] special case individual union

---
 roaring/roaring.go | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index abd81c59f..80d548456 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -399,9 +399,14 @@ func (b *Bitmap) Intersect(other *Bitmap) *Bitmap {
 	return output
 }
 
-// Union returns the bitwise union of b and other.
+// Union returns the bitwise union of b and other as a new bitmap.
 func (b *Bitmap) Union(others ...*Bitmap) *Bitmap {
 	output := NewBitmap()
+	if len(others) == 1 {
+		b.unionIntoTargetSingle(output, others[0])
+		return output
+	}
+
 	output.UnionInPlace(b)
 	output.UnionInPlace(others...)
 	return output
@@ -413,6 +418,30 @@ func (b *Bitmap) UnionInPlace(others ...*Bitmap) {
 	b.unionIntoTarget(b, others...)
 }
 
+func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
+	iiter, _ := b.Containers.Iterator(0)
+	jiter, _ := other.Containers.Iterator(0)
+	i, j := iiter.Next(), jiter.Next()
+	ki, ci := iiter.Value()
+	kj, cj := jiter.Value()
+	for i || j {
+		if i && (!j || ki < kj) {
+			target.Containers.Put(ki, ci.Clone())
+			i = iiter.Next()
+			ki, ci = iiter.Value()
+		} else if j && (!i || ki > kj) {
+			target.Containers.Put(kj, cj.Clone())
+			j = jiter.Next()
+			kj, cj = jiter.Value()
+		} else { // ki == kj
+			target.Containers.Put(ki, union(ci, cj))
+			i, j = iiter.Next(), jiter.Next()
+			ki, ci = iiter.Value()
+			kj, cj = jiter.Value()
+		}
+	}
+}
+
 // unionIntoTarget stores the union of b and others into target. b and others will
 // be left unchanged (unless one of them is also target), but target will be modified
 // in place.

From 038e3d4304f9cc21273c35138a594c735ce7fa21 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:34:43 -0500
Subject: [PATCH 30/61] fix diagram

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 80d548456..180768ea8 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -517,7 +517,7 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 // interpreted from left -> right, top -> bottom. The ^ symbol represents the bitmaps current container iteration position,
 // and the - symbol represents a container that is at the current iteration position, but has been marked as "handled".
 //
-//          ----------------------------      |      ----------------------------      |     ----------------------------
+//          ----------------------------      |      ----------------------------      |      ----------------------------
 // Bitmap 1 |___X____________X__________|     |      |___X____________X__________|     |      |___X____________X__________|
 //              ^                             |          _                             |
 //          ----------------------------      |      ----------------------------      |      ----------------------------

From efd6116d3f279626953ec675f0ba21a2941fc92e Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:35:35 -0500
Subject: [PATCH 31/61] replace word in comment for clarity

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 180768ea8..584a614df 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -454,7 +454,7 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 // participate in the union. This significantly reduces allocations. In addition,
 // because we perform the unions one container at a time accross all the bitmaps, we
 // can calculate summary statistics that allow us to make more efficient decisions
-// up front. For example, imagine trying to combine a union accross the following three
+// up front. For example, imagine trying to perform a union accross the following three
 // bitsets:
 //
 //     1. Bitmap A: Single array container at key 0 with 400 values in it.

From 3f3fec38249d95953f80be289f7d5f5e7900a5a2 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:36:38 -0500
Subject: [PATCH 32/61] refactor comment for clarity

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 584a614df..b295ebec0 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -485,7 +485,7 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 // In the naive approach, we had to allocate two containers, whereas in the optimized approach
 // we only had to allocate one container, and we also had to perform less union operations. This
 // example is simplistic, but the impact in terms of CPU cycles and memory allocations achieved
-// by using the optimized alogorithm when working with a large number of large bitmaps is huge.
+// by using the optimized alogorithm when unioning many large bitmaps can be huge.
 //
 // An additional optimization that this function makes is that it recognizes that even when
 // CPU support is present, performing the popcount() operation isn't free. Imagine a scenario

From 3193fc98ab94db68877718eb8486828bcf5c721d Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:38:13 -0500
Subject: [PATCH 33/61] refactor comment for clarity

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index b295ebec0..54af56b2c 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -502,7 +502,7 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 // At every "tick" of the outermost loop, we increment our pointer into the bitmaps list of containers
 // by 1 (if we haven't reached the end of the containers for that bitmap.)
 //
-// We then loop through all of the "current" values of the current container for all of the bitmaps
+// We then loop through all of the "current" values(containers) for all of the bitmaps
 // and for each container with a specific key that we encounter, we scan forward to see if any of the
 // other bitmaps have a container for the same key. If so, we calculate some summary statistics and
 // then use that information to make a decision about how to union all of the containers with the same

From d6e2d0768783930a140fee56d4113c611a80f495 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:39:07 -0500
Subject: [PATCH 34/61] refactor comment for clarity

---
 roaring/roaring.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 54af56b2c..dc80d0dcb 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -506,8 +506,8 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 // and for each container with a specific key that we encounter, we scan forward to see if any of the
 // other bitmaps have a container for the same key. If so, we calculate some summary statistics and
 // then use that information to make a decision about how to union all of the containers with the same
-// key together, perform the union, and then move on to the next batch of containers that share the same
-// key.
+// key together, perform the union, mark the unioned containers as "handled" and then move on to the next
+// batch of containers that share the same key.
 //
 // We repeat this process until every single bitmaps current container has been "handled". Then  we start the
 // outer loop over again and the process repeats until we've iterated through every container in every bitmap

From d35aabfa86701ea6696aaeea63cc68090dab8976 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:39:27 -0500
Subject: [PATCH 35/61] remove double space

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index dc80d0dcb..2bbe19feb 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -509,7 +509,7 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 // key together, perform the union, mark the unioned containers as "handled" and then move on to the next
 // batch of containers that share the same key.
 //
-// We repeat this process until every single bitmaps current container has been "handled". Then  we start the
+// We repeat this process until every single bitmaps current container has been "handled". Then we start the
 // outer loop over again and the process repeats until we've iterated through every container in every bitmap
 // and unioned everything into a single target bitmap.
 //

From 5ddeb0f6a07a92f009c8ef1d8bef1fd177d90744 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:40:21 -0500
Subject: [PATCH 36/61] refactor comment for clarity

---
 roaring/roaring.go | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 2bbe19feb..04f6bed69 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -513,8 +513,9 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 // outer loop over again and the process repeats until we've iterated through every container in every bitmap
 // and unioned everything into a single target bitmap.
 //
-// The diagram below shows the iteration state of four different maps as the algorithm progresses. The diagrams should be
-// interpreted from left -> right, top -> bottom. The ^ symbol represents the bitmaps current container iteration position,
+// The diagram below shows the iteration state of four different bitmaps as the algorithm progresses them.
+// The diagrams should BE interpreted from left -> right, top -> bottom. The X's represent a container in
+// the bitmap at a specific key,  ^ symbol represents the bitmaps current container iteration position,
 // and the - symbol represents a container that is at the current iteration position, but has been marked as "handled".
 //
 //          ----------------------------      |      ----------------------------      |      ----------------------------

From c1c1121e51179217bee47e34a361e88ec10812e0 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:41:30 -0500
Subject: [PATCH 37/61] more comment refactoring

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 04f6bed69..d790bb24f 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -699,7 +699,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 
 	// Performing the popcount() operation with every union is wasteful because
 	// its likely the value will be invalidated by the next union operation. As
-	// a result, when we're performing all our in-place unions we allow the value of
+	// a result, when we're performing all of our in-place unions we allow the value of
 	// n (container cardinality) to fall out of sync, and then at the very end we perform
 	// a "Repair" to recalculate all the container values. That way we never popcount()
 	// an entire bitmap container more than once per bulk union operation.

From 76aea6d9bc361f27d0c80192c106fe91bd521e3d Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 16:53:37 -0500
Subject: [PATCH 38/61] rename structs

---
 roaring/roaring.go | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index d790bb24f..51e26f037 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -544,11 +544,19 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 // Bitmap 4 |___X_______________________|     |      |___X_______________________|     |      |___X_______________________|
 //              _
 func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
-	otherIters := make(wrappedIters, 0, len(others)+1)
+	var (
+		wrappedArray = [20]handledIter{}
+		otherIters   handledIters
+	)
+	if len(others)+1 < 20 {
+		otherIters = wrappedArray[:0]
+	} else {
+		otherIters = make(handledIters, 0, len(others)+1)
+	}
 	bIter, _ := b.Containers.Iterator(0)
 	next := bIter.Next()
 	if next {
-		otherIters = append(otherIters, wrapperIter{
+		otherIters = append(otherIters, handledIter{
 			iter:    bIter,
 			hasNext: true,
 			handled: false,
@@ -559,7 +567,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 		otherIter, _ := other.Containers.Iterator(0)
 		next := otherIter.Next()
 		if next {
-			otherIters = append(otherIters, wrapperIter{
+			otherIters = append(otherIters, handledIter{
 				iter:    otherIter,
 				hasNext: true,
 				handled: false,
@@ -3981,15 +3989,15 @@ func readWithRuns(b *Bitmap, data []byte, pos int, keyN uint32) {
 	}
 }
 
-type wrapperIter struct {
+type handledIter struct {
 	iter    ContainerIterator
 	hasNext bool
 	handled bool
 }
 
-type wrappedIters []wrapperIter
+type handledIters []handledIter
 
-func (w wrappedIters) next() bool {
+func (w handledIters) next() bool {
 	hasNext := false
 
 	for i, wrapped := range w {
@@ -4004,7 +4012,7 @@ func (w wrappedIters) next() bool {
 	return hasNext
 }
 
-func (w wrappedIters) markItersWithCurrentKeyAsHandled(key uint64) {
+func (w handledIters) markItersWithCurrentKeyAsHandled(key uint64) {
 	for i, wrapped := range w {
 		currKey, _ := wrapped.iter.Value()
 		if currKey == key {

From 3d0d0db2e94b197968bdd781d869ae276ec6e077 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 17:01:52 -0500
Subject: [PATCH 39/61] more micro-optimizations

---
 roaring/roaring.go | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 51e26f037..ba5998b72 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -545,22 +545,36 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 //              _
 func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 	var (
-		wrappedArray = [20]handledIter{}
-		otherIters   handledIters
+		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{}
+		otherIters         handledIters
 	)
-	if len(others)+1 < 20 {
-		otherIters = wrappedArray[:0]
-	} else {
-		otherIters = make(handledIters, 0, len(others)+1)
+	if b != target {
+		// If b and target are not the same, we will need to union b into target which
+		// means we need room for one more iter.
+		requiredSliceSize++
 	}
-	bIter, _ := b.Containers.Iterator(0)
-	next := bIter.Next()
-	if next {
-		otherIters = append(otherIters, handledIter{
-			iter:    bIter,
-			hasNext: true,
-			handled: false,
-		})
+
+	if requiredSliceSize <= 20 {
+		otherIters = staticHandledIters[:0]
+	} else {
+		otherIters = make(handledIters, 0, requiredSliceSize)
+	}
+
+	// Only include b in the list of iters if its not the same as target to avoid
+	// a wasteful self union.
+	if b != target {
+		bIter, _ := b.Containers.Iterator(0)
+		next := bIter.Next()
+		if next {
+			otherIters = append(otherIters, handledIter{
+				iter:    bIter,
+				hasNext: true,
+				handled: false,
+			})
+		}
 	}
 
 	for _, other := range others {

From d96bde179d37c9261f3a96f6e3cbb1dd293bde34 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 17:19:48 -0500
Subject: [PATCH 40/61] factor out summary stats calculation into helper

---
 roaring/roaring.go | 89 ++++++++++++++++++++++++++--------------------
 1 file changed, 50 insertions(+), 39 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index ba5998b72..6cf4313f6 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -606,45 +606,9 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 			// Summary statistics about all the containers in the other bitmaps
 			// that share the same key so we can make smarter union strategy
 			// decisions later.
-			var (
-				// Estimated cardinality of the union of all containers with the same
-				// key as iKey across all bitmaps. This calculation is very rough as
-				// we just sum the cardinality of the container across the different
-				// bitmaps which could result in very inflated values, but it allows
-				// us to avoid allocating expensive bitmaps when unioning many low
-				// density containers.
-				n = iContainer.n
-				// Whether iContainer is the only container across all the bitmaps
-				// with the key iKey. If true, we can skip all the unioning logic
-				// and just clone the container into target.
-				isOnlyContainerWithKey = true
-				// Whether any of the containers are storing every possible value that
-				// they can. If so, we can short-circuit all the unioning logic and use
-				// a RLE container with a single value in it. This is an optimization to
-				// avoid using an expensive bitmap container for bitmaps that have some
-				// extremely dense containers.
-				hasMaxRange = iContainer.n == maxContainerVal+1
-			)
-			for _, jIter := range otherIters[i:] {
-				if hasMaxRange {
-					// If we already know that we're going to use a max range RLE container,
-					// then there is no reason to continue calculating statistics.
-					continue
-				}
+			summaryStats := otherIters[i:].calculateSummaryStats(iKey)
 
-				// Calculate key-level statistics here
-				jKey, jContainer := jIter.iter.Value()
-
-				if iKey == jKey {
-					isOnlyContainerWithKey = false
-					n += jContainer.n
-					if !hasMaxRange {
-						hasMaxRange = jContainer.n == maxContainerVal+1
-					}
-				}
-			}
-
-			if isOnlyContainerWithKey {
+			if summaryStats.isOnlyContainerWithKey {
 				// TODO(rartoul): We can avoid these clones if we can determine
 				// if the container is coming from an immutable bitmap and we
 				// know that we can mark the target bitmap as immutable as well.
@@ -655,7 +619,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 
 			// There was more than one container across the bitmaps with key iKey
 			// so we need to calculate a union.
-			if hasMaxRange {
+			if summaryStats.hasMaxRange {
 				// One (or more) of the containers represented the maximum possible
 				// range that a container can store, so instead of calculating a
 				// union we can generate an RLE container that represents the entire
@@ -4034,3 +3998,50 @@ func (w handledIters) markItersWithCurrentKeyAsHandled(key uint64) {
 		}
 	}
 }
+
+func (w handledIters) calculateSummaryStats(key uint64) containerUnionSummaryStats {
+	summary := containerUnionSummaryStats{}
+
+	for _, iter := range w {
+		if summary.hasMaxRange {
+			// If we already know that we're going to use a max range RLE container,
+			// then there is no reason to continue calculating statistics.
+			continue
+		}
+
+		// Calculate key-level statistics here
+		currKey, currContainer := iter.iter.Value()
+
+		if key == currKey {
+			summary.isOnlyContainerWithKey = false
+			summary.n += currContainer.n
+			if !summary.hasMaxRange {
+				summary.hasMaxRange = (currContainer.n == maxContainerVal+1)
+			}
+		}
+	}
+
+	return summary
+}
+
+// Summary statistics about all the containers in the other bitmaps
+// that share the same key so we can make smarter union strategy
+// decisions.
+type containerUnionSummaryStats struct {
+	// Estimated cardinality of the union of all containers with the same
+	// key across all bitmaps. This calculation is very rough as we just sum
+	// the cardinality of the container across the different bitmaps which could
+	// result in very inflated values, but it allows us to avoid allocating
+	// expensive bitmaps when unioning many low density containers.
+	n int32
+	// Whether any other is the only container across all the bitmaps
+	// with the specified key. If true, we can skip all the unioning logic
+	// and just clone the container into target.
+	isOnlyContainerWithKey bool
+	// Whether any of the containers with the specified keys are storing every possible
+	// value that they can. If so, we can short-circuit all the unioning logic and use
+	// a RLE container with a single value in it. This is an optimization to
+	// avoid using an expensive bitmap container for bitmaps that have some
+	// extremely dense containers.
+	hasMaxRange bool
+}

From a2bb87771dc93f59b42ad172b33273eec83c2d7c Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 17:21:38 -0500
Subject: [PATCH 41/61] Refactor

---
 roaring/roaring.go | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 6cf4313f6..776e2d614 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -601,12 +601,14 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				continue
 			}
 
-			iKey, iContainer := iIter.iter.Value()
-
-			// Summary statistics about all the containers in the other bitmaps
-			// that share the same key so we can make smarter union strategy
-			// decisions later.
-			summaryStats := otherIters[i:].calculateSummaryStats(iKey)
+			var (
+				iKey, iContainer = iIter.iter.Value()
+				// Summary statistics about all the containers in the other bitmaps
+				// that share the same key so we can make smarter union strategy
+				// decisions later. Note that we slice to [i:] not [i+1:] because we
+				// want to include the current containers information in the stats.
+				summaryStats = otherIters[i:].calculateSummaryStats(iKey)
+			)
 
 			if summaryStats.isOnlyContainerWithKey {
 				// TODO(rartoul): We can avoid these clones if we can determine

From 7a57b24b46a8dad349aa8b5fa4096df8bcb0f451 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 17:21:58 -0500
Subject: [PATCH 42/61] Fix comment

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 776e2d614..1e684ba6f 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -656,7 +656,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 					}
 				}
 
-				// Once we've acquire a bitmap container (either by reusing the existing one
+				// Once we've acquired a bitmap container (either by reusing the existing one
 				// or allocating a new one) then the last step is to iterate through all the
 				// other containers to see which ones have the same key, and union all of them
 				// into the target bitmap container.

From ddfc95070b5f8f92dd374d80aa5475b561d1934c Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 17:23:18 -0500
Subject: [PATCH 43/61] rename var

---
 roaring/roaring.go | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 1e684ba6f..7b382ec6f 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -549,7 +549,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 		// 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{}
-		otherIters         handledIters
+		bitmapIters        handledIters
 	)
 	if b != target {
 		// If b and target are not the same, we will need to union b into target which
@@ -558,9 +558,9 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 	}
 
 	if requiredSliceSize <= 20 {
-		otherIters = staticHandledIters[:0]
+		bitmapIters = staticHandledIters[:0]
 	} else {
-		otherIters = make(handledIters, 0, requiredSliceSize)
+		bitmapIters = make(handledIters, 0, requiredSliceSize)
 	}
 
 	// Only include b in the list of iters if its not the same as target to avoid
@@ -569,7 +569,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 		bIter, _ := b.Containers.Iterator(0)
 		next := bIter.Next()
 		if next {
-			otherIters = append(otherIters, handledIter{
+			bitmapIters = append(bitmapIters, handledIter{
 				iter:    bIter,
 				hasNext: true,
 				handled: false,
@@ -581,7 +581,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 		otherIter, _ := other.Containers.Iterator(0)
 		next := otherIter.Next()
 		if next {
-			otherIters = append(otherIters, handledIter{
+			bitmapIters = append(bitmapIters, handledIter{
 				iter:    otherIter,
 				hasNext: true,
 				handled: false,
@@ -593,7 +593,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 	hasNext := true
 	for hasNext {
 		// Loop until every iters current value has been handled.
-		for i, iIter := range otherIters {
+		for i, iIter := range bitmapIters {
 			if !iIter.hasNext || iIter.handled {
 				// Either we've exhausted this iter (it has no more containers), or
 				// we've already handled the current container by unioning it with
@@ -607,7 +607,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				// that share the same key so we can make smarter union strategy
 				// decisions later. Note that we slice to [i:] not [i+1:] because we
 				// want to include the current containers information in the stats.
-				summaryStats = otherIters[i:].calculateSummaryStats(iKey)
+				summaryStats = bitmapIters[i:].calculateSummaryStats(iKey)
 			)
 
 			if summaryStats.isOnlyContainerWithKey {
@@ -615,7 +615,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				// if the container is coming from an immutable bitmap and we
 				// know that we can mark the target bitmap as immutable as well.
 				target.Containers.Put(iKey, iContainer.Clone())
-				otherIters[i].handled = true
+				bitmapIters[i].handled = true
 				continue
 			}
 
@@ -632,7 +632,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 					n:             maxContainerVal + 1,
 				}
 				target.Containers.Put(iKey, container)
-				otherIters[i:].markItersWithCurrentKeyAsHandled(iKey)
+				bitmapIters[i:].markItersWithCurrentKeyAsHandled(iKey)
 			} else {
 				// Use a bitmap container for the target bitmap, and union everything
 				// into it.
@@ -660,7 +660,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				// or allocating a new one) then the last step is to iterate through all the
 				// other containers to see which ones have the same key, and union all of them
 				// into the target bitmap container.
-				for j, jIter := range otherIters {
+				for j, jIter := range bitmapIters {
 					jKey, jContainer := jIter.iter.Value()
 
 					if iKey == jKey {
@@ -671,7 +671,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 						} else {
 							unionBitmapBitmapInPlace(container, jContainer)
 						}
-						otherIters[j].handled = true
+						bitmapIters[j].handled = true
 					}
 				}
 
@@ -682,7 +682,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 			}
 		}
 
-		hasNext = otherIters.next()
+		hasNext = bitmapIters.next()
 	}
 
 	// Performing the popcount() operation with every union is wasteful because

From 12d45415bb6986c6fd3b689fb15d21d5e892238f Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 17:29:32 -0500
Subject: [PATCH 44/61] more refactoring and micro optimizations

---
 roaring/roaring.go | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 7b382ec6f..575515743 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -659,8 +659,9 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				// Once we've acquired a bitmap container (either by reusing the existing one
 				// or allocating a new one) then the last step is to iterate through all the
 				// other containers to see which ones have the same key, and union all of them
-				// into the target bitmap container.
-				for j, jIter := range bitmapIters {
+				// into the target bitmap container. Only need to loop starting from i because
+				// anything previous to that has already been handled.
+				for j, jIter := range bitmapIters[i:] {
 					jKey, jContainer := jIter.iter.Value()
 
 					if iKey == jKey {

From 741f8e8b84fc6f8b02e9b62b1c5cb7dbd5c90263 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 17:35:04 -0500
Subject: [PATCH 45/61] remove repairBitmaps from public iface

---
 roaring/containers.go | 2 +-
 roaring/roaring.go    | 6 +-----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/roaring/containers.go b/roaring/containers.go
index 102ffd3bd..0e8fdf988 100644
--- a/roaring/containers.go
+++ b/roaring/containers.go
@@ -156,7 +156,7 @@ func (sc *sliceContainers) Iterator(key uint64) (citer ContainerIterator, found
 	return &sliceIterator{e: sc, i: i}, found
 }
 
-func (sc *sliceContainers) RepairBitmaps() {
+func (sc *sliceContainers) repairBitmaps() {
 	for _, c := range sc.containers {
 		if c.isBitmap() {
 			c.bitmapRepair()
diff --git a/roaring/roaring.go b/roaring/roaring.go
index 575515743..3cec6cf85 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -98,10 +98,6 @@ type Containers interface {
 
 	Count() uint64
 
-	// RepairBitmaps will repair cardinality(n) values on bitmap containers after
-	// in-place operations.
-	RepairBitmaps()
-
 	//Reset will clear the containers collection to allow for recycling during snapshot
 	Reset()
 }
@@ -692,7 +688,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 	// n (container cardinality) to fall out of sync, and then at the very end we perform
 	// a "Repair" to recalculate all the container values. That way we never popcount()
 	// an entire bitmap container more than once per bulk union operation.
-	target.Containers.RepairBitmaps()
+	target.Containers.(*sliceContainers).repairBitmaps()
 }
 
 // Difference returns the difference of b and other.

From 549595cd2e8b2887cca998051124e73a3792ce4b Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 17:45:10 -0500
Subject: [PATCH 46/61] fix lint issues

---
 roaring/roaring.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 3cec6cf85..31dd18ddf 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -448,9 +448,9 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 // containers for each union operation for a given container key, only one
 // new container needs to be allocated (or re-used) regardless of how many bitmaps
 // participate in the union. This significantly reduces allocations. In addition,
-// because we perform the unions one container at a time accross all the bitmaps, we
+// because we perform the unions one container at a time across all the bitmaps, we
 // can calculate summary statistics that allow us to make more efficient decisions
-// up front. For example, imagine trying to perform a union accross the following three
+// up front. For example, imagine trying to perform a union across the following three
 // bitsets:
 //
 //     1. Bitmap A: Single array container at key 0 with 400 values in it.

From 6d021fe870d815349009c82153a535518de0ff59 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 17:45:49 -0500
Subject: [PATCH 47/61] add comment

---
 roaring/roaring.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 31dd18ddf..13f0efede 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -3966,6 +3966,9 @@ func readWithRuns(b *Bitmap, data []byte, pos int, keyN uint32) {
 	}
 }
 
+// handledIter and handledIters are wrappers around Bitmap Container iterators
+// and assist with the unionIntoTarget algorithm by abstracting away some tedious
+// operations.
 type handledIter struct {
 	iter    ContainerIterator
 	hasNext bool

From 42e756b316818b4987080cffbfb3d8c05f7d2d1e Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 20:45:26 -0500
Subject: [PATCH 48/61] Add benchmark

---
 roaring/roaring_internal_test.go | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go
index 78b686371..223953658 100644
--- a/roaring/roaring_internal_test.go
+++ b/roaring/roaring_internal_test.go
@@ -3267,6 +3267,27 @@ func TestUnmarshalOfficialRoaring(t *testing.T) {
 
 }
 
+func BenchmarkUnionBitmapBitmapInPlace(b *testing.B) {
+	b1 := newTestBitmapContainer()
+	b2 := newTestBitmapContainer()
+	for n := 0; n < b.N; n++ {
+		unionBitmapBitmapInPlace(b1, b2)
+	}
+}
+
+func newTestBitmapContainer() *Container {
+	var (
+		buf       = make([]uint64, bitmapN)
+		ob        = buf[:bitmapN]
+		container = &Container{
+			bitmap:        ob,
+			n:             0,
+			containerType: containerBitmap,
+		}
+	)
+	return container
+}
+
 /*
 // This function exercises an arcane edge case in dead code.
 // It doesn't need to be run right now.

From c412bc595d62cf17396727c30a6b4dab6e2faff9 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 21:00:35 -0500
Subject: [PATCH 49/61] add benchmark

---
 roaring/roaring_internal_test.go | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go
index 223953658..a5157a97d 100644
--- a/roaring/roaring_internal_test.go
+++ b/roaring/roaring_internal_test.go
@@ -3275,6 +3275,13 @@ func BenchmarkUnionBitmapBitmapInPlace(b *testing.B) {
 	}
 }
 
+func BenchmarkBitmapRepair(b *testing.B) {
+	b1 := newTestBitmapContainer()
+	for n := 0; n < b.N; n++ {
+		b1.bitmapRepair()
+	}
+}
+
 func newTestBitmapContainer() *Container {
 	var (
 		buf       = make([]uint64, bitmapN)

From 069c2a281daa8cd24ea9c2e3d31b3df79c2809c7 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Thu, 29 Nov 2018 21:04:04 -0500
Subject: [PATCH 50/61] unroll to make a little faster

---
 roaring/roaring.go | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 13f0efede..e6ed7d4f4 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -2094,8 +2094,14 @@ func (c *Container) check() error {
 
 func (c *Container) bitmapRepair() {
 	n := int32(0)
-	for i := 0; i < bitmapN; i++ {
+	// Manually unroll loop to make it a little faster.
+	// TODO(rartoul): Can probably make this a few x faster using
+	// SIMD instructions.
+	for i := 0; i < bitmapN; i += 4 {
 		n += int32(popcount(c.bitmap[i]))
+		n += int32(popcount(c.bitmap[i+1]))
+		n += int32(popcount(c.bitmap[i+2]))
+		n += int32(popcount(c.bitmap[i+3]))
 	}
 	c.n = n
 }
@@ -2852,8 +2858,14 @@ func unionBitmapBitmapInPlace(a, b *Container) {
 		bb = b.bitmap[:bitmapN]
 	)
 
-	for i := 0; i < bitmapN; i++ {
+	// Manually unroll loop to make it a little faster.
+	// TODO(rartoul): Can probably make this a few x faster using
+	// SIMD instructions.
+	for i := 0; i < bitmapN; i += 4 {
 		ab[i] = ab[i] | bb[i]
+		ab[i+1] = ab[i+1] | bb[i+1]
+		ab[i+2] = ab[i+2] | bb[i+2]
+		ab[i+3] = ab[i+3] | bb[i+3]
 	}
 }
 

From 531b9d616dbfa3b8f21e7495f7cb7bd6f524c918 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Fri, 30 Nov 2018 14:40:06 -0500
Subject: [PATCH 51/61] Refactor code and comment for clarity

---
 roaring/roaring.go | 95 +++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 47 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index e6ed7d4f4..4aed09141 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -629,54 +629,55 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 				}
 				target.Containers.Put(iKey, container)
 				bitmapIters[i:].markItersWithCurrentKeyAsHandled(iKey)
-			} else {
-				// Use a bitmap container for the target bitmap, and union everything
-				// into it.
-				//
-				// TODO(rartoul): Add another conditional case for n < ArrayMaxSize
-				// (or some fraction of that value) to avoid allocating expensive
-				// bitmaps when unioning many low-density array containers, but this
-				// will require writing a union in place algorithm for an array container
-				// that accepts multiple different containers to union into it for
-				// efficiency.
-				container := target.Containers.Get(iKey)
-				// If target already has a bitmap container for iKey then we can reuse that,
-				// otherwise we have to allocate a new one.
-				if container == nil || container.containerType != containerBitmap {
-					buf := make([]uint64, bitmapN)
-					ob := buf[:bitmapN]
-					container = &Container{
-						bitmap:        ob,
-						n:             0,
-						containerType: containerBitmap,
-					}
-				}
-
-				// Once we've acquired a bitmap container (either by reusing the existing one
-				// or allocating a new one) then the last step is to iterate through all the
-				// other containers to see which ones have the same key, and union all of them
-				// into the target bitmap container. Only need to loop starting from i because
-				// anything previous to that has already been handled.
-				for j, jIter := range bitmapIters[i:] {
-					jKey, jContainer := jIter.iter.Value()
-
-					if iKey == jKey {
-						if jContainer.isArray() {
-							unionBitmapArrayInPlace(container, jContainer)
-						} else if jContainer.isRun() {
-							unionBitmapRunInPlace(container, jContainer)
-						} else {
-							unionBitmapBitmapInPlace(container, jContainer)
-						}
-						bitmapIters[j].handled = true
-					}
-				}
-
-				// Now that we've calculated a container is that a union of all the containers
-				// with the same key across all the bitmaps, we store it in the list of containers
-				// for the target.
-				target.Containers.Put(iKey, container)
+				continue
 			}
+
+			// Use a bitmap container for the target bitmap, and union everything
+			// into it.
+			//
+			// TODO(rartoul): Add another conditional case for n < ArrayMaxSize
+			// (or some fraction of that value) to avoid allocating expensive
+			// bitmaps when unioning many low-density array containers, but this
+			// will require writing a union in place algorithm for an array container
+			// that accepts multiple different containers to union into it for
+			// efficiency.
+			container := target.Containers.Get(iKey)
+			// If target already has a bitmap container for iKey then we can reuse that,
+			// otherwise we have to allocate a new one.
+			if container == nil || container.containerType != containerBitmap {
+				buf := make([]uint64, bitmapN)
+				ob := buf[:bitmapN]
+				container = &Container{
+					bitmap:        ob,
+					n:             0,
+					containerType: containerBitmap,
+				}
+			}
+
+			// Once we've acquired a bitmap container (either by reusing the existing one
+			// or allocating a new one) then the last step is to iterate through all the
+			// other containers to see which ones have the same key, and union all of them
+			// into the target bitmap container. Only need to loop starting from i because
+			// anything previous to that has already been handled.
+			for j, jIter := range bitmapIters[i:] {
+				jKey, jContainer := jIter.iter.Value()
+
+				if iKey == jKey {
+					if jContainer.isArray() {
+						unionBitmapArrayInPlace(container, jContainer)
+					} else if jContainer.isRun() {
+						unionBitmapRunInPlace(container, jContainer)
+					} else {
+						unionBitmapBitmapInPlace(container, jContainer)
+					}
+					bitmapIters[j].handled = true
+				}
+			}
+
+			// Now that we've calculated a container that is a union of all the containers
+			// with the same key across all the bitmaps, we store it in the list of containers
+			// for the target.
+			target.Containers.Put(iKey, container)
 		}
 
 		hasNext = bitmapIters.next()

From 3570ec7ab633e3ae6d29fd944b8d2d81959e533f Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Fri, 30 Nov 2018 14:41:30 -0500
Subject: [PATCH 52/61] collapse next calls into conditonals

---
 roaring/roaring.go | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 4aed09141..be6010b52 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -563,8 +563,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 	// a wasteful self union.
 	if b != target {
 		bIter, _ := b.Containers.Iterator(0)
-		next := bIter.Next()
-		if next {
+		if bIter.Next() {
 			bitmapIters = append(bitmapIters, handledIter{
 				iter:    bIter,
 				hasNext: true,
@@ -575,8 +574,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 
 	for _, other := range others {
 		otherIter, _ := other.Containers.Iterator(0)
-		next := otherIter.Next()
-		if next {
+		if otherIter.Next() {
 			bitmapIters = append(bitmapIters, handledIter{
 				iter:    otherIter,
 				hasNext: true,

From 34b1f2199f16eadbef0e0cd2c1137d03b08c164b Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Fri, 30 Nov 2018 14:41:59 -0500
Subject: [PATCH 53/61] Fix comment

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index be6010b52..2d88f2868 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -395,7 +395,7 @@ func (b *Bitmap) Intersect(other *Bitmap) *Bitmap {
 	return output
 }
 
-// Union returns the bitwise union of b and other as a new bitmap.
+// Union returns the bitwise union of b and others as a new bitmap.
 func (b *Bitmap) Union(others ...*Bitmap) *Bitmap {
 	output := NewBitmap()
 	if len(others) == 1 {

From 71621e60baab8ab5fe4033535d33df76c955b65e Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Fri, 30 Nov 2018 19:23:59 -0500
Subject: [PATCH 54/61] Refactor roaring repair operations

---
 roaring/containers.go |  6 ++----
 roaring/roaring.go    | 16 ++++++++++++++--
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/roaring/containers.go b/roaring/containers.go
index 0e8fdf988..b9928823a 100644
--- a/roaring/containers.go
+++ b/roaring/containers.go
@@ -156,11 +156,9 @@ func (sc *sliceContainers) Iterator(key uint64) (citer ContainerIterator, found
 	return &sliceIterator{e: sc, i: i}, found
 }
 
-func (sc *sliceContainers) repairBitmaps() {
+func (sc *sliceContainers) Repair() {
 	for _, c := range sc.containers {
-		if c.isBitmap() {
-			c.bitmapRepair()
-		}
+		c.Repair()
 	}
 }
 
diff --git a/roaring/roaring.go b/roaring/roaring.go
index 2d88f2868..a1a535ed3 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -98,8 +98,12 @@ type Containers interface {
 
 	Count() uint64
 
-	//Reset will clear the containers collection to allow for recycling during snapshot
+	// Reset will clear the containers collection to allow for recycling during snapshot
 	Reset()
+
+	// Repair will repair the cardinality of any containers whose cardinality were corrupted
+	// due to optimized operations.
+	Repair()
 }
 
 type ContainerIterator interface {
@@ -687,7 +691,7 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 	// n (container cardinality) to fall out of sync, and then at the very end we perform
 	// a "Repair" to recalculate all the container values. That way we never popcount()
 	// an entire bitmap container more than once per bulk union operation.
-	target.Containers.(*sliceContainers).repairBitmaps()
+	target.Containers.Repair()
 }
 
 // Difference returns the difference of b and other.
@@ -2091,6 +2095,14 @@ func (c *Container) check() error {
 	return a
 }
 
+// Repair repairs the cardinality of c if it has been corrupted by
+// optimized operations.
+func (c *Container) Repair() {
+	if c.isBitmap() {
+		c.bitmapRepair()
+	}
+}
+
 func (c *Container) bitmapRepair() {
 	n := int32(0)
 	// Manually unroll loop to make it a little faster.

From bb20f058ba6c77e26029d963e974487273208529 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Fri, 30 Nov 2018 19:25:54 -0500
Subject: [PATCH 55/61] Add Repair operation to btree containers

---
 enterprise/b/containers_btree.go | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/enterprise/b/containers_btree.go b/enterprise/b/containers_btree.go
index c2f95c0c7..fe357d933 100644
--- a/enterprise/b/containers_btree.go
+++ b/enterprise/b/containers_btree.go
@@ -177,6 +177,15 @@ func (btc *bTreeContainers) Iterator(key uint64) (citer roaring.ContainerIterato
 	}, found
 }
 
+func (btc *bTreeContainers) Repair() {
+	e, _ := btc.tree.Seek(0)
+	_, c, err := e.Next()
+	for err != io.EOF {
+		c.Repair()
+		_, c, err = e.Next()
+	}
+}
+
 type btcIterator struct {
 	e   *enumerator
 	key uint64

From 5b72544d73ec316d7ef9cd7ca2be1a6d8907ff8b Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Mon, 3 Dec 2018 15:25:54 -0500
Subject: [PATCH 56/61] simplify helper with early return

---
 roaring/roaring.go | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index a1a535ed3..f78f98fba 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -4028,20 +4028,17 @@ func (w handledIters) calculateSummaryStats(key uint64) containerUnionSummarySta
 	summary := containerUnionSummaryStats{}
 
 	for _, iter := range w {
-		if summary.hasMaxRange {
-			// If we already know that we're going to use a max range RLE container,
-			// then there is no reason to continue calculating statistics.
-			continue
-		}
-
 		// Calculate key-level statistics here
 		currKey, currContainer := iter.iter.Value()
 
 		if key == currKey {
 			summary.isOnlyContainerWithKey = false
 			summary.n += currContainer.n
-			if !summary.hasMaxRange {
-				summary.hasMaxRange = (currContainer.n == maxContainerVal+1)
+
+			if currContainer.n == maxContainerVal+1 {
+				summary.hasMaxRange = true
+				summary.n = maxContainerVal + 1
+				return summary
 			}
 		}
 	}

From c495d08d1b9a42a46811c6dd907cac241e04a441 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Mon, 3 Dec 2018 15:30:54 -0500
Subject: [PATCH 57/61] simplify logic by removing concept or target

---
 roaring/roaring.go | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index f78f98fba..2c38cfea2 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -415,7 +415,7 @@ func (b *Bitmap) Union(others ...*Bitmap) *Bitmap {
 // UnionInPlace returns the bitwise union of b and others, modifying
 // b in place.
 func (b *Bitmap) UnionInPlace(others ...*Bitmap) {
-	b.unionIntoTarget(b, others...)
+	b.unionInPlace(others...)
 }
 
 func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
@@ -543,19 +543,15 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 //          ----------------------------      |      ----------------------------      |      ----------------------------
 // Bitmap 4 |___X_______________________|     |      |___X_______________________|     |      |___X_______________________|
 //              _
-func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
+func (b *Bitmap) unionInPlace(others ...*Bitmap) {
 	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
 	)
-	if b != target {
-		// If b and target are not the same, we will need to union b into target which
-		// means we need room for one more iter.
-		requiredSliceSize++
-	}
 
 	if requiredSliceSize <= 20 {
 		bitmapIters = staticHandledIters[:0]
@@ -563,19 +559,6 @@ func (b *Bitmap) unionIntoTarget(target *Bitmap, others ...*Bitmap) {
 		bitmapIters = make(handledIters, 0, requiredSliceSize)
 	}
 
-	// Only include b in the list of iters if its not the same as target to avoid
-	// a wasteful self union.
-	if b != target {
-		bIter, _ := b.Containers.Iterator(0)
-		if bIter.Next() {
-			bitmapIters = append(bitmapIters, handledIter{
-				iter:    bIter,
-				hasNext: true,
-				handled: false,
-			})
-		}
-	}
-
 	for _, other := range others {
 		otherIter, _ := other.Containers.Iterator(0)
 		if otherIter.Next() {

From 9da6d43b760293111dc95c8880af9e5a830f5489 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Mon, 3 Dec 2018 15:31:29 -0500
Subject: [PATCH 58/61] fix docstring nit

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 2c38cfea2..e2fd3f893 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -98,7 +98,7 @@ type Containers interface {
 
 	Count() uint64
 
-	// Reset will clear the containers collection to allow for recycling during snapshot
+	// Reset clears the containers collection to allow for recycling during snapshot
 	Reset()
 
 	// Repair will repair the cardinality of any containers whose cardinality were corrupted

From a11d04c061e258b045716ba793bc0afd41676fa8 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Fri, 7 Dec 2018 15:59:57 -0500
Subject: [PATCH 59/61] clarify comment

---
 roaring/roaring.go | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index e2fd3f893..93aa65add 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -478,7 +478,11 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 //     1. Detect that bitmaps A, B, and C all have containers for key 0.
 //     2. Estimate the resulting cardinality of the union of all their containers to be
 //        400 + 500 + 3500 > ArrayMaxSize and decide upfront to use a bitset for the target
-//        container.
+//        container. Note that this is just an approximation of the final cardinality and can
+//        be off by a wide margin if there is a lot of overlap between containers, but that is
+//        fine, we'll still get the same result at the end, we'll just be more biased towards
+//        using bitmap containers will still being able to use array containers when all the
+//        cardinalities are small.
 //     3. Union the containers from bitmaps A, B, and C into the new bitset container directly
 //        using fast bitwise operations.
 //

From dcfed6ebb63182c89fc72c3441306f2822e4868e Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Fri, 7 Dec 2018 16:00:49 -0500
Subject: [PATCH 60/61] fix grammar

---
 roaring/roaring.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 93aa65add..61827fc77 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -494,7 +494,7 @@ func (b *Bitmap) unionIntoTargetSingle(target *Bitmap, other *Bitmap) {
 // An additional optimization that this function makes is that it recognizes that even when
 // CPU support is present, performing the popcount() operation isn't free. Imagine a scenario
 // where 10 bitset containers are being unioned together one after the next. If every
-// bitset<->bitset union operation needs to keep the containers cardinality up to date, then
+// bitset<->bitset union operation needs to keep the containers' cardinality up to date, then
 // the algorithm will waste a lot of time performing intermediary popcount() operations that
 // will immediately be invalidated by the next union operation. As a result, we allow the cardinality
 // of containers to degrade when we perform the in-place union operations, and then when the algorithm

From 082c8aba56108df22f4ade87e35b2000f4d17e92 Mon Sep 17 00:00:00 2001
From: Richard Artoul <rartoul@uber.com>
Date: Fri, 7 Dec 2018 16:13:42 -0500
Subject: [PATCH 61/61] switch to |=

---
 roaring/roaring.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/roaring/roaring.go b/roaring/roaring.go
index 61827fc77..4df42afdc 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -2860,10 +2860,10 @@ func unionBitmapBitmapInPlace(a, b *Container) {
 	// TODO(rartoul): Can probably make this a few x faster using
 	// SIMD instructions.
 	for i := 0; i < bitmapN; i += 4 {
-		ab[i] = ab[i] | bb[i]
-		ab[i+1] = ab[i+1] | bb[i+1]
-		ab[i+2] = ab[i+2] | bb[i+2]
-		ab[i+3] = ab[i+3] | bb[i+3]
+		ab[i] |= bb[i]
+		ab[i+1] |= bb[i+1]
+		ab[i+2] |= bb[i+2]
+		ab[i+3] |= bb[i+3]
 	}
 }