From 3dfe957cb33d068ce204454cd674b40792f7dcce Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Fri, 20 May 2022 11:12:05 -0400 Subject: [PATCH 01/10] Add --memprofile option. Add memprofile to sample config. --- config.go | 1 + lbcd.go | 19 +++++++++++++++++++ sample-lbcd.conf | 3 +++ 3 files changed, 23 insertions(+) diff --git a/config.go b/config.go index fe77a8180e..aee85c3fed 100644 --- a/config.go +++ b/config.go @@ -117,6 +117,7 @@ type config struct { ConfigFile string `short:"C" long:"configfile" description:"Path to configuration file"` ConnectPeers []string `long:"connect" description:"Connect only to the specified peers at startup"` CPUProfile string `long:"cpuprofile" description:"Write CPU profile to the specified file"` + MemProfile string `long:"memprofile" description:"Write memory profile to the specified file"` DataDir string `short:"b" long:"datadir" description:"Directory to store data"` DbType string `long:"dbtype" description:"Database backend to use for the Block Chain"` DebugLevel string `short:"d" long:"debuglevel" description:"Logging level for all subsystems {trace, debug, info, warn, error, critical} -- You may also specify =,=,... to set the log level for individual subsystems -- Use show to list available subsystems"` diff --git a/lbcd.go b/lbcd.go index 1110eeaba1..0bc6841d47 100644 --- a/lbcd.go +++ b/lbcd.go @@ -92,6 +92,25 @@ func btcdMain(serverChan chan<- *server) error { defer pprof.StopCPUProfile() } + // Write memory profile if requested. + if cfg.MemProfile != "" { + f, err := os.Create(cfg.MemProfile + ".heap") + if err != nil { + btcdLog.Errorf("Unable to create mem profile: %v", err) + return err + } + defer f.Close() + defer pprof.Lookup("heap").WriteTo(f, 0) + + f, err = os.Create(cfg.MemProfile + ".allocs") + if err != nil { + btcdLog.Errorf("Unable to create mem profile: %v", err) + return err + } + defer f.Close() + defer pprof.Lookup("allocs").WriteTo(f, 0) + } + // Perform upgrades to btcd as new versions require it. if err := doUpgrades(); err != nil { btcdLog.Errorf("%v", err) diff --git a/sample-lbcd.conf b/sample-lbcd.conf index 2a84c79729..f2c0b8c7ad 100644 --- a/sample-lbcd.conf +++ b/sample-lbcd.conf @@ -376,6 +376,9 @@ ; Write CPU profile to the specified file. ; cpuprofile= +; Write memory profile to the specified file. +; memprofile= + ; The port used to listen for HTTP profile requests. The profile server will ; be disabled if this option is not specified. The profile information can be ; accessed at http://localhost:/debug/pprof once running. From fb871440aa3c5221b32166339a547200c660d8d7 Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Tue, 24 May 2022 08:56:24 -0400 Subject: [PATCH 02/10] Allow environment var GOGC= to override hard-coded SetGCPercent(10). --- lbcd.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lbcd.go b/lbcd.go index 0bc6841d47..3a7860b17d 100644 --- a/lbcd.go +++ b/lbcd.go @@ -300,7 +300,9 @@ func main() { // limits the garbage collector from excessively overallocating during // bursts. This value was arrived at with the help of profiling live // usage. - debug.SetGCPercent(10) + if _, ok := os.LookupEnv("GOGC"); !ok { + debug.SetGCPercent(10) + } // Up some limits. if err := limits.SetLimits(); err != nil { From f947011572ba443c224cc99a3a977e1458a34e5a Mon Sep 17 00:00:00 2001 From: Brannon King Date: Wed, 21 Jul 2021 15:58:47 -0400 Subject: [PATCH 03/10] added buffer pool for pebble merge string --- claimtrie/node/noderepo/pebble.go | 64 ++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/claimtrie/node/noderepo/pebble.go b/claimtrie/node/noderepo/pebble.go index cb5d25e4cc..e5ee75edc1 100644 --- a/claimtrie/node/noderepo/pebble.go +++ b/claimtrie/node/noderepo/pebble.go @@ -2,7 +2,9 @@ package noderepo import ( "bytes" + "io" "sort" + "sync" "github.com/cockroachdb/pebble" "github.com/lbryio/lbcd/claimtrie/change" @@ -13,9 +15,69 @@ type Pebble struct { db *pebble.DB } +type pooledMerger struct { + values [][]byte + index []int + pool *sync.Pool + buffer []byte +} + +func (a *pooledMerger) Len() int { return len(a.index) } +func (a *pooledMerger) Less(i, j int) bool { return a.index[i] < a.index[j] } +func (a *pooledMerger) Swap(i, j int) { + a.index[i], a.index[j] = a.index[j], a.index[i] + a.values[i], a.values[j] = a.values[j], a.values[i] +} + +func (a *pooledMerger) MergeNewer(value []byte) error { + a.values = append(a.values, value) + a.index = append(a.index, len(a.values)) + return nil +} + +func (a *pooledMerger) MergeOlder(value []byte) error { + a.values = append(a.values, value) + a.index = append(a.index, -len(a.values)) + return nil +} + +func (a *pooledMerger) Finish(includesBase bool) ([]byte, io.Closer, error) { + sort.Sort(a) + + a.buffer = a.pool.Get().([]byte)[:0] + for i := range a.values { + a.buffer = append(a.buffer, a.values[i]...) + } + + return a.buffer, a, nil +} + +func (a *pooledMerger) Close() error { + a.pool.Put(a.buffer) + return nil +} + func NewPebble(path string) (*Pebble, error) { - db, err := pebble.Open(path, &pebble.Options{Cache: pebble.NewCache(64 << 20), BytesPerSync: 8 << 20, MaxOpenFiles: 2000}) + mp := &sync.Pool{ + New: func() interface{} { + return make([]byte, 0, 256) + }, + } + + db, err := pebble.Open(path, &pebble.Options{ + Merger: &pebble.Merger{ + Merge: func(key, value []byte) (pebble.ValueMerger, error) { + p := &pooledMerger{pool: mp} + return p, p.MergeNewer(value) + }, + Name: pebble.DefaultMerger.Name, // yes, it's a lie + }, + Cache: pebble.NewCache(64 << 20), + BytesPerSync: 8 << 20, + MaxOpenFiles: 2000, + }) + repo := &Pebble{db: db} return repo, errors.Wrapf(err, "unable to open %s", path) From ca1f4c6301f609016cca5ac4e37cbeae713c86a6 Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Tue, 24 May 2022 09:00:09 -0400 Subject: [PATCH 04/10] Harden Marshal/Unmarshal logic for Change.SpentChildren. --- claimtrie/change/change.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/claimtrie/change/change.go b/claimtrie/change/change.go index aac349c651..95a0e3d5b8 100644 --- a/claimtrie/change/change.go +++ b/claimtrie/change/change.go @@ -78,9 +78,10 @@ func (c *Change) Marshal(enc *bytes.Buffer) error { binary.BigEndian.PutUint32(temp[:4], uint32(len(c.SpentChildren))) enc.Write(temp[:4]) for key := range c.SpentChildren { - binary.BigEndian.PutUint16(temp[:2], uint16(len(key))) // technically limited to 255; not sure we trust it + keySize := uint16(len(key)) + binary.BigEndian.PutUint16(temp[:2], keySize) // technically limited to 255; not sure we trust it enc.Write(temp[:2]) - enc.WriteString(key) + enc.WriteString(key[:keySize]) } } else { binary.BigEndian.PutUint32(temp[:4], 0) From 113883f98223dfadf44f3aff2b5b4d50e1ba3223 Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Tue, 24 May 2022 09:17:45 -0400 Subject: [PATCH 05/10] Copy value received by MergeOlder/MergeNewer so caller can't trash the merge result by modifying the contents. --- claimtrie/node/noderepo/pebble.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/claimtrie/node/noderepo/pebble.go b/claimtrie/node/noderepo/pebble.go index e5ee75edc1..32ca2c0412 100644 --- a/claimtrie/node/noderepo/pebble.go +++ b/claimtrie/node/noderepo/pebble.go @@ -30,13 +30,17 @@ func (a *pooledMerger) Swap(i, j int) { } func (a *pooledMerger) MergeNewer(value []byte) error { - a.values = append(a.values, value) + vc := a.pool.Get().([]byte)[:0] + vc = append(vc, value...) + a.values = append(a.values, vc) a.index = append(a.index, len(a.values)) return nil } func (a *pooledMerger) MergeOlder(value []byte) error { - a.values = append(a.values, value) + vc := a.pool.Get().([]byte)[:0] + vc = append(vc, value...) + a.values = append(a.values, vc) a.index = append(a.index, -len(a.values)) return nil } @@ -53,6 +57,9 @@ func (a *pooledMerger) Finish(includesBase bool) ([]byte, io.Closer, error) { } func (a *pooledMerger) Close() error { + for i := range a.values { + a.pool.Put(a.values[i]) + } a.pool.Put(a.buffer) return nil } From 3b40ce1b2a9fa500974834050b354be612b1143f Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Wed, 25 May 2022 06:47:08 -0400 Subject: [PATCH 06/10] Add Pool for []Change returned by LoadChanges(name []byte). Return []Changes and a "closer" func to return them to Pool. Update tests for LoadChanges new return values. --- claimtrie/cmd/cmd/node.go | 3 ++- claimtrie/node/manager.go | 3 ++- claimtrie/node/noderepo/noderepo_test.go | 6 +++-- claimtrie/node/noderepo/pebble.go | 30 +++++++++++++++++------- claimtrie/node/repo.go | 4 +++- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/claimtrie/cmd/cmd/node.go b/claimtrie/cmd/cmd/node.go index 08112e9446..4ae82e6bca 100644 --- a/claimtrie/cmd/cmd/node.go +++ b/claimtrie/cmd/cmd/node.go @@ -52,10 +52,11 @@ func NewNodeDumpCommand() *cobra.Command { } defer repo.Close() - changes, err := repo.LoadChanges([]byte(name)) + changes, closer, err := repo.LoadChanges([]byte(name)) if err != nil { return errors.Wrapf(err, "load commands") } + defer closer() for _, chg := range changes { if chg.Height > height { diff --git a/claimtrie/node/manager.go b/claimtrie/node/manager.go index 31ba0f1a20..912940e1fb 100644 --- a/claimtrie/node/manager.go +++ b/claimtrie/node/manager.go @@ -53,10 +53,11 @@ func (nm *BaseManager) NodeAt(height int32, name []byte) (*Node, error) { n, changes, oldHeight := nm.cache.fetch(name, height) if n == nil { - changes, err := nm.repo.LoadChanges(name) + changes, closer, err := nm.repo.LoadChanges(name) if err != nil { return nil, errors.Wrap(err, "in load changes") } + defer closer() if nm.tempChanges != nil { // making an assumption that we only ever have tempChanges for a single block changes = append(changes, nm.tempChanges[string(name)]...) diff --git a/claimtrie/node/noderepo/noderepo_test.go b/claimtrie/node/noderepo/noderepo_test.go index fb0a9764b8..c634ea5690 100644 --- a/claimtrie/node/noderepo/noderepo_test.go +++ b/claimtrie/node/noderepo/noderepo_test.go @@ -92,8 +92,9 @@ func testNodeRepo(t *testing.T, repo node.Repo, setup, cleanup func()) { err := repo.AppendChanges(tt.changes) r.NoError(err) - changes, err := repo.LoadChanges(testNodeName1) + changes, closer, err := repo.LoadChanges(testNodeName1) r.NoError(err) + defer closer() r.Equalf(tt.expected, changes[:len(tt.expected)], tt.name) cleanup() @@ -150,8 +151,9 @@ func testNodeRepo(t *testing.T, repo node.Repo, setup, cleanup func()) { r.NoError(err) } - changes, err := repo.LoadChanges(testNodeName1) + changes, closer, err := repo.LoadChanges(testNodeName1) r.NoError(err) + defer closer() r.Equalf(tt.expected, changes[:len(tt.expected)], tt.name) cleanup() diff --git a/claimtrie/node/noderepo/pebble.go b/claimtrie/node/noderepo/pebble.go index 32ca2c0412..63333a55d0 100644 --- a/claimtrie/node/noderepo/pebble.go +++ b/claimtrie/node/noderepo/pebble.go @@ -112,11 +112,11 @@ func (repo *Pebble) AppendChanges(changes []change.Change) error { return errors.Wrap(batch.Commit(pebble.NoSync), "in commit") } -func (repo *Pebble) LoadChanges(name []byte) ([]change.Change, error) { +func (repo *Pebble) LoadChanges(name []byte) ([]change.Change, func(), error) { data, closer, err := repo.db.Get(name) if err != nil && err != pebble.ErrNotFound { - return nil, errors.Wrapf(err, "in get %s", name) // does returning a name in an error expose too much? + return nil, nil, errors.Wrapf(err, "in get %s", name) // does returning a name in an error expose too much? } if closer != nil { defer closer.Close() @@ -125,9 +125,16 @@ func (repo *Pebble) LoadChanges(name []byte) ([]change.Change, error) { return unmarshalChanges(name, data) } -func unmarshalChanges(name, data []byte) ([]change.Change, error) { - // data is 84bytes+ per change - changes := make([]change.Change, 0, len(data)/84+1) // average is 5.1 changes +var changesPool = sync.Pool{ + New: func() interface{} { + return make([]change.Change, 0, 6) + }, +} + +func unmarshalChanges(name, data []byte) ([]change.Change, func(), error) { + // data is 84bytes+ per change, average is 5.1 changes + changes := changesPool.Get().([]change.Change)[:0] + closer := func() { changesPool.Put(changes) } buffer := bytes.NewBuffer(data) sortNeeded := false @@ -135,7 +142,8 @@ func unmarshalChanges(name, data []byte) ([]change.Change, error) { var chg change.Change err := chg.Unmarshal(buffer) if err != nil { - return nil, errors.Wrap(err, "in decode") + closer() + return nil, nil, errors.Wrap(err, "in decode") } chg.Name = name if len(changes) > 0 && chg.Height < changes[len(changes)-1].Height { @@ -150,14 +158,17 @@ func unmarshalChanges(name, data []byte) ([]change.Change, error) { return changes[i].Height < changes[j].Height }) } - return changes, nil + + return changes, closer, nil } func (repo *Pebble) DropChanges(name []byte, finalHeight int32) error { - changes, err := repo.LoadChanges(name) + changes, closer, err := repo.LoadChanges(name) if err != nil { return errors.Wrapf(err, "in load changes for %s", name) } + defer closer() + buffer := bytes.NewBuffer(nil) for i := 0; i < len(changes); i++ { // assuming changes are ordered by height if changes[i].Height > finalHeight { @@ -206,10 +217,11 @@ func (repo *Pebble) IterateChildren(name []byte, f func(changes []change.Change) for iter.First(); iter.Valid(); iter.Next() { // NOTE! iter.Key() is ephemeral! - changes, err := unmarshalChanges(iter.Key(), iter.Value()) + changes, closer, err := unmarshalChanges(iter.Key(), iter.Value()) if err != nil { return errors.Wrapf(err, "from unmarshaller at %s", iter.Key()) } + defer closer() if !f(changes) { break } diff --git a/claimtrie/node/repo.go b/claimtrie/node/repo.go index 4aaa65e894..a180db213c 100644 --- a/claimtrie/node/repo.go +++ b/claimtrie/node/repo.go @@ -13,7 +13,9 @@ type Repo interface { // LoadChanges loads changes of a node up to (includes) the specified height. // If no changes found, both returned slice and error will be nil. - LoadChanges(name []byte) ([]change.Change, error) + // The returned closer func() should be called to release changes after + // work on them is finished. + LoadChanges(name []byte) (changes []change.Change, closer func(), err error) DropChanges(name []byte, finalHeight int32) error From 63842b236dc7ddd1df927a000a36326f07108a4c Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Wed, 25 May 2022 06:50:11 -0400 Subject: [PATCH 07/10] Use map[ClaimID] instead of map[string] since conversion ClaimID -> string entails lots of temporary allocations. --- blockchain/claimtrie.go | 4 ++-- claimtrie/change/claimid.go | 4 ++-- claimtrie/node/node.go | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/blockchain/claimtrie.go b/blockchain/claimtrie.go index f821537ad5..0bf89978a2 100644 --- a/blockchain/claimtrie.go +++ b/blockchain/claimtrie.go @@ -35,7 +35,7 @@ func (b *BlockChain) ParseClaimScripts(block *btcutil.Block, bn *blockNode, view ht := block.Height() for _, tx := range block.Transactions() { - h := handler{ht, tx, view, map[string][]byte{}} + h := handler{ht, tx, view, map[change.ClaimID][]byte{}} if err := h.handleTxIns(b.claimTrie); err != nil { return err } @@ -67,7 +67,7 @@ type handler struct { ht int32 tx *btcutil.Tx view *UtxoViewpoint - spent map[string][]byte + spent map[change.ClaimID][]byte } func (h *handler) handleTxIns(ct *claimtrie.ClaimTrie) error { diff --git a/claimtrie/change/claimid.go b/claimtrie/change/claimid.go index e7a9256577..b05f498310 100644 --- a/claimtrie/change/claimid.go +++ b/claimtrie/change/claimid.go @@ -39,8 +39,8 @@ func NewIDFromString(s string) (id ClaimID, err error) { } // Key is for in-memory maps -func (id ClaimID) Key() string { - return string(id[:]) +func (id ClaimID) Key() ClaimID { + return id } // String is for anything written to a DB diff --git a/claimtrie/node/node.go b/claimtrie/node/node.go index ff45fc11e1..2e072d2723 100644 --- a/claimtrie/node/node.go +++ b/claimtrie/node/node.go @@ -14,12 +14,12 @@ type Node struct { TakenOverAt int32 // The height at when the current BestClaim took over. Claims ClaimList // List of all Claims. Supports ClaimList // List of all Supports, including orphaned ones. - SupportSums map[string]int64 + SupportSums map[change.ClaimID]int64 } // New returns a new node. func New() *Node { - return &Node{SupportSums: map[string]int64{}} + return &Node{SupportSums: map[change.ClaimID]int64{}} } func (n *Node) HasActiveBestClaim() bool { @@ -166,7 +166,7 @@ func (n *Node) handleExpiredAndActivated(height int32) int { } changes := 0 - update := func(items ClaimList, sums map[string]int64) ClaimList { + update := func(items ClaimList, sums map[change.ClaimID]int64) ClaimList { for i := 0; i < len(items); i++ { c := items[i] if c.Status == Accepted && c.ActiveAt <= height && c.VisibleAt <= height { @@ -343,7 +343,7 @@ func (n *Node) SortClaimsByBid() { func (n *Node) Clone() *Node { clone := New() if n.SupportSums != nil { - clone.SupportSums = map[string]int64{} + clone.SupportSums = map[change.ClaimID]int64{} for key, value := range n.SupportSums { clone.SupportSums[key] = value } From 928fd0898586d75b82b05e9ecfc43e036a5f9bc0 Mon Sep 17 00:00:00 2001 From: Brannon King Date: Thu, 22 Jul 2021 11:24:05 -0400 Subject: [PATCH 08/10] recycle treap nodes --- database/ffldb/dbcache.go | 10 +++++++ database/internal/treap/common.go | 7 +++++ database/internal/treap/immutable.go | 42 ++++++++++++++++++++++------ database/internal/treap/mutable.go | 25 ++++++++++++++++- 4 files changed, 75 insertions(+), 9 deletions(-) diff --git a/database/ffldb/dbcache.go b/database/ffldb/dbcache.go index eff239c603..3f06d0e7fc 100644 --- a/database/ffldb/dbcache.go +++ b/database/ffldb/dbcache.go @@ -516,6 +516,9 @@ func (c *dbCache) flush() error { return err } + cachedKeys.Recycle() + cachedRemove.Recycle() + return nil } @@ -574,9 +577,16 @@ func (c *dbCache) commitTx(tx *transaction) error { return err } + pk := tx.pendingKeys + pr := tx.pendingRemove + // Clear the transaction entries since they have been committed. tx.pendingKeys = nil tx.pendingRemove = nil + + pk.Recycle() + pr.Recycle() + return nil } diff --git a/database/internal/treap/common.go b/database/internal/treap/common.go index 090a7bd5ab..011aaffa55 100644 --- a/database/internal/treap/common.go +++ b/database/internal/treap/common.go @@ -42,6 +42,13 @@ type treapNode struct { right *treapNode } +func (n *treapNode) Reset() { + n.key = nil + n.value = nil + n.left = nil + n.right = nil +} + // nodeSize returns the number of bytes the specified node occupies including // the struct fields and the contents of the key and value. func nodeSize(node *treapNode) uint64 { diff --git a/database/internal/treap/immutable.go b/database/internal/treap/immutable.go index a6e13ff4a5..a23c6f7048 100644 --- a/database/internal/treap/immutable.go +++ b/database/internal/treap/immutable.go @@ -7,17 +7,20 @@ package treap import ( "bytes" "math/rand" + "sync" ) +var nodePool = &sync.Pool{New: func() interface{} { return newTreapNode(nil, nil, 0) }} + // cloneTreapNode returns a shallow copy of the passed node. func cloneTreapNode(node *treapNode) *treapNode { - return &treapNode{ - key: node.key, - value: node.value, - priority: node.priority, - left: node.left, - right: node.right, - } + clone := nodePool.Get().(*treapNode) + clone.key = node.key + clone.value = node.value + clone.priority = node.priority + clone.left = node.left + clone.right = node.right + return clone } // Immutable represents a treap data structure which is used to hold ordered @@ -165,7 +168,10 @@ func (t *Immutable) Put(key, value []byte) *Immutable { } // Link the new node into the binary tree in the correct position. - node := newTreapNode(key, value, rand.Int()) + node := nodePool.Get().(*treapNode) + node.key = key + node.value = value + node.priority = rand.Int() parent := parents.At(0) if compareResult < 0 { parent.left = node @@ -358,3 +364,23 @@ func (t *Immutable) ForEach(fn func(k, v []byte) bool) { func NewImmutable() *Immutable { return &Immutable{} } + +func (t *Immutable) Recycle() { + var parents parentStack + for node := t.root; node != nil; node = node.left { + parents.Push(node) + } + + for parents.Len() > 0 { + node := parents.Pop() + + // Extend the nodes to traverse by all children to the left of + // the current node's right child. + for n := node.right; n != nil; n = n.left { + parents.Push(n) + } + + node.Reset() + nodePool.Put(node) + } +} diff --git a/database/internal/treap/mutable.go b/database/internal/treap/mutable.go index 84ebe6715f..c68f326e65 100644 --- a/database/internal/treap/mutable.go +++ b/database/internal/treap/mutable.go @@ -145,7 +145,10 @@ func (t *Mutable) Put(key, value []byte) { } // Link the new node into the binary tree in the correct position. - node := newTreapNode(key, value, rand.Int()) + node := nodePool.Get().(*treapNode) + node.key = key + node.value = value + node.priority = rand.Int() t.count++ t.totalSize += nodeSize(node) parent := parents.At(0) @@ -276,3 +279,23 @@ func (t *Mutable) Reset() { func NewMutable() *Mutable { return &Mutable{} } + +func (t *Mutable) Recycle() { + var parents parentStack + for node := t.root; node != nil; node = node.left { + parents.Push(node) + } + + for parents.Len() > 0 { + node := parents.Pop() + + // Extend the nodes to traverse by all children to the left of + // the current node's right child. + for n := node.right; n != nil; n = n.left { + parents.Push(n) + } + + node.Reset() + nodePool.Put(node) + } +} From 289134656b04e3d30ab359ed07fd4f630ed2cfdc Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Sat, 28 May 2022 07:37:53 -0400 Subject: [PATCH 09/10] Distinguish between Mutable/Immutable treap node recycling, and make Mutable treap recycle nodes consistently. Rework Immutable treap node recycling attempting to make it safer in the presence of code that takes snapshots (dbCacheSnapshot) of the treap. Add special mutable PutM and DeleteM methods which DB transaction can use to apply changes more efficiently without creating lots of garbage memory. --- database/ffldb/dbcache.go | 42 ++++-- database/internal/treap/common.go | 59 ++++++-- database/internal/treap/immutable.go | 215 +++++++++++++++++++++++---- database/internal/treap/mutable.go | 14 +- 4 files changed, 263 insertions(+), 67 deletions(-) diff --git a/database/ffldb/dbcache.go b/database/ffldb/dbcache.go index 3f06d0e7fc..52c6737f9a 100644 --- a/database/ffldb/dbcache.go +++ b/database/ffldb/dbcache.go @@ -285,9 +285,11 @@ func (iter *dbCacheIterator) Error() error { // dbCacheSnapshot defines a snapshot of the database cache and underlying // database at a particular point in time. type dbCacheSnapshot struct { - dbSnapshot *leveldb.Snapshot - pendingKeys *treap.Immutable - pendingRemove *treap.Immutable + dbSnapshot *leveldb.Snapshot + pendingKeys *treap.Immutable + pendingRemove *treap.Immutable + pendingKeysSnap *treap.SnapRecord + pendingRemoveSnap *treap.SnapRecord } // Has returns whether or not the passed key exists. @@ -327,6 +329,8 @@ func (snap *dbCacheSnapshot) Get(key []byte) []byte { // Release releases the snapshot. func (snap *dbCacheSnapshot) Release() { snap.dbSnapshot.Release() + snap.pendingKeysSnap.Release() + snap.pendingRemoveSnap.Release() snap.pendingKeys = nil snap.pendingRemove = nil } @@ -407,9 +411,11 @@ func (c *dbCache) Snapshot() (*dbCacheSnapshot, error) { // which is used to atomically swap the root. c.cacheLock.RLock() cacheSnapshot := &dbCacheSnapshot{ - dbSnapshot: dbSnapshot, - pendingKeys: c.cachedKeys, - pendingRemove: c.cachedRemove, + dbSnapshot: dbSnapshot, + pendingKeys: c.cachedKeys, + pendingRemove: c.cachedRemove, + pendingKeysSnap: c.cachedKeys.Snapshot(), + pendingRemoveSnap: c.cachedRemove.Snapshot(), } c.cacheLock.RUnlock() return cacheSnapshot, nil @@ -499,12 +505,10 @@ func (c *dbCache) flush() error { // Since the cached keys to be added and removed use an immutable treap, // a snapshot is simply obtaining the root of the tree under the lock // which is used to atomically swap the root. - c.cacheLock.Lock() + c.cacheLock.RLock() cachedKeys := c.cachedKeys cachedRemove := c.cachedRemove - c.cachedKeys = treap.NewImmutable() - c.cachedRemove = treap.NewImmutable() - c.cacheLock.Unlock() + c.cacheLock.RUnlock() // Nothing to do if there is no data to flush. if cachedKeys.Len() == 0 && cachedRemove.Len() == 0 { @@ -516,6 +520,11 @@ func (c *dbCache) flush() error { return err } + c.cacheLock.Lock() + c.cachedKeys = treap.NewImmutable() + c.cachedRemove = treap.NewImmutable() + c.cacheLock.Unlock() + cachedKeys.Recycle() cachedRemove.Recycle() @@ -603,19 +612,23 @@ func (c *dbCache) commitTx(tx *transaction) error { // Apply every key to add in the database transaction to the cache. tx.pendingKeys.ForEach(func(k, v []byte) bool { - newCachedRemove = newCachedRemove.Delete(k) - newCachedKeys = newCachedKeys.Put(k, v) + treap.DeleteM(&newCachedRemove, k, tx.snapshot.pendingRemoveSnap) + treap.PutM(&newCachedKeys, k, v, tx.snapshot.pendingKeysSnap) return true }) + pk := tx.pendingKeys tx.pendingKeys = nil + pk.Recycle() // Apply every key to remove in the database transaction to the cache. tx.pendingRemove.ForEach(func(k, v []byte) bool { - newCachedKeys = newCachedKeys.Delete(k) - newCachedRemove = newCachedRemove.Put(k, nil) + treap.DeleteM(&newCachedKeys, k, tx.snapshot.pendingKeysSnap) + treap.PutM(&newCachedRemove, k, nil, tx.snapshot.pendingRemoveSnap) return true }) + pr := tx.pendingRemove tx.pendingRemove = nil + pr.Recycle() // Atomically replace the immutable treaps which hold the cached keys to // add and delete. @@ -623,6 +636,7 @@ func (c *dbCache) commitTx(tx *transaction) error { c.cachedKeys = newCachedKeys c.cachedRemove = newCachedRemove c.cacheLock.Unlock() + return nil } diff --git a/database/internal/treap/common.go b/database/internal/treap/common.go index 011aaffa55..3ab2504576 100644 --- a/database/internal/treap/common.go +++ b/database/internal/treap/common.go @@ -6,6 +6,7 @@ package treap import ( "math/rand" + "sync" "time" ) @@ -23,7 +24,7 @@ const ( // size in that case is acceptable since it avoids the need to import // unsafe. It consists of 24-bytes for each key and value + 8 bytes for // each of the priority, left, and right fields (24*2 + 8*3). - nodeFieldsSize = 72 + nodeFieldsSize = 80 ) var ( @@ -33,20 +34,21 @@ var ( emptySlice = make([]byte, 0) ) +const ( + // Generation number for nodes in a Mutable treap. + MutableGeneration int = -1 + // Generation number for nodes in the free Pool. + PoolGeneration int = -2 +) + // treapNode represents a node in the treap. type treapNode struct { - key []byte - value []byte - priority int - left *treapNode - right *treapNode -} - -func (n *treapNode) Reset() { - n.key = nil - n.value = nil - n.left = nil - n.right = nil + key []byte + value []byte + priority int + left *treapNode + right *treapNode + generation int } // nodeSize returns the number of bytes the specified node occupies including @@ -55,10 +57,35 @@ func nodeSize(node *treapNode) uint64 { return nodeFieldsSize + uint64(len(node.key)+len(node.value)) } -// newTreapNode returns a new node from the given key, value, and priority. The +// Pool of treapNode available for reuse. +var nodePool = &sync.Pool{ + New: func() interface{} { + return &treapNode{key: nil, value: nil, priority: 0, generation: PoolGeneration} + }, +} + +// getTreapNode returns a new node from the given key, value, and priority. The // node is not initially linked to any others. -func newTreapNode(key, value []byte, priority int) *treapNode { - return &treapNode{key: key, value: value, priority: priority} +func getTreapNode(key, value []byte, priority int, generation int) *treapNode { + n := nodePool.Get().(*treapNode) + n.key = key + n.value = value + n.priority = priority + n.left = nil + n.right = nil + n.generation = generation + return n +} + +// Put treapNode back in the nodePool for reuse. +func putTreapNode(n *treapNode) { + n.key = nil + n.value = nil + n.priority = 0 + n.left = nil + n.right = nil + n.generation = PoolGeneration + nodePool.Put(n) } // parentStack represents a stack of parent treap nodes that are used during diff --git a/database/internal/treap/immutable.go b/database/internal/treap/immutable.go index a23c6f7048..a9d59fb312 100644 --- a/database/internal/treap/immutable.go +++ b/database/internal/treap/immutable.go @@ -10,14 +10,9 @@ import ( "sync" ) -var nodePool = &sync.Pool{New: func() interface{} { return newTreapNode(nil, nil, 0) }} - // cloneTreapNode returns a shallow copy of the passed node. func cloneTreapNode(node *treapNode) *treapNode { - clone := nodePool.Get().(*treapNode) - clone.key = node.key - clone.value = node.value - clone.priority = node.priority + clone := getTreapNode(node.key, node.value, node.priority, node.generation+1) clone.left = node.left clone.right = node.right return clone @@ -46,11 +41,19 @@ type Immutable struct { // totalSize is the best estimate of the total size of of all data in // the treap including the keys, values, and node sizes. totalSize uint64 + + // generation number starts at 0 after NewImmutable(), and + // is incremented with every Put()/Delete(). + generation int + + // snap is a pointer to a node in snapshot history linked list. + // A value nil means no snapshots are outstanding. + snap *SnapRecord } // newImmutable returns a new immutable treap given the passed parameters. -func newImmutable(root *treapNode, count int, totalSize uint64) *Immutable { - return &Immutable{root: root, count: count, totalSize: totalSize} +func newImmutable(root *treapNode, count int, totalSize uint64, generation int, snap *SnapRecord) *Immutable { + return &Immutable{root: root, count: count, totalSize: totalSize, generation: generation, snap: snap} } // Len returns the number of items stored in the treap. @@ -107,8 +110,8 @@ func (t *Immutable) Get(key []byte) []byte { return nil } -// Put inserts the passed key/value pair. -func (t *Immutable) Put(key, value []byte) *Immutable { +// put inserts the passed key/value pair. +func (t *Immutable) put(key, value []byte, bumpGen int) (tp *Immutable, old parentStack) { // Use an empty byte slice for the value when none was provided. This // ultimately allows key existence to be determined from the value since // an empty byte slice is distinguishable from nil. @@ -118,8 +121,8 @@ func (t *Immutable) Put(key, value []byte) *Immutable { // The node is the root of the tree if there isn't already one. if t.root == nil { - root := newTreapNode(key, value, rand.Int()) - return newImmutable(root, 1, nodeSize(root)) + root := getTreapNode(key, value, rand.Int(), t.generation+bumpGen) + return newImmutable(root, 1, nodeSize(root), t.generation+bumpGen, t.snap), parentStack{} } // Find the binary tree insertion point and construct a replaced list of @@ -131,9 +134,11 @@ func (t *Immutable) Put(key, value []byte) *Immutable { // When the key matches an entry already in the treap, replace the node // with a new one that has the new value set and return. var parents parentStack + var oldParents parentStack var compareResult int for node := t.root; node != nil; { // Clone the node and link its parent to it if needed. + oldParents.Push(node) nodeCopy := cloneTreapNode(node) if oldParent := parents.At(0); oldParent != nil { if oldParent.left == node { @@ -164,14 +169,11 @@ func (t *Immutable) Put(key, value []byte) *Immutable { newRoot := parents.At(parents.Len() - 1) newTotalSize := t.totalSize - uint64(len(node.value)) + uint64(len(value)) - return newImmutable(newRoot, t.count, newTotalSize) + return newImmutable(newRoot, t.count, newTotalSize, t.generation+bumpGen, t.snap), oldParents } // Link the new node into the binary tree in the correct position. - node := nodePool.Get().(*treapNode) - node.key = key - node.value = value - node.priority = rand.Int() + node := getTreapNode(key, value, rand.Int(), t.generation+bumpGen) parent := parents.At(0) if compareResult < 0 { parent.left = node @@ -211,19 +213,59 @@ func (t *Immutable) Put(key, value []byte) *Immutable { } } - return newImmutable(newRoot, t.count+1, t.totalSize+nodeSize(node)) + return newImmutable(newRoot, t.count+1, t.totalSize+nodeSize(node), t.generation+bumpGen, t.snap), oldParents } -// Delete removes the passed key from the treap and returns the resulting treap +// Put is the immutable variant of put. Generation number is bumped, and old +// nodes become garbage unless referenced elswhere. +func (t *Immutable) Put(key, value []byte) *Immutable { + tp, _ := t.put(key, value, 1) + return tp +} + +// PutM is the mutable variant of put. Generation number is NOT bumped, and old +// nodes are recycled if possible. This is only safe/useful in scenarios where +// multiple Put/Delete() ops are applied to a unique treap and no snapshots/aliases +// of the intermediate treap states are created or desired. For example: +// +// for i := range keys { +// t = t.Put(keys[i]) +// } +// +// ...may be replaced with: +// +// for i := range keys { +// PutM(t, keys[i], nil) +// } +// +// If "excluded" is provided, that snapshot is ignored when counting +// snapshot records. +// +func PutM(dest **Immutable, key, value []byte, excluded *SnapRecord) { + tp, old := (*dest).put(key, value, 0) + // Examine old nodes and recycle if possible. + snapRecordMutex.Lock() + defer snapRecordMutex.Unlock() + snapCount := (*dest).snapCount(excluded) + for old.Len() > 0 { + node := old.Pop() + if node.generation == tp.generation && snapCount == 0 { + putTreapNode(node) + } + } + *dest = tp +} + +// del removes the passed key from the treap and returns the resulting treap // if it exists. The original immutable treap is returned if the key does not // exist. -func (t *Immutable) Delete(key []byte) *Immutable { +func (t *Immutable) del(key []byte, bumpGen int) (d *Immutable, old parentStack) { // Find the node for the key while constructing a list of parents while // doing so. - var parents parentStack + var oldParents parentStack var delNode *treapNode for node := t.root; node != nil; { - parents.Push(node) + oldParents.Push(node) // Traverse left or right depending on the result of the // comparison. @@ -244,14 +286,14 @@ func (t *Immutable) Delete(key []byte) *Immutable { // There is nothing to do if the key does not exist. if delNode == nil { - return t + return t, parentStack{} } // When the only node in the tree is the root node and it is the one // being deleted, there is nothing else to do besides removing it. - parent := parents.At(1) + parent := oldParents.At(1) if parent == nil && delNode.left == nil && delNode.right == nil { - return newImmutable(nil, 0, 0) + return newImmutable(nil, 0, 0, t.generation+bumpGen, t.snap), oldParents } // Construct a replaced list of parents and the node to delete itself. @@ -259,8 +301,8 @@ func (t *Immutable) Delete(key []byte) *Immutable { // therefore all ancestors of the node that will be deleted, up to and // including the root, need to be replaced. var newParents parentStack - for i := parents.Len(); i > 0; i-- { - node := parents.At(i - 1) + for i := oldParents.Len(); i > 0; i-- { + node := oldParents.At(i - 1) nodeCopy := cloneTreapNode(node) if oldParent := newParents.At(0); oldParent != nil { if oldParent.left == node { @@ -332,7 +374,47 @@ func (t *Immutable) Delete(key []byte) *Immutable { parent.left = nil } - return newImmutable(newRoot, t.count-1, t.totalSize-nodeSize(delNode)) + return newImmutable(newRoot, t.count-1, t.totalSize-nodeSize(delNode), t.generation+bumpGen, t.snap), oldParents +} + +// Delete is the immutable variant of del. Generation number is bumped, and old +// nodes become garbage unless referenced elswhere. +func (t *Immutable) Delete(key []byte) *Immutable { + tp, _ := t.del(key, 1) + return tp +} + +// DeleteM is the mutable variant of del. Generation number is NOT bumped, and old +// nodes are recycled if possible. This is only safe/useful in scenarios where +// multiple Put/Delete() ops are applied to a unique treap and no snapshots/aliases +// of the intermediate treap states are created or desired. For example: +// +// for i := range keys { +// t = t.Delete(keys[i]) +// } +// +// ...may be replaced with: +// +// for i := range keys { +// DeleteM(t, keys[i], nil) +// } +// +// If "excluded" is provided, that snapshot is ignored when counting +// snapshot records. +// +func DeleteM(dest **Immutable, key []byte, excluded *SnapRecord) { + tp, old := (*dest).del(key, 0) + // Examine old nodes and recycle if possible. + snapRecordMutex.Lock() + defer snapRecordMutex.Unlock() + snapCount := (*dest).snapCount(excluded) + for old.Len() > 0 { + node := old.Pop() + if node.generation == tp.generation && snapCount == 0 { + putTreapNode(node) + } + } + *dest = tp } // ForEach invokes the passed function with every key/value pair in the treap @@ -365,7 +447,79 @@ func NewImmutable() *Immutable { return &Immutable{} } +// SnapRecord assists in tracking/releasing outstanding snapshots. +type SnapRecord struct { + prev *SnapRecord + next *SnapRecord +} + +var snapRecordMutex sync.Mutex + +// Snapshot makes a SnapRecord and linkis it into the snapshot history of a treap. +func (t *Immutable) Snapshot() *SnapRecord { + snapRecordMutex.Lock() + defer snapRecordMutex.Unlock() + + // Link this record so it follows the existing t.snap record, if any. + prev := t.snap + var next *SnapRecord = nil + if prev != nil { + next = prev.next + } + t.snap = &SnapRecord{prev: prev, next: next} + if prev != nil { + prev.next = t.snap + } + + return t.snap +} + +// Release of SnapRecord unlinks that record from the snapshot history of a treap. +func (r *SnapRecord) Release() { + snapRecordMutex.Lock() + defer snapRecordMutex.Unlock() + + // Unlink this record. + if r.prev != nil { + r.prev.next = r.next + } + if r.next != nil { + r.next.prev = r.prev + } +} + +// snapCount returns the number of snapshots outstanding which were created +// but not released. When snapshots are absent, mutable PutM()/DeleteM() can +// recycle nodes more aggressively. The record "exclude" is not counted. +func (t *Immutable) snapCount(exclude *SnapRecord) int { + // snapRecordMutex should be locked already + + sum := 0 + if t.snap == nil { + // No snapshots. + return sum + } + + // Count snapshots taken BEFORE creation of this instance. + for h := t.snap; h != nil; h = h.prev { + if h != exclude { + sum++ + } + } + + // Count snapshots taken AFTER creation of this instance. + for h := t.snap.next; h != nil; h = h.next { + if h != exclude { + sum++ + } + } + + return sum +} + func (t *Immutable) Recycle() { + snapCount := t.snapCount(nil) - 1 + var parents parentStack for node := t.root; node != nil; node = node.left { parents.Push(node) @@ -380,7 +534,8 @@ func (t *Immutable) Recycle() { parents.Push(n) } - node.Reset() - nodePool.Put(node) + if node.generation == t.generation && snapCount == 0 { + putTreapNode(node) + } } } diff --git a/database/internal/treap/mutable.go b/database/internal/treap/mutable.go index c68f326e65..1ded6808e4 100644 --- a/database/internal/treap/mutable.go +++ b/database/internal/treap/mutable.go @@ -113,7 +113,7 @@ func (t *Mutable) Put(key, value []byte) { // The node is the root of the tree if there isn't already one. if t.root == nil { - node := newTreapNode(key, value, rand.Int()) + node := getTreapNode(key, value, rand.Int(), MutableGeneration) t.count = 1 t.totalSize = nodeSize(node) t.root = node @@ -145,10 +145,7 @@ func (t *Mutable) Put(key, value []byte) { } // Link the new node into the binary tree in the correct position. - node := nodePool.Get().(*treapNode) - node.key = key - node.value = value - node.priority = rand.Int() + node := getTreapNode(key, value, rand.Int(), MutableGeneration) t.count++ t.totalSize += nodeSize(node) parent := parents.At(0) @@ -193,6 +190,7 @@ func (t *Mutable) Delete(key []byte) { t.root = nil t.count = 0 t.totalSize = 0 + putTreapNode(node) return } @@ -241,6 +239,7 @@ func (t *Mutable) Delete(key []byte) { } t.count-- t.totalSize -= nodeSize(node) + putTreapNode(node) } // ForEach invokes the passed function with every key/value pair in the treap @@ -295,7 +294,8 @@ func (t *Mutable) Recycle() { parents.Push(n) } - node.Reset() - nodePool.Put(node) + if node.generation == MutableGeneration { + putTreapNode(node) + } } } From 51c44e48a56a4ccc62338f046fb8b75295d5e991 Mon Sep 17 00:00:00 2001 From: Jonathan Moody <103143855+moodyjon@users.noreply.github.com> Date: Thu, 2 Jun 2022 14:03:26 -0400 Subject: [PATCH 10/10] Fix lots of flaws in treap snapshot tracking. Update tests to make them work. --- database/ffldb/dbcache.go | 45 +++++-- database/ffldb/whitebox_test.go | 2 +- database/internal/treap/common.go | 36 +++-- database/internal/treap/common_test.go | 2 +- database/internal/treap/immutable.go | 177 ++++++++++++++++--------- database/internal/treap/mutable.go | 20 ++- database/internal/treap/treapiter.go | 1 + 7 files changed, 189 insertions(+), 94 deletions(-) diff --git a/database/ffldb/dbcache.go b/database/ffldb/dbcache.go index 52c6737f9a..9a690c226b 100644 --- a/database/ffldb/dbcache.go +++ b/database/ffldb/dbcache.go @@ -290,6 +290,7 @@ type dbCacheSnapshot struct { pendingRemove *treap.Immutable pendingKeysSnap *treap.SnapRecord pendingRemoveSnap *treap.SnapRecord + cacheFlushed int } // Has returns whether or not the passed key exists. @@ -329,8 +330,18 @@ func (snap *dbCacheSnapshot) Get(key []byte) []byte { // Release releases the snapshot. func (snap *dbCacheSnapshot) Release() { snap.dbSnapshot.Release() - snap.pendingKeysSnap.Release() - snap.pendingRemoveSnap.Release() + if snap.cacheFlushed > 0 && snap.pendingKeys != nil { + snap.pendingKeys.Recycle(snap.pendingKeysSnap) + } + if snap.cacheFlushed > 0 && snap.pendingRemove != nil { + snap.pendingRemove.Recycle(snap.pendingRemoveSnap) + } + if snap.pendingKeysSnap != nil { + snap.pendingKeysSnap.Release() + } + if snap.pendingRemoveSnap != nil { + snap.pendingRemoveSnap.Release() + } snap.pendingKeys = nil snap.pendingRemove = nil } @@ -411,11 +422,15 @@ func (c *dbCache) Snapshot() (*dbCacheSnapshot, error) { // which is used to atomically swap the root. c.cacheLock.RLock() cacheSnapshot := &dbCacheSnapshot{ - dbSnapshot: dbSnapshot, - pendingKeys: c.cachedKeys, - pendingRemove: c.cachedRemove, - pendingKeysSnap: c.cachedKeys.Snapshot(), - pendingRemoveSnap: c.cachedRemove.Snapshot(), + dbSnapshot: dbSnapshot, + pendingKeys: c.cachedKeys, + pendingRemove: c.cachedRemove, + } + if cacheSnapshot.pendingKeys != nil { + cacheSnapshot.pendingKeysSnap = cacheSnapshot.pendingKeys.Snapshot() + } + if cacheSnapshot.pendingRemove != nil { + cacheSnapshot.pendingRemoveSnap = cacheSnapshot.pendingRemove.Snapshot() } c.cacheLock.RUnlock() return cacheSnapshot, nil @@ -491,7 +506,7 @@ func (c *dbCache) commitTreaps(pendingKeys, pendingRemove TreapForEacher) error // cache to the underlying database. // // This function MUST be called with the database write lock held. -func (c *dbCache) flush() error { +func (c *dbCache) flush(tx *transaction) error { c.lastFlush = time.Now() // Sync the current write file associated with the block store. This is @@ -525,8 +540,14 @@ func (c *dbCache) flush() error { c.cachedRemove = treap.NewImmutable() c.cacheLock.Unlock() - cachedKeys.Recycle() - cachedRemove.Recycle() + cachedKeys.Recycle(nil) + cachedRemove.Recycle(nil) + + // Make a note that cache was flushed so tx.snapshot.Release() + // can also call Recycle() to free more nodes. + if tx != nil && tx.snapshot != nil { + tx.snapshot.cacheFlushed++ + } return nil } @@ -576,7 +597,7 @@ func (c *dbCache) commitTx(tx *transaction) error { // Flush the cache and write the current transaction directly to the // database if a flush is needed. if c.needsFlush(tx) { - if err := c.flush(); err != nil { + if err := c.flush(tx); err != nil { return err } @@ -646,7 +667,7 @@ func (c *dbCache) commitTx(tx *transaction) error { // This function MUST be called with the database write lock held. func (c *dbCache) Close() error { // Flush any outstanding cached entries to disk. - if err := c.flush(); err != nil { + if err := c.flush(nil); err != nil { // Even if there is an error while flushing, attempt to close // the underlying database. The error is ignored since it would // mask the flush error. diff --git a/database/ffldb/whitebox_test.go b/database/ffldb/whitebox_test.go index 31a8ab34d8..47ac6b740d 100644 --- a/database/ffldb/whitebox_test.go +++ b/database/ffldb/whitebox_test.go @@ -319,7 +319,7 @@ func testWriteFailures(tc *testContext) bool { file: &mockFile{forceSyncErr: true, maxSize: -1}, } store.writeCursor.Unlock() - err := tc.db.(*db).cache.flush() + err := tc.db.(*db).cache.flush(nil) if !checkDbError(tc.t, testName, err, database.ErrDriverSpecific) { return false } diff --git a/database/internal/treap/common.go b/database/internal/treap/common.go index 3ab2504576..8df868729a 100644 --- a/database/internal/treap/common.go +++ b/database/internal/treap/common.go @@ -24,7 +24,7 @@ const ( // size in that case is acceptable since it avoids the need to import // unsafe. It consists of 24-bytes for each key and value + 8 bytes for // each of the priority, left, and right fields (24*2 + 8*3). - nodeFieldsSize = 80 + nodeFieldsSize = 96 ) var ( @@ -35,10 +35,12 @@ var ( ) const ( - // Generation number for nodes in a Mutable treap. - MutableGeneration int = -1 - // Generation number for nodes in the free Pool. - PoolGeneration int = -2 + // mutableGeneration is the generation number for nodes in a Mutable treap. + mutableGeneration int = -1 + // recycleGeneration indicates node is scheduled for recycling back to nodePool. + recycleGeneration int = -2 + // poolGeneration is the generation number for free nodes in the nodePool. + poolGeneration int = -3 ) // treapNode represents a node in the treap. @@ -49,6 +51,7 @@ type treapNode struct { left *treapNode right *treapNode generation int + next *treapNode } // nodeSize returns the number of bytes the specified node occupies including @@ -57,15 +60,15 @@ func nodeSize(node *treapNode) uint64 { return nodeFieldsSize + uint64(len(node.key)+len(node.value)) } -// Pool of treapNode available for reuse. -var nodePool = &sync.Pool{ - New: func() interface{} { - return &treapNode{key: nil, value: nil, priority: 0, generation: PoolGeneration} - }, +func poolNewTreapNode() interface{} { + return &treapNode{key: nil, value: nil, priority: 0, generation: poolGeneration} } -// getTreapNode returns a new node from the given key, value, and priority. The -// node is not initially linked to any others. +// Pool of treapNode available for reuse. +var nodePool sync.Pool = sync.Pool{New: poolNewTreapNode} + +// getTreapNode returns a node from nodePool with the given key, value, priority, +// and generation. The node is not initially linked to any others. func getTreapNode(key, value []byte, priority int, generation int) *treapNode { n := nodePool.Get().(*treapNode) n.key = key @@ -74,17 +77,22 @@ func getTreapNode(key, value []byte, priority int, generation int) *treapNode { n.left = nil n.right = nil n.generation = generation + n.next = nil return n } -// Put treapNode back in the nodePool for reuse. +// putTreapNode returns a node back to nodePool for reuse. func putTreapNode(n *treapNode) { + if n.generation <= poolGeneration { + panic("double free of treapNode detected") + } n.key = nil n.value = nil n.priority = 0 n.left = nil n.right = nil - n.generation = PoolGeneration + n.generation = poolGeneration + n.next = nil nodePool.Put(n) } diff --git a/database/internal/treap/common_test.go b/database/internal/treap/common_test.go index c43e678de7..6cd1363b70 100644 --- a/database/internal/treap/common_test.go +++ b/database/internal/treap/common_test.go @@ -49,7 +49,7 @@ testLoop: for j := 0; j < test.numNodes; j++ { var key [4]byte binary.BigEndian.PutUint32(key[:], uint32(j)) - node := newTreapNode(key[:], key[:], 0) + node := getTreapNode(key[:], key[:], 0, 0) nodes = append(nodes, node) } diff --git a/database/internal/treap/immutable.go b/database/internal/treap/immutable.go index a9d59fb312..be58a8af02 100644 --- a/database/internal/treap/immutable.go +++ b/database/internal/treap/immutable.go @@ -48,11 +48,11 @@ type Immutable struct { // snap is a pointer to a node in snapshot history linked list. // A value nil means no snapshots are outstanding. - snap *SnapRecord + snap **SnapRecord } // newImmutable returns a new immutable treap given the passed parameters. -func newImmutable(root *treapNode, count int, totalSize uint64, generation int, snap *SnapRecord) *Immutable { +func newImmutable(root *treapNode, count int, totalSize uint64, generation int, snap **SnapRecord) *Immutable { return &Immutable{root: root, count: count, totalSize: totalSize, generation: generation, snap: snap} } @@ -111,7 +111,7 @@ func (t *Immutable) Get(key []byte) []byte { } // put inserts the passed key/value pair. -func (t *Immutable) put(key, value []byte, bumpGen int) (tp *Immutable, old parentStack) { +func (t *Immutable) put(key, value []byte) (tp *Immutable, old parentStack) { // Use an empty byte slice for the value when none was provided. This // ultimately allows key existence to be determined from the value since // an empty byte slice is distinguishable from nil. @@ -121,8 +121,8 @@ func (t *Immutable) put(key, value []byte, bumpGen int) (tp *Immutable, old pare // The node is the root of the tree if there isn't already one. if t.root == nil { - root := getTreapNode(key, value, rand.Int(), t.generation+bumpGen) - return newImmutable(root, 1, nodeSize(root), t.generation+bumpGen, t.snap), parentStack{} + root := getTreapNode(key, value, rand.Int(), t.generation+1) + return newImmutable(root, 1, nodeSize(root), t.generation+1, t.snap), parentStack{} } // Find the binary tree insertion point and construct a replaced list of @@ -169,11 +169,11 @@ func (t *Immutable) put(key, value []byte, bumpGen int) (tp *Immutable, old pare newRoot := parents.At(parents.Len() - 1) newTotalSize := t.totalSize - uint64(len(node.value)) + uint64(len(value)) - return newImmutable(newRoot, t.count, newTotalSize, t.generation+bumpGen, t.snap), oldParents + return newImmutable(newRoot, t.count, newTotalSize, t.generation+1, t.snap), oldParents } // Link the new node into the binary tree in the correct position. - node := getTreapNode(key, value, rand.Int(), t.generation+bumpGen) + node := getTreapNode(key, value, rand.Int(), t.generation+1) parent := parents.At(0) if compareResult < 0 { parent.left = node @@ -213,20 +213,21 @@ func (t *Immutable) put(key, value []byte, bumpGen int) (tp *Immutable, old pare } } - return newImmutable(newRoot, t.count+1, t.totalSize+nodeSize(node), t.generation+bumpGen, t.snap), oldParents + return newImmutable(newRoot, t.count+1, t.totalSize+nodeSize(node), t.generation+1, t.snap), oldParents } -// Put is the immutable variant of put. Generation number is bumped, and old -// nodes become garbage unless referenced elswhere. +// Put is the immutable variant of put. Old nodes become garbage unless referenced elswhere. func (t *Immutable) Put(key, value []byte) *Immutable { - tp, _ := t.put(key, value, 1) + tp, _ := t.put(key, value) return tp } -// PutM is the mutable variant of put. Generation number is NOT bumped, and old -// nodes are recycled if possible. This is only safe/useful in scenarios where -// multiple Put/Delete() ops are applied to a unique treap and no snapshots/aliases -// of the intermediate treap states are created or desired. For example: +// PutM is the mutable variant of put. Old nodes are recycled if possible. This is +// only safe in structured scenarios using SnapRecord to track treap instances. +// The outstanding SnapRecords serve to protect nodes from recycling when they might +// be present in one or more snapshots. This is useful in scenarios where multiple +// Put/Delete() ops are applied to a treap and intermediate treap states are not +// created or desired. For example: // // for i := range keys { // t = t.Put(keys[i]) @@ -242,15 +243,20 @@ func (t *Immutable) Put(key, value []byte) *Immutable { // snapshot records. // func PutM(dest **Immutable, key, value []byte, excluded *SnapRecord) { - tp, old := (*dest).put(key, value, 0) + tp, old := (*dest).put(key, value) // Examine old nodes and recycle if possible. snapRecordMutex.Lock() defer snapRecordMutex.Unlock() - snapCount := (*dest).snapCount(excluded) + snapCount, maxSnap, minSnap := (*dest).snapCount(nil) for old.Len() > 0 { node := old.Pop() - if node.generation == tp.generation && snapCount == 0 { + if snapCount == 0 || node.generation > maxSnap.generation { putTreapNode(node) + } else { + // Defer recycle until Release() on oldest snap (minSnap). + node.generation = recycleGeneration + node.next = minSnap.recycle + minSnap.recycle = node } } *dest = tp @@ -259,7 +265,7 @@ func PutM(dest **Immutable, key, value []byte, excluded *SnapRecord) { // del removes the passed key from the treap and returns the resulting treap // if it exists. The original immutable treap is returned if the key does not // exist. -func (t *Immutable) del(key []byte, bumpGen int) (d *Immutable, old parentStack) { +func (t *Immutable) del(key []byte) (d *Immutable, old parentStack) { // Find the node for the key while constructing a list of parents while // doing so. var oldParents parentStack @@ -293,7 +299,7 @@ func (t *Immutable) del(key []byte, bumpGen int) (d *Immutable, old parentStack) // being deleted, there is nothing else to do besides removing it. parent := oldParents.At(1) if parent == nil && delNode.left == nil && delNode.right == nil { - return newImmutable(nil, 0, 0, t.generation+bumpGen, t.snap), oldParents + return newImmutable(nil, 0, 0, t.generation+1, t.snap), oldParents } // Construct a replaced list of parents and the node to delete itself. @@ -374,20 +380,21 @@ func (t *Immutable) del(key []byte, bumpGen int) (d *Immutable, old parentStack) parent.left = nil } - return newImmutable(newRoot, t.count-1, t.totalSize-nodeSize(delNode), t.generation+bumpGen, t.snap), oldParents + return newImmutable(newRoot, t.count-1, t.totalSize-nodeSize(delNode), t.generation+1, t.snap), oldParents } -// Delete is the immutable variant of del. Generation number is bumped, and old -// nodes become garbage unless referenced elswhere. +// Delete is the immutable variant of del. Old nodes become garbage unless referenced elswhere. func (t *Immutable) Delete(key []byte) *Immutable { - tp, _ := t.del(key, 1) + tp, _ := t.del(key) return tp } -// DeleteM is the mutable variant of del. Generation number is NOT bumped, and old -// nodes are recycled if possible. This is only safe/useful in scenarios where -// multiple Put/Delete() ops are applied to a unique treap and no snapshots/aliases -// of the intermediate treap states are created or desired. For example: +// DeleteM is the mutable variant of del. Old nodes are recycled if possible. This is +// only safe in structured scenarios using SnapRecord to track treap instances. +// The outstanding SnapRecords serve to protect nodes from recycling when they might +// be present in one or more snapshots. This is useful in scenarios where multiple +// Put/Delete() ops are applied to a treap and intermediate treap states are not +// created or desired. For example: // // for i := range keys { // t = t.Delete(keys[i]) @@ -403,15 +410,20 @@ func (t *Immutable) Delete(key []byte) *Immutable { // snapshot records. // func DeleteM(dest **Immutable, key []byte, excluded *SnapRecord) { - tp, old := (*dest).del(key, 0) + tp, old := (*dest).del(key) // Examine old nodes and recycle if possible. snapRecordMutex.Lock() defer snapRecordMutex.Unlock() - snapCount := (*dest).snapCount(excluded) + snapCount, maxSnap, minSnap := (*dest).snapCount(nil) for old.Len() > 0 { node := old.Pop() - if node.generation == tp.generation && snapCount == 0 { + if snapCount == 0 || node.generation > maxSnap.generation { putTreapNode(node) + } else { + // Defer recycle until Release() on oldest snap (minSnap). + node.generation = recycleGeneration + node.next = minSnap.recycle + minSnap.recycle = node } } *dest = tp @@ -447,31 +459,45 @@ func NewImmutable() *Immutable { return &Immutable{} } -// SnapRecord assists in tracking/releasing outstanding snapshots. +// SnapRecord assists in tracking outstanding snapshots. While a SnapRecord +// is present and has not been Released(), treap nodes at or below this +// generation are protected from Recycle(). type SnapRecord struct { - prev *SnapRecord - next *SnapRecord + generation int + rp **SnapRecord + prev *SnapRecord + next *SnapRecord + recycle *treapNode } var snapRecordMutex sync.Mutex -// Snapshot makes a SnapRecord and linkis it into the snapshot history of a treap. +// Snapshot makes a SnapRecord and links it into the snapshot history of a treap. func (t *Immutable) Snapshot() *SnapRecord { snapRecordMutex.Lock() defer snapRecordMutex.Unlock() - // Link this record so it follows the existing t.snap record, if any. - prev := t.snap + rp := t.snap var next *SnapRecord = nil - if prev != nil { - next = prev.next + var prev *SnapRecord = nil + if rp != nil { + prev = *rp + if *rp != nil { + next = (*rp).next + } } - t.snap = &SnapRecord{prev: prev, next: next} - if prev != nil { - prev.next = t.snap + + // Create a new record stamped with the current generation. Link it + // following the existing snapshot record, if any. + p := new(*SnapRecord) + *p = &SnapRecord{generation: t.generation, rp: p, prev: prev, next: next} + t.snap = p + + if rp != nil && *rp != nil { + (*rp).next = *(t.snap) } - return t.snap + return *(t.snap) } // Release of SnapRecord unlinks that record from the snapshot history of a treap. @@ -480,45 +506,73 @@ func (r *SnapRecord) Release() { defer snapRecordMutex.Unlock() // Unlink this record. + *(r.rp) = nil + if r.next != nil { + r.next.prev = r.prev + *(r.rp) = r.next + } if r.prev != nil { r.prev.next = r.next + *(r.rp) = r.prev } - if r.next != nil { - r.next.prev = r.prev + + // Handle deferred recycle list. + for node := r.recycle; node != nil; { + next := node.next + putTreapNode(node) + node = next } } // snapCount returns the number of snapshots outstanding which were created // but not released. When snapshots are absent, mutable PutM()/DeleteM() can -// recycle nodes more aggressively. The record "exclude" is not counted. -func (t *Immutable) snapCount(exclude *SnapRecord) int { +// recycle nodes more aggressively. The record "excluded" is not counted. +func (t *Immutable) snapCount(excluded *SnapRecord) (count int, maxSnap, minSnap *SnapRecord) { // snapRecordMutex should be locked already - sum := 0 - if t.snap == nil { + count, maxSnap, minSnap = 0, nil, nil + if t.snap == nil || *(t.snap) == nil { // No snapshots. - return sum + return count, maxSnap, minSnap } // Count snapshots taken BEFORE creation of this instance. - for h := t.snap; h != nil; h = h.prev { - if h != exclude { - sum++ + for h := *(t.snap); h != nil; h = h.prev { + if h != excluded { + count++ + if maxSnap == nil || maxSnap.generation < h.generation { + maxSnap = h + } + if minSnap == nil || minSnap.generation > h.generation { + minSnap = h + } } } // Count snapshots taken AFTER creation of this instance. - for h := t.snap.next; h != nil; h = h.next { - if h != exclude { - sum++ + for h := (*(t.snap)).next; h != nil; h = h.next { + if h != excluded { + count++ + if maxSnap == nil || maxSnap.generation < h.generation { + maxSnap = h + } + if minSnap == nil || minSnap.generation > h.generation { + minSnap = h + } } } - return sum + return count, maxSnap, minSnap } -func (t *Immutable) Recycle() { - snapCount := t.snapCount(nil) - 1 +func (t *Immutable) Recycle(excluded *SnapRecord) { + snapRecordMutex.Lock() + _, maxSnap, _ := t.snapCount(excluded) + snapGen := 0 + if maxSnap != nil { + snapGen = maxSnap.generation + } + snapRecordMutex.Unlock() var parents parentStack for node := t.root; node != nil; node = node.left { @@ -534,7 +588,10 @@ func (t *Immutable) Recycle() { parents.Push(n) } - if node.generation == t.generation && snapCount == 0 { + // Recycle node if it cannot be in a snapshot. Note that nodes + // scheduled for deferred recycling will have negative generation + // (recycleGeneration) and will not qualify. + if node.generation > snapGen { putTreapNode(node) } } diff --git a/database/internal/treap/mutable.go b/database/internal/treap/mutable.go index 1ded6808e4..3289d31865 100644 --- a/database/internal/treap/mutable.go +++ b/database/internal/treap/mutable.go @@ -21,6 +21,10 @@ type Mutable struct { // totalSize is the best estimate of the total size of of all data in // the treap including the keys, values, and node sizes. totalSize uint64 + + // generation number is the constant mutableGeneration, unless + // creation of a treap.Iterator bumps it. + generation int } // Len returns the number of items stored in the treap. @@ -113,7 +117,7 @@ func (t *Mutable) Put(key, value []byte) { // The node is the root of the tree if there isn't already one. if t.root == nil { - node := getTreapNode(key, value, rand.Int(), MutableGeneration) + node := getTreapNode(key, value, rand.Int(), t.generation) t.count = 1 t.totalSize = nodeSize(node) t.root = node @@ -145,7 +149,7 @@ func (t *Mutable) Put(key, value []byte) { } // Link the new node into the binary tree in the correct position. - node := getTreapNode(key, value, rand.Int(), MutableGeneration) + node := getTreapNode(key, value, rand.Int(), t.generation) t.count++ t.totalSize += nodeSize(node) parent := parents.At(0) @@ -190,7 +194,9 @@ func (t *Mutable) Delete(key []byte) { t.root = nil t.count = 0 t.totalSize = 0 - putTreapNode(node) + if node.generation == t.generation && node.generation == mutableGeneration { + putTreapNode(node) + } return } @@ -239,7 +245,9 @@ func (t *Mutable) Delete(key []byte) { } t.count-- t.totalSize -= nodeSize(node) - putTreapNode(node) + if node.generation == t.generation && node.generation == mutableGeneration { + putTreapNode(node) + } } // ForEach invokes the passed function with every key/value pair in the treap @@ -276,7 +284,7 @@ func (t *Mutable) Reset() { // NewMutable returns a new empty mutable treap ready for use. See the // documentation for the Mutable structure for more details. func NewMutable() *Mutable { - return &Mutable{} + return &Mutable{generation: mutableGeneration} } func (t *Mutable) Recycle() { @@ -294,7 +302,7 @@ func (t *Mutable) Recycle() { parents.Push(n) } - if node.generation == MutableGeneration { + if node.generation == t.generation && node.generation == mutableGeneration { putTreapNode(node) } } diff --git a/database/internal/treap/treapiter.go b/database/internal/treap/treapiter.go index d6981aafd8..81028ed7fa 100644 --- a/database/internal/treap/treapiter.go +++ b/database/internal/treap/treapiter.go @@ -326,6 +326,7 @@ func (iter *Iterator) ForceReseek() { // } // } func (t *Mutable) Iterator(startKey, limitKey []byte) *Iterator { + t.generation++ iter := &Iterator{ t: t, root: t.root,