dataTriesHolder with eviction cache by BeniaminDrasovean · Pull Request #7191 · multiversx/mx-chain-go

BeniaminDrasovean · 2025-08-14T11:48:09Z

Reasoning behind the pull request

The current implementation of dataTriesHolder is a simple cache that holds data tries. The cache is cleaned after each commit operation

Proposed changes

Add a new implementation of dataTriesHolder as a linked list with memory management and eviction capabilities.
Add config values for the size of the cache and for the size of the main trie

Testing procedure

Normal system test

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

was the PR targeted to the correct branch?
if this is a larger feature that probably needs more than one PR, is there a feat branch created?
if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

Copilot

Pull Request Overview

This PR implements a dataTriesHolder as a linked list with memory management and eviction capabilities. The changes introduce proper memory limits for tries, replace the simple tries holder with a more sophisticated implementation, and update the trie creation API to include memory size parameters.

Implements a new linked-list-based dataTriesHolder with memory-aware eviction
Adds memory size parameters to trie creation throughout the codebase
Replaces the original simple tries holder with a more advanced memory-managed implementation

Reviewed Changes

Copilot reviewed 58 out of 58 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
state/triesHolder/dataTriesHolder.go	Implements new linked-list-based data tries holder with memory management and LRU eviction
state/triesHolder/triesHolder.go	Refactors original tries holder to be simpler without Replace method
trie/patriciaMerkleTrie.go	Adds maxSizeInMemory parameter to trie constructor and validation
state/accountsDB.go	Updates to use new data tries holder with memory limits
config/config.go	Adds configuration options for trie memory limits
Multiple test files	Updates test files to use new trie creation API with memory parameters

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

…sed-on-size # Conflicts: # dataRetriever/factory/resolverscontainer/metaResolversContainerFactory_test.go # dataRetriever/factory/resolverscontainer/shardResolversContainerFactory_test.go # epochStart/bootstrap/process.go # epochStart/metachain/systemSCs_test.go # factory/api/apiResolverFactory.go # factory/processing/blockProcessorCreator_test.go # factory/state/stateComponents.go # genesis/process/memoryComponents.go # integrationTests/state/stateTrie/stateTrie_test.go # integrationTests/testInitializer.go # integrationTests/testProcessorNode.go # integrationTests/vm/staking/componentsHolderCreator.go # state/accountsDB.go # state/accountsDB_test.go # state/factory/accountsAdapterAPICreator_test.go # state/storagePruningManager/storagePruningManager_test.go # testscommon/integrationtests/factory.go # trie/snapshotTrieStorageManager_test.go # trie/trieStorageManagerInEpoch_test.go # update/factory/dataTrieFactory.go # update/genesis/import.go

Copilot

Pull request overview

Copilot reviewed 59 out of 59 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

state/triesHolder/triesHolder.go:43

Get and GetAll acquire the write lock (Lock) even though they only read from the map. Since this uses an RWMutex, switching these to RLock/RUnlock would allow concurrent readers and improve contention under read-heavy access patterns.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-03-03T13:15:17Z

Codecov Report

❌ Patch coverage is 76.74419% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.53%. Comparing base (fe6d54f) to head (96ad288).
⚠️ Report is 4421 commits behind head on feat/collapse-trie-based-on-size.

Files with missing lines	Patch %	Lines
state/triesHolder/dataTriesHolder.go	80.53%	14 Missing and 8 partials ⚠️
update/genesis/import.go	0.00%	20 Missing ⚠️
state/accountsDB.go	71.42%	1 Missing and 1 partial ⚠️
state/peerAccountsDB.go	50.00%	1 Missing and 1 partial ⚠️
state/syncer/baseAccountsSyncer.go	0.00%	2 Missing ⚠️
update/factory/dataTrieFactory.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@                         Coverage Diff                          @@
##           feat/collapse-trie-based-on-size    #7191      +/-   ##
====================================================================
+ Coverage                             76.64%   77.53%   +0.89%     
====================================================================
  Files                                   823      884      +61     
  Lines                                110227   123727   +13500     
====================================================================
+ Hits                                  84484    95934   +11450     
- Misses                                20019    21441    +1422     
- Partials                               5724     6352     +628

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

miiu96 · 2026-03-10T11:14:08Z

 	storageFactory "github.com/multiversx/mx-chain-go/storage/factory"
 	"github.com/multiversx/mx-chain-go/storage/storageunit"
 	"github.com/multiversx/mx-chain-go/testscommon"
+	common2 "github.com/multiversx/mx-chain-go/testscommon/common"


maybe rename "common2" in something else

miiu96 · 2026-03-10T11:16:42Z

 	"github.com/multiversx/mx-chain-go/trie"
 )

+const tenMbSize = uint64(10485760)


maybe move this in the common package and use it everywhere

miiu96 · 2026-03-10T11:17:31Z

 	"github.com/multiversx/mx-chain-core-go/hashing/blake2b"
 	"github.com/multiversx/mx-chain-core-go/marshal"
 	"github.com/multiversx/mx-chain-go/integrationTests"
+	trie2 "github.com/multiversx/mx-chain-go/testscommon/common"


maybe rename "trie2" in something else

miiu96 · 2026-03-10T11:17:40Z

 	"github.com/multiversx/mx-chain-core-go/hashing/sha256"
 	crypto "github.com/multiversx/mx-chain-crypto-go"
 	"github.com/multiversx/mx-chain-go/epochStart/notifier"
+	common2 "github.com/multiversx/mx-chain-go/testscommon/common"


miiu96 · 2026-03-10T11:17:57Z

 	"github.com/multiversx/mx-chain-go/common/errChan"
 	"github.com/multiversx/mx-chain-go/integrationTests"
 	"github.com/multiversx/mx-chain-go/state/parsers"
+	trie2 "github.com/multiversx/mx-chain-go/testscommon/common"


miiu96 · 2026-03-10T11:21:31Z

+	return tries
+}
+
+func (dth *dataTriesHolder) getDirtyTrie(key string) common.Trie {


maybe rename in getDirtyTrieNoLock ( similar with putNoLock )

miiu96 · 2026-03-10T11:22:59Z

 	"github.com/multiversx/mx-chain-go/state/storagePruningManager/disabled"
 	"github.com/multiversx/mx-chain-go/state/storagePruningManager/evictionWaitingList"
 	"github.com/multiversx/mx-chain-go/testscommon"
+	common2 "github.com/multiversx/mx-chain-go/testscommon/common"


miiu96 · 2026-03-10T11:23:25Z

 	"github.com/multiversx/mx-chain-go/storage/factory"
 	"github.com/multiversx/mx-chain-go/storage/storageunit"
 	"github.com/multiversx/mx-chain-go/testscommon"
+	common2 "github.com/multiversx/mx-chain-go/testscommon/common"


miiu96 · 2026-03-10T11:24:28Z


 var emptyTrieHash = make([]byte, 32)

+const tenMBSize = uint64(10485760)


this const is defined in a lot of places

It is refactored in the next PR.

miiu96 · 2026-03-10T11:24:52Z


 	"github.com/multiversx/mx-chain-go/storage"
 	"github.com/multiversx/mx-chain-go/testscommon"
+	storage2 "github.com/multiversx/mx-chain-go/testscommon/storage"


ssd04 · 2026-03-10T13:56:22Z

+	tenMBSize := uint64(10485760)
+	dataTrie, err := trie.NewTrie(b.trieStorageManager, b.marshalizer, b.hasher, b.enableEpochsHandler, tenMBSize)


is this func still used? is it ok to set static size limit?

It is not used anywhere.

ssd04 · 2026-03-10T13:59:22Z

+	for i := 0; i < numTries; i++ {
+		go func(key int) {
+			dth.Put([]byte(strconv.Itoa(key)), &trieMock.TrieStub{})
+			wg.Done()
+		}(i)
+	}


trigger all methods here?

ssd04 · 2026-03-10T14:03:06Z

 		Identifier:          dataRetriever.UserAccountsUnit.String(),
 		EnableEpochsHandler: &enableEpochsHandlerMock.EnableEpochsHandlerStub{},
 		StatsCollector:      disabled.NewStateStatistics(),
+		MaxSizeInMemory:     10 * 1024 * 1024, // 10 MB


use tenMBSize const here also

ssd04 · 2026-03-10T14:06:19Z

 	oldRoot      []byte
 	chanClose    chan struct{}
 	sizeInMemory int
+	maxSizeInMem uint64


max size in mem will be used later on in another PR?

Yes. It will be moved to another component and used there.

ssd04 · 2026-03-16T13:41:14Z

+	if len(dth.evictedBuffer) > 0 {
+		// this means that this trie was evicted while being dirty
+		delete(dth.evictedBuffer, keyString)
+	}


why try to delete current key if there are entries on evictedBuffer? just to make sure it's removed from evicted in case it might be there?

If a dirty trie is evicted from the cache (this is an edgecase, if the cache is large enough it should never happen), we still need to commit it, so we keep it in dth.evictedBuffer until the commit is triggered. But if that trie is accessed again after it was evicted from the cache but before it was committed, we re-add it to the cache, so we can remove it from the dth.evictedBuffer.

BeniaminDrasovean added 5 commits August 14, 2025 14:47

dataTriesHolder as linked list

65c8aa5

add eviction and unit tests

b78c176

keep totalSizeInMemory updated between commits

621f6f9

add size limits to config

c55760b

create dataTriesHolder with size read from config

d4eb6a2

BeniaminDrasovean requested a review from Copilot October 8, 2025 12:12

Copilot AI reviewed Oct 8, 2025

View reviewed changes

Comment thread state/triesHolder/dataTriesHolder.go

Comment thread state/accountsDB.go

Comment thread testscommon/storage/snapshotPruningStorerStub.go

BeniaminDrasovean marked this pull request as ready for review October 8, 2025 12:16

BeniaminDrasovean added 3 commits October 14, 2025 11:41

use existing cacher instead of linked list

a1a9635

fix cyclic imports

5bfd75f

add nil check for inserted data

dc8dc6e

BeniaminDrasovean changed the title ~~dataTriesHolder as linked list~~ dataTriesHolder with eviction cache Oct 21, 2025

Base automatically changed from collect-trie-size-in-mem to feat/collapse-trie-based-on-size March 2, 2026 14:38

BeniaminDrasovean added 2 commits March 3, 2026 14:02

fix after merge

19f9527

BeniaminDrasovean requested a review from Copilot March 3, 2026 13:01

Copilot started reviewing on behalf of BeniaminDrasovean March 3, 2026 13:01 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

Comment thread epochStart/bootstrap/process.go Outdated

Comment thread testscommon/components/components.go Outdated

BeniaminDrasovean added 2 commits March 3, 2026 15:20

fix linter issues

ad74409

fix typo in test

17870e2

miiu96 reviewed Mar 10, 2026

View reviewed changes

ssd04 reviewed Mar 11, 2026

View reviewed changes

fix after review

96ad288

miiu96 approved these changes Mar 16, 2026

View reviewed changes

ssd04 reviewed Mar 16, 2026

View reviewed changes

ssd04 approved these changes Mar 16, 2026

View reviewed changes

BeniaminDrasovean merged commit a1758e0 into feat/collapse-trie-based-on-size Mar 16, 2026
11 checks passed

BeniaminDrasovean deleted the collapse-trie-based-on-size branch March 16, 2026 14:30


		var emptyTrieHash = make([]byte, 32)

		const tenMBSize = uint64(10485760)

		tenMBSize := uint64(10485760)
		dataTrie, err := trie.NewTrie(b.trieStorageManager, b.marshalizer, b.hasher, b.enableEpochsHandler, tenMBSize)

Conversation

BeniaminDrasovean commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reasoning behind the pull request

Proposed changes

Testing procedure

Pre-requisites

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BeniaminDrasovean Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BeniaminDrasovean commented Aug 14, 2025 •

edited

Loading

codecov Bot commented Mar 3, 2026 •

edited

Loading

BeniaminDrasovean Mar 12, 2026 •

edited

Loading