diff --git a/fragment_internal_test.go b/fragment_internal_test.go index 19dc3f587..e45d3cce8 100644 --- a/fragment_internal_test.go +++ b/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, "") diff --git a/roaring/container_stash.go b/roaring/container_stash.go index 46725238f..fe03f9d2e 100644 --- a/roaring/container_stash.go +++ b/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 } diff --git a/roaring/roaring.go b/roaring/roaring.go index 8930981e0..f09667104 100644 --- a/roaring/roaring.go +++ b/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 }