mirror of
https://github.com/featurebasedb/featurebase.git
synced 2026-10-08 11:57:51 +00:00
unbreak a subtle breakage that only test cases could hit
It turns out the logic for "don't update everything if the incoming slice pointer is the stash" is wrong; it should really be "don't update everything if the incoming slice pointer is the one we already have". The reason this breaks is that one of the tests directly sets the mapped bit. This breaks my assumption that we'd never be using the stash and have the mapped bit set, and that in turn breaks my assumption that the pointer of an incoming array can't be the stash address unless we were previously using the stash. If unmap moved us to non-stashed memory, then a future write could try to write, notice that it would fit in the stash, copy the data ... and not update the pointer because the stash pointer was handled separately. This way, if you do that, you can end up not using the stash when you probably could, but you get the expected behavior. But also, don't set the mapped bit directly. (I guess there's a good reason to for the test case, which is using it to verify that unmaps happen when modifications happen.) Also the unmap functions should indicate that they have successfully unmapped, which may help performance in some test cases.
This commit is contained in:
parent
5c8106bead
commit
2af5d64e2c
2 changed files with 9 additions and 5 deletions
|
|
@ -111,9 +111,9 @@ func (c *Container) setArray(array []uint16) {
|
|||
return
|
||||
}
|
||||
h := (*reflect.SliceHeader)(unsafe.Pointer(&array))
|
||||
if h.Data == uintptr(unsafe.Pointer(&c.data[0])) {
|
||||
if h.Data == uintptr(unsafe.Pointer(c.pointer)) {
|
||||
// nothing to do but update length
|
||||
c.len, c.cap = int32(h.Len), stashedArraySize
|
||||
c.len = int32(h.Len)
|
||||
return
|
||||
}
|
||||
// array we can fit in data store:
|
||||
|
|
@ -172,9 +172,9 @@ func (c *Container) setRuns(runs []interval16) {
|
|||
return
|
||||
}
|
||||
h := (*reflect.SliceHeader)(unsafe.Pointer(&runs))
|
||||
if h.Data == uintptr(unsafe.Pointer(&c.data[0])) {
|
||||
// nothing to do but update cap and length
|
||||
c.len, c.cap = int32(h.Len), stashedRunSize
|
||||
if h.Data == uintptr(unsafe.Pointer(c.pointer)) {
|
||||
// nothing to do but update length
|
||||
c.len = int32(h.Len)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -232,6 +232,7 @@ func (c *Container) unmapArray() {
|
|||
h := (*reflect.SliceHeader)(unsafe.Pointer(&tmp))
|
||||
c.pointer, c.cap = (*uint16)(unsafe.Pointer(h.Data)), int32(h.Cap)
|
||||
runtime.KeepAlive(&tmp)
|
||||
c.mapped = false
|
||||
}
|
||||
|
||||
// unmapBitmap ensures that the container is not using mmapped storage.
|
||||
|
|
@ -245,6 +246,7 @@ func (c *Container) unmapBitmap() {
|
|||
h := (*reflect.SliceHeader)(unsafe.Pointer(&tmp))
|
||||
c.pointer, c.cap = (*uint16)(unsafe.Pointer(h.Data)), int32(h.Cap)
|
||||
runtime.KeepAlive(&tmp)
|
||||
c.mapped = false
|
||||
}
|
||||
|
||||
// unmapRun ensures that the container is not using mmapped storage.
|
||||
|
|
@ -257,4 +259,5 @@ func (c *Container) unmapRun() {
|
|||
copy(tmp, runs)
|
||||
h := (*reflect.SliceHeader)(unsafe.Pointer(&tmp))
|
||||
c.pointer, c.cap = (*uint16)(unsafe.Pointer(h.Data)), int32(h.Cap)
|
||||
c.mapped = false
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1831,6 +1831,7 @@ func (c *Container) runRemove(v uint16) bool {
|
|||
return false
|
||||
}
|
||||
c.unmapRun()
|
||||
runs = c.runs()
|
||||
if v == runs[i].last && v == runs[i].start {
|
||||
runs = append(runs[:i], runs[i+1:]...)
|
||||
} else if v == runs[i].last {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue