From 95916e67ea1b252199f60e4d2881be378f208e76 Mon Sep 17 00:00:00 2001 From: VolodymyrBg Date: Fri, 21 Feb 2025 23:05:49 +0200 Subject: [PATCH 1/3] kvcache: optimize View() method and fix redundant atomic store The commit improves the View() method in the kvcache package by: 1. Removing redundant atomic store operation in OnNewBlock() after CompareAndSwap 2. Adding proper lock protection when accessing shared state in View() 3. Using time.NewTimer instead of time.After to prevent potential goroutine leaks 4. Optimizing ready state check flow to reduce lock contention The changes make the code more efficient while maintaining thread safety. --- erigon-lib/kv/kvcache/cache.go | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/erigon-lib/kv/kvcache/cache.go b/erigon-lib/kv/kvcache/cache.go index 4f5ab728dd3..77517cd8360 100644 --- a/erigon-lib/kv/kvcache/cache.go +++ b/erigon-lib/kv/kvcache/cache.go @@ -334,40 +334,37 @@ func (c *Coherent) OnNewBlock(stateChanges *remote.StateChangeBatch) { } func (c *Coherent) View(ctx context.Context, tx kv.Tx) (CacheView, error) { - idBytes, err := tx.GetOne(kv.Sequence, kv.PlainStateVersion) + id, err := tx.ReadSequence(string(kv.PlainStateVersion)) if err != nil { return nil, err } - var id uint64 - if len(idBytes) == 0 { - id = 0 - } else { - id = binary.BigEndian.Uint64(idBytes) - } + + c.lock.Lock() r := c.selectOrCreateRoot(id) + isReady := r.readyChanClosed.Load() + c.lock.Unlock() if !c.cfg.WaitForNewBlock || c.waitExceededCount.Load() >= MAX_WAITS { return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil } - select { // fast non-blocking path - case <-r.ready: - //fmt.Printf("recv broadcast: %d\n", id) + if isReady { return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil - default: } - select { // slow blocking path + timer := time.NewTimer(c.cfg.NewBlockWait) + defer timer.Stop() + + select { case <-r.ready: - //fmt.Printf("recv broadcast2: %d\n", tx.ViewID()) + return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil case <-ctx.Done(): return nil, fmt.Errorf("kvcache rootNum=%x, %w", tx.ViewID(), ctx.Err()) - case <-time.After(c.cfg.NewBlockWait): //TODO: switch to timer to save resources + case <-timer.C: c.timeout.Inc() c.waitExceededCount.Add(1) - //log.Info("timeout", "db_id", id, "has_btree", r.cache != nil) + return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil } - return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil } func (c *Coherent) getFromCache(k []byte, id uint64, code bool) (*Element, *CoherentRoot, error) { @@ -524,11 +521,11 @@ func (c *Coherent) ValidateCurrentRoot(ctx context.Context, tx kv.Tx) (*CacheVal default: } - idBytes, err := tx.GetOne(kv.Sequence, kv.PlainStateVersion) + stateID, err := tx.ReadSequence(string(kv.PlainStateVersion)) if err != nil { return nil, err } - stateID := binary.BigEndian.Uint64(idBytes) + result.LatestStateID = stateID // if the latest view id in the cache is not the same as the tx or one below it From 1367bbc519efdb3bb633b3ec08d8a6f86acd11fb Mon Sep 17 00:00:00 2001 From: VolodymyrBg Date: Sat, 22 Feb 2025 20:14:01 +0200 Subject: [PATCH 2/3] Update cache.go --- erigon-lib/kv/kvcache/cache.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/erigon-lib/kv/kvcache/cache.go b/erigon-lib/kv/kvcache/cache.go index 77517cd8360..61ca58c291f 100644 --- a/erigon-lib/kv/kvcache/cache.go +++ b/erigon-lib/kv/kvcache/cache.go @@ -352,15 +352,12 @@ func (c *Coherent) View(ctx context.Context, tx kv.Tx) (CacheView, error) { return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil } - timer := time.NewTimer(c.cfg.NewBlockWait) - defer timer.Stop() - select { case <-r.ready: return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil case <-ctx.Done(): return nil, fmt.Errorf("kvcache rootNum=%x, %w", tx.ViewID(), ctx.Err()) - case <-timer.C: + case <-time.After(c.cfg.NewBlockWait): c.timeout.Inc() c.waitExceededCount.Add(1) return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil From c27f93725ec09c1d7fde1a8533e74ff6501316ce Mon Sep 17 00:00:00 2001 From: VolodymyrBg Date: Sat, 22 Feb 2025 20:15:15 +0200 Subject: [PATCH 3/3] Update cache.go --- erigon-lib/kv/kvcache/cache.go | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/erigon-lib/kv/kvcache/cache.go b/erigon-lib/kv/kvcache/cache.go index 61ca58c291f..10faaf368ab 100644 --- a/erigon-lib/kv/kvcache/cache.go +++ b/erigon-lib/kv/kvcache/cache.go @@ -122,15 +122,10 @@ type Coherent struct { } type CoherentRoot struct { - cache *btree2.BTreeG[*Element] - codeCache *btree2.BTreeG[*Element] - ready chan struct{} // close when ready - readyChanClosed atomic.Bool // protecting `ready` field from double-close (on unwind). Consumers don't need check this field. - - // Views marked as `Canonical` if it received onNewBlock message - // we may drop `Non-Canonical` views even if they had fresh keys - // keys added to `Non-Canonical` views SHOULD NOT be added to stateEvict - // cache.latestStateView is always `Canonical` + cache *btree2.BTreeG[*Element] + codeCache *btree2.BTreeG[*Element] + ready chan struct{} // close when ready + closeOnce sync.Once // protecting `ready` field from double-close isCanonical bool } @@ -326,10 +321,9 @@ func (c *Coherent) OnNewBlock(stateChanges *remote.StateChangeBatch) { } } - switched := r.readyChanClosed.CompareAndSwap(false, true) - if switched { + r.closeOnce.Do(func() { close(r.ready) //broadcast - } + }) //log.Info("on new block handled", "viewID", stateChanges.StateVersionID) } @@ -339,19 +333,12 @@ func (c *Coherent) View(ctx context.Context, tx kv.Tx) (CacheView, error) { return nil, err } - c.lock.Lock() r := c.selectOrCreateRoot(id) - isReady := r.readyChanClosed.Load() - c.lock.Unlock() if !c.cfg.WaitForNewBlock || c.waitExceededCount.Load() >= MAX_WAITS { return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil } - if isReady { - return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil - } - select { case <-r.ready: return &CoherentView{stateVersionID: id, tx: tx, cache: c}, nil