Merge pull request #1989 from seebs/rowcacheFix

Rowcache fixes

fragment_internal_test.go

@@ -26,6 +26,7 @@ import (
 	"os"
 	"reflect"
 	"sort"
+	"sync/atomic"
 	"testing"
 	"testing/quick"
 
@@ -104,6 +105,44 @@ func TestFragment_ClearBit(t *testing.T) {
 	}
 }
 
+// What about rowcache timing.
+func TestFragment_RowcacheMap(t *testing.T) {
+	var done int64
+	f := mustOpenFragment("i", "f", viewStandard, 0, "")
+	defer f.Clean(t)
+
+	ch := make(chan struct{})
+
+	for i := 0; i < f.MaxOpN; i++ {
+		_, _ = f.setBit(0, uint64(i*32))
+	}
+	// force snapshot so we get a mmapped row...
+	_ = f.snapshot()
+	row := f.row(0)
+	segment := row.Segments()[0]
+	bitmap := segment.data
+
+	// request information from the frozen bitmap we got back
+	go func() {
+		for atomic.LoadInt64(&done) == 0 {
+			for i := 0; i < f.MaxOpN; i++ {
+				_ = bitmap.Contains(uint64(i * 32))
+			}
+		}
+		close(ch)
+	}()
+
+	// modify the original bitmap, until it causes a snapshot, which
+	// then invalidates the other map...
+	for j := 0; j < 5; j++ {
+		for i := 0; i < f.MaxOpN; i++ {
+			_, _ = f.setBit(0, uint64(i*32+j+1))
+		}
+	}
+	atomic.StoreInt64(&done, 1)
+	<-ch
+}
+
 // Ensure a fragment can clear a row.
 func TestFragment_ClearRow(t *testing.T) {
 	f := mustOpenFragment("i", "f", viewStandard, 0, "")

roaring/container_stash.go

@@ -256,11 +256,20 @@ func (c *Container) setMapped(mapped bool) {
 }
 
 // Freeze returns an unmodifiable container identical to c. This might
-// be c, now marked unmodifiable, or might be a new container.
+// be c, now marked unmodifiable, or might be a new container. If c
+// is currently marked as "mapped", referring to a backing store that's
+// not a conventional Go pointer, the storage may be copied.
 func (c *Container) Freeze() *Container {
 	if c == nil {
 		return nil
 	}
+	// don't need to freeze
+	if c.flags&flagFrozen != 0 {
+		return c
+	}
+	// unmapOrClone should unmap-in-place because the existing
+	// container isn't frozen (or we'd already have returned it).
+	c = c.unmapOrClone()
 	c.flags |= flagFrozen
 	return c
 }

roaring/roaring.go

@@ -971,11 +971,9 @@ func (b *Bitmap) countEmptyContainers() int {
 
 // Optimize converts array and bitmap containers to run containers as necessary.
 func (b *Bitmap) Optimize() {
-	citer, _ := b.Containers.Iterator(0)
-	for citer.Next() {
-		_, c := citer.Value()
-		c.optimize()
-	}
+	b.Containers.UpdateEvery(func(c *Container, existed bool) (*Container, bool) {
+		return c.optimize(), true
+	})
 }
 
 type errWriter struct {
@@ -3519,7 +3517,7 @@ RUNLOOP:
 		}
 	}
 	output := NewContainerRun(runs)
-	output.optimize()
+	output = output.optimize()
 	return output
 }