diff --git a/rbf/db.go b/rbf/db.go index 1d693fac6..f2eeceea3 100644 --- a/rbf/db.go +++ b/rbf/db.go @@ -69,6 +69,8 @@ type DB struct { // Path represents the path to the database file. Path string + + freelistCursor Cursor // cursor to reuse for freelist operations } // NewDB returns a new instance of DB. @@ -808,6 +810,11 @@ func (db *DB) readMetaPage() ([]byte, error) { return db.readDBPage(0) } +// getCursor returns a cursor which has not been zeroed. The only thing +// a caller should need to do is set c.stack's top correctly (it should be +// 0, and the [0] elem should be the root page to start on). +// +// TODO: Should this do anything about c.buffered? func (db *DB) getCursor(tx *Tx) *Cursor { c := cursorSyncPool.Get().(*Cursor) c.tx = tx @@ -828,6 +835,22 @@ type DebugInfo struct { Txs []*TxDebugInfo `json:"txs"` } +// when we want a cursor to access a free list, we are always doing this in +// a context specific to a write transaction, of which any DB can only have +// one at a time, and the operations modifying the free list don't recurse, +// because that would corrupt the list (see tx.freelistCleanup for the hairy +// details), which means that there is only ever one cursor being used for the +// free list, but also we use that cursor very often, and if we have to allocate +// it or zero it we end up with a lot of excess allocations and zeroing. +func (db *DB) getFreelistCursor(tx *Tx) *Cursor { + c := &db.freelistCursor + c.tx = tx + c.stack.elems[0] = stackElem{pgno: readMetaFreelistPageNo(tx.meta[:])} + c.stack.top = 0 + c.buffered = false + return c +} + // Shared pool for in-memory database pages. // These are used before being flushed to disk. var pagePool = &sync.Pool{} diff --git a/rbf/tx.go b/rbf/tx.go index b2a1a5185..4fa8f441e 100644 --- a/rbf/tx.go +++ b/rbf/tx.go @@ -984,6 +984,10 @@ func (tx *Tx) walkTree(pgno, parent uint32, fn func(pgno, parent, typ uint32, er // about that removing things from the free list, because the add logic // already just uses new pages rather than trying to use the free list // when it knows the free list is involved. +// +// Because this is expected to be used in a defer, instead of returning an +// error, it will set the error it got the address of to a new error if it +// encounters one and there wasn't one already. func (tx *Tx) freelistCleanup(outErr *error) { defer func() { // no matter what, we're done with this after this, but we still @@ -994,8 +998,7 @@ func (tx *Tx) freelistCleanup(outErr *error) { if len(tx.pendingFreelistAdds) == 0 { return } - c := Cursor{tx: tx} - c.stack.elems[0] = stackElem{pgno: readMetaFreelistPageNo(tx.meta[:])} + c := tx.db.getFreelistCursor(tx) for len(tx.pendingFreelistAdds) > 0 { var pass []uint32 pass, tx.pendingFreelistAdds = tx.pendingFreelistAdds, nil @@ -1006,7 +1009,7 @@ func (tx *Tx) freelistCleanup(outErr *error) { } return } else if !changed { - vprint.PanicOn(fmt.Sprintf("rbf.Tx.freePgno(): double free: %d", tx.pendingFreelistAdds)) + vprint.PanicOn(fmt.Sprintf("rbf.Tx.freelistCleanup(): double free: %d", pass)) } } } @@ -1015,44 +1018,25 @@ func (tx *Tx) freelistCleanup(outErr *error) { // allocatePgno returns a page number for a new available page. This page may be // pulled from the free list or, if no free pages are available, it will be // created by extending the file size. +// +// allocatePgno uses the freelist cursor (a shared db-wide thing), and sets +// the "modifyingFreelist" flag while it's running. If for some reason a +// modification to the freelist would require a new allocation or free, +// allocations always just create a new page, and frees are processed later +// by a separate call through a deferred tx.freelistCleanup(). func (tx *Tx) allocatePgno() (_ uint32, outErr error) { if tx.modifyingFreelist { return tx.allocateNewPgno(), nil } - // Attempt to find page in freelist. - pgno, err := tx.nextFreelistPageNo() - - if err != nil { - return 0, err - } else if pgno != 0 { - tx.modifyingFreelist = true - defer tx.freelistCleanup(&outErr) - c := Cursor{tx: tx} - c.stack.elems[0] = stackElem{pgno: readMetaFreelistPageNo(tx.meta[:])} - if changed, err := c.Remove(uint64(pgno)); err != nil { - return 0, err - } else if !changed { - vprint.PanicOn(fmt.Sprintf("tx.Tx.allocatePgno(): double alloc: %d", pgno)) - } - return pgno, nil - } - // no freelist pages, fall back - return tx.allocateNewPgno(), nil -} - -// allocateNewPgno requests a new page unconditionally, ignoring the free list. -func (tx *Tx) allocateNewPgno() uint32 { - // Increment the total page count by one and return the last page. - pgno := readMetaPageN(tx.meta[:]) - writeMetaPageN(tx.meta[:], pgno+1) - return pgno -} - -func (tx *Tx) nextFreelistPageNo() (uint32, error) { - c := Cursor{tx: tx} - c.stack.elems[0] = stackElem{pgno: readMetaFreelistPageNo(tx.meta[:])} + // this serves as a precaution against double-use of the freelist cursor + // used database-wide. we don't have actual synchronization here because + // only one write Tx should exist at once and it's not safe to use its + // write-capable ops concurrently anyway. + tx.modifyingFreelist = true + defer tx.freelistCleanup(&outErr) + c := tx.db.getFreelistCursor(tx) if err := c.First(); err == io.EOF { - return 0, nil + return tx.allocateNewPgno(), nil } else if err != nil { return 0, err } @@ -1067,17 +1051,30 @@ func (tx *Tx) nextFreelistPageNo() (uint32, error) { v := cell.firstValue(tx) pgno := uint32((cell.Key << 16) | uint64(v)) + + if changed, err := c.Remove(uint64(pgno)); err != nil { + return 0, err + } else if !changed { + vprint.PanicOn(fmt.Sprintf("tx.Tx.allocatePgno(): double alloc: %d", pgno)) + } return pgno, nil } +// allocateNewPgno requests a new page unconditionally, ignoring the free list. +func (tx *Tx) allocateNewPgno() uint32 { + // Increment the total page count by one and return the last page. + pgno := readMetaPageN(tx.meta[:]) + writeMetaPageN(tx.meta[:], pgno+1) + return pgno +} + // deallocate releases a page number to the freelist. func (tx *Tx) freePgno(pgno uint32) (outErr error) { if tx.modifyingFreelist { tx.pendingFreelistAdds = append(tx.pendingFreelistAdds, pgno) return nil } - c := Cursor{tx: tx} - c.stack.elems[0] = stackElem{pgno: readMetaFreelistPageNo(tx.meta[:])} + c := tx.db.getFreelistCursor(tx) tx.modifyingFreelist = true defer tx.freelistCleanup(&outErr)