diff --git a/badger.go b/badger.go index 3af0ce477..39500f262 100644 --- a/badger.go +++ b/badger.go @@ -1628,7 +1628,7 @@ func (tx *BadgerTx) ImportRoaringBits(index, field, view string, shard uint64, i return } -func (tx *BadgerTx) toContainer(typ byte, v []byte) (r *roaring.Container) { +func (tx *BadgerTx) toContainer(typ byte, v []byte) (c *roaring.Container) { if len(v) == 0 { return nil @@ -1669,29 +1669,28 @@ func (tx *BadgerTx) toContainer(typ byte, v []byte) (r *roaring.Container) { switch typ { case roaring.ContainerArray: - c := roaring.NewContainerArray(toArray16(w)) + c = roaring.NewContainerArray(toArray16(w)) if tx.doAllocZero { // tx.acMu was acquired above, and Unlock deferred. tx.ourContainers = append(tx.ourContainers, c) } - return c case roaring.ContainerBitmap: - c := roaring.NewContainerBitmap(-1, toArray64(w)) + c = roaring.NewContainerBitmap(-1, toArray64(w)) if tx.doAllocZero { // tx.acMu was acquired above, and Unlock deferred. tx.ourContainers = append(tx.ourContainers, c) } - return c case roaring.ContainerRun: - c := roaring.NewContainerRun(toInterval16(w)) + c = roaring.NewContainerRun(toInterval16(w)) if tx.doAllocZero { // tx.acMu was acquired above, and Unlock deferred. tx.ourContainers = append(tx.ourContainers, c) } - return c default: panic(fmt.Sprintf("unknown container: %v", typ)) } + c.SetMapped(true) + return c } // StringifiedBadgerKeys returns a string with all the container diff --git a/lmdb.go b/lmdb.go index cf1db9f43..e18907d55 100644 --- a/lmdb.go +++ b/lmdb.go @@ -1520,20 +1520,19 @@ func (tx *LMDBTx) toContainer(typ byte, v []byte) (r *roaring.Container) { return ToContainer(typ, w) } -func ToContainer(typ byte, w []byte) (r *roaring.Container) { +func ToContainer(typ byte, w []byte) (c *roaring.Container) { switch typ { case roaring.ContainerArray: - c := roaring.NewContainerArray(toArray16(w)) - return c + c = roaring.NewContainerArray(toArray16(w)) case roaring.ContainerBitmap: - c := roaring.NewContainerBitmap(-1, toArray64(w)) - return c + c = roaring.NewContainerBitmap(-1, toArray64(w)) case roaring.ContainerRun: - c := roaring.NewContainerRun(toInterval16(w)) - return c + c = roaring.NewContainerRun(toInterval16(w)) default: panic(fmt.Sprintf("unknown container: %v", typ)) } + c.SetMapped(true) + return c } // StringifiedLMDBKeys returns a string with all the container diff --git a/rbf/cursorx.go b/rbf/cursorx.go index 7d2d9332c..8f3c6ea96 100644 --- a/rbf/cursorx.go +++ b/rbf/cursorx.go @@ -150,22 +150,25 @@ func (c *Cursor) CurrentPageType() int { return cell.Type } -func toContainer(l leafCell, tx *Tx) *roaring.Container { +func toContainer(l leafCell, tx *Tx) (c *roaring.Container) { orig := l.Data var cpMaybe []byte + var mapped bool if EnableRowCache || DoAllocZero { // make a copy, otherwise the rowCache will see corrupted data // or mmapped data that may disappear. cpMaybe = make([]byte, len(orig)) copy(cpMaybe, orig) + mapped = false } else { // not a copy cpMaybe = orig + mapped = true } switch l.Type { case ContainerTypeArray: - return roaring.NewContainerArray(toArray16(cpMaybe)) + c = roaring.NewContainerArray(toArray16(cpMaybe)) case ContainerTypeBitmapPtr: _, bm, _ := tx.leafCellBitmap(toPgno(cpMaybe)) cloneMaybe := bm @@ -173,13 +176,14 @@ func toContainer(l leafCell, tx *Tx) *roaring.Container { cloneMaybe = make([]uint64, len(bm)) copy(cloneMaybe, bm) } - return roaring.NewContainerBitmap(l.N, cloneMaybe) + c = roaring.NewContainerBitmap(l.N, cloneMaybe) case ContainerTypeBitmap: - return roaring.NewContainerBitmap(l.N, toArray64(cpMaybe)) + c = roaring.NewContainerBitmap(l.N, toArray64(cpMaybe)) case ContainerTypeRLE: - return roaring.NewContainerRun(toInterval16(cpMaybe)) + c = roaring.NewContainerRun(toInterval16(cpMaybe)) } - return nil + c.SetMapped(mapped) + return c } type Nodetype int diff --git a/roaring/container_stash.go b/roaring/container_stash.go index e08f823a6..e87a65fb8 100644 --- a/roaring/container_stash.go +++ b/roaring/container_stash.go @@ -309,6 +309,13 @@ func (c *Container) setMapped(mapped bool) { } } +// SetMapped marks a container as "mapped"; do this if you're setting a +// container's storage to something that it shouldn't write to, like mmapped +// memory. +func (c *Container) SetMapped(mapped bool) { + c.setMapped(mapped) +} + // setDirty marks a container as "dirty" -- we don't trust container's n. // this should never happen except for bitmaps. func (c *Container) setDirty(dirty bool) { diff --git a/roaring/roaring.go b/roaring/roaring.go index 52e806f52..384149ea6 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3300,33 +3300,8 @@ func (c *Container) arrayRemove(v uint16) (*Container, bool) { } c = c.Thaw() array = c.array() - - const needCopyOnWriteDueToReadOnlyMmap = true - - // TODO(jea) quick benchmarks don't show less performance if needCopyOnWriteDueToReadOnlyMmap - // is true, but we may need more rigorous measurement. - // - // Don't have a COW: - // all benchmarks in roaring/ - // ok github.com/pilosa/pilosa/v2/roaring 217.138s - // - // ok, have a COW: - // all benchmarks in roaring/ - // ok github.com/pilosa/pilosa/v2/roaring 214.295s - - if needCopyOnWriteDueToReadOnlyMmap { - n := len(array) - array2 := make([]uint16, n-1) - copy(array2, array[:i]) - copy(array2[i:], array[i+1:]) - c.setArray(array2) - } else { - // seg fault here with read-only mmap; go 1.14.7 linux. - // example: cap = 7 i = 0 len = 7 - // the append tries to write read-only memory? - array = append(array[:i], array[i+1:]...) - c.setArray(array) - } + array = append(array[:i], array[i+1:]...) + c.setArray(array) return c, true }