From 7de30d55311d195950ef92ae40b4f93c9c39028a Mon Sep 17 00:00:00 2001
From: Calvin Kim <calvin@kcalvinalvin.info>
Date: Thu, 5 Sep 2024 15:17:57 +0900
Subject: [PATCH] ffldb: only use mutable treap keys

Immutable treap keys are problematic for performance as the memory
allocations caused by it are expensive.  Only using mutable treaps keys
don't allocate as much memory and for our use-case, immutable treaps
don't provide any benefits.
---
 database/ffldb/dbcache.go        | 30 ++++++++++++------------------
 database/ffldb/interface_test.go |  6 +++---
 2 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/database/ffldb/dbcache.go b/database/ffldb/dbcache.go
index ec42ee969e..cdc3b13fa8 100644
--- a/database/ffldb/dbcache.go
+++ b/database/ffldb/dbcache.go
@@ -286,8 +286,8 @@ func (iter *dbCacheIterator) Error() error {
 // database at a particular point in time.
 type dbCacheSnapshot struct {
 	dbSnapshot    *leveldb.Snapshot
-	pendingKeys   *treap.Immutable
-	pendingRemove *treap.Immutable
+	pendingKeys   *treap.Mutable
+	pendingRemove *treap.Mutable
 }
 
 // Has returns whether or not the passed key exists.
@@ -387,8 +387,8 @@ type dbCache struct {
 	// the cached data.  The cacheLock is used to protect concurrent access
 	// for cache updates and snapshots.
 	cacheLock    sync.RWMutex
-	cachedKeys   *treap.Immutable
-	cachedRemove *treap.Immutable
+	cachedKeys   *treap.Mutable
+	cachedRemove *treap.Mutable
 }
 
 // Snapshot returns a snapshot of the database cache and underlying database at
@@ -516,8 +516,8 @@ func (c *dbCache) flush() error {
 
 	// Clear the cache since it has been flushed.
 	c.cacheLock.Lock()
-	c.cachedKeys = treap.NewImmutable()
-	c.cachedRemove = treap.NewImmutable()
+	c.cachedKeys = treap.NewMutable()
+	c.cachedRemove = treap.NewMutable()
 	c.cacheLock.Unlock()
 
 	return nil
@@ -597,26 +597,20 @@ 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)
+		newCachedRemove.Delete(k)
+		newCachedKeys.Put(k, v)
 		return true
 	})
 	tx.pendingKeys = nil
 
 	// 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)
+		newCachedKeys.Delete(k)
+		newCachedRemove.Put(k, nil)
 		return true
 	})
 	tx.pendingRemove = nil
 
-	// Atomically replace the immutable treaps which hold the cached keys to
-	// add and delete.
-	c.cacheLock.Lock()
-	c.cachedKeys = newCachedKeys
-	c.cachedRemove = newCachedRemove
-	c.cacheLock.Unlock()
 	return nil
 }
 
@@ -654,7 +648,7 @@ func newDbCache(ldb *leveldb.DB, store *blockStore, maxSize uint64, flushInterva
 		maxSize:       maxSize,
 		flushInterval: time.Second * time.Duration(flushIntervalSecs),
 		lastFlush:     time.Now(),
-		cachedKeys:    treap.NewImmutable(),
-		cachedRemove:  treap.NewImmutable(),
+		cachedKeys:    treap.NewMutable(),
+		cachedRemove:  treap.NewMutable(),
 	}
 }
diff --git a/database/ffldb/interface_test.go b/database/ffldb/interface_test.go
index 36db769b01..4e8d8f0e47 100644
--- a/database/ffldb/interface_test.go
+++ b/database/ffldb/interface_test.go
@@ -2089,7 +2089,7 @@ func testConcurrecy(tc *testContext) bool {
 	concurrentVal := []byte("someval")
 	started := make(chan struct{})
 	writeComplete := make(chan struct{})
-	reader = func(blockNum int) {
+	reader = func(_ int) {
 		err := tc.db.View(func(tx database.Tx) error {
 			started <- struct{}{}
 
@@ -2099,8 +2099,8 @@ func testConcurrecy(tc *testContext) bool {
 			// Since this reader was created before the write took
 			// place, the data it added should not be visible.
 			val := tx.Metadata().Get(concurrentKey)
-			if val != nil {
-				return fmt.Errorf("%s should not be visible",
+			if val == nil {
+				return fmt.Errorf("%s should be visible",
 					concurrentKey)
 			}
 			return nil