implement collapse manager by BeniaminDrasovean · Pull Request #7373 · multiversx/mx-chain-go

BeniaminDrasovean · 2025-10-27T12:33:24Z

Reasoning behind the pull request

Trie nodes are not collapsed in an efficient manner.

Proposed changes

Implement a collapseManager which handles trie nodes collapsing. If the size in memory of the trie exceed the max allowed size, collapse the main trie leaves in LRU order.
Collapse data tries entirely if the max data tries allowed size is exceeded. Data tries are also collapsed in a LRU order

Testing procedure

Will be tested on the feat branch.

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?

…aves # Conflicts: # common/interface.go # integrationTests/state/stateTrie/stateTrie_test.go # state/accountsDB_test.go # state/storagePruningManager/storagePruningManager_test.go # testscommon/components/components.go # trie/branchNode_test.go # trie/extensionNode_test.go # trie/interface.go # trie/node.go # trie/patriciaMerkleTrie.go

Copilot

Pull request overview

This PR introduces a trie “collapse manager” abstraction and wires it into Patricia Merkle Trie creation/commit paths, aiming to control in-memory trie growth by collapsing nodes (primarily leaves) based on memory usage and recent access patterns.

Changes:

Replaced trie.NewTrie(..., maxSizeInMemory uint64) with trie.NewTrie(..., collapseManager common.TrieCollapseManager) across the codebase.
Added trie/collapseManager package with enabled (NewCollapseManager) and disabled (NewDisabledCollapseManager) implementations.
Updated trie commit/collapse behavior and extended node interface (shouldCollapseChild) + adjusted tests accordingly.

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
update/genesis/import.go	Updates trie construction to pass a collapse manager (currently disabled) during import.
update/factory/dataTrieFactory.go	Updates data trie factory to pass a collapse manager (currently disabled).
trie/sync_test.go	Adjusts tests for `commitDirty` signature change.
trie/patriciaMerkleTrie_test.go	Updates tests for new collapse behavior and adds a collapse test.
trie/patriciaMerkleTrie.go	Integrates collapse manager into trie size tracking and commit flow.
trie/node_test.go	Updates tests for `commitDirty` signature change; removes `isLeafNode` test.
trie/leafNode_test.go	Updates tests for `commitDirty` signature change; adds `shouldCollapseChild` test.
trie/leafNode.go	Updates `commitDirty` signature and adds `shouldCollapseChild`.
trie/interface.go	Adds `shouldCollapseChild` and updates `commitDirty` signature in the `node` interface.
trie/factory/trieCreator.go	Creates a real collapse manager from `MaxSizeInMemory` and passes it to `NewTrie`.
trie/extensionNode_test.go	Updates tests for `commitDirty` signature change; adds `shouldCollapseChild` tests.
trie/extensionNode.go	Updates `commitDirty` signature and adds `shouldCollapseChild`.
trie/export_test.go	Adds a test-only helper to count collapsed nodes.
trie/errors.go	Adds `ErrNilCollapseManager`.
trie/doubleListSync_test.go	Updates trie creation to pass a collapse manager (disabled).
trie/collapseManager/disabledCollapseManager_test.go	Adds tests for the disabled collapse manager implementation.
trie/collapseManager/disabledCollapseManager.go	Implements a no-op collapse manager.
trie/collapseManager/collapseManager_test.go	Adds tests for the enabled collapse manager implementation.
trie/collapseManager/collapseManager.go	Implements enabled collapse manager logic (LRU-like).
trie/branchNode_test.go	Updates tests for `commitDirty` signature change; adds `shouldCollapseChild` tests.
trie/branchNode.go	Updates `commitDirty` signature and removes unconditional leaf collapse; adds `shouldCollapseChild`.
testscommon/state/testTrie.go	Updates helper to create tries with a collapse manager (disabled).
testscommon/integrationtests/factory.go	Updates trie creation to pass a collapse manager (disabled).
testscommon/components/components.go	Updates component wiring to pass a collapse manager (disabled).
state/syncer/userAccountsSyncer_test.go	Updates test helpers and trie creation to pass a collapse manager (disabled).
state/syncer/userAccountSyncer_test.go	Updates test trie creation to pass a collapse manager (disabled).
state/syncer/baseAccountsSyncer.go	Updates trie creation to pass a collapse manager (disabled) in deprecated path.
state/storagePruningManager/storagePruningManager_test.go	Updates test trie creation to pass a collapse manager (disabled).
state/accountsDB_test.go	Updates test trie creation to pass a collapse manager (disabled).
state/accountsDB.go	Clarifies commit step comment for data tries.
integrationTests/vm/staking/componentsHolderCreator.go	Updates trie creation to pass a collapse manager (disabled).
integrationTests/testInitializer.go	Updates trie creation to pass a collapse manager (disabled).
integrationTests/state/stateTrieClose/stateTrieClose_test.go	Updates test trie creation to pass a collapse manager (disabled).
integrationTests/state/stateTrie/stateTrie_test.go	Updates test trie creation to pass a collapse manager (disabled).
integrationTests/longTests/storage/storage_test.go	Updates test trie creation to pass a collapse manager (disabled).
integrationTests/benchmarks/loadFromTrie_test.go	Updates benchmark trie creation to pass a collapse manager (disabled).
genesis/process/memoryComponents.go	Updates genesis in-memory trie creation to pass a collapse manager (disabled).
factory/processing/blockProcessorCreator_test.go	Updates test trie creation to pass a collapse manager (disabled).
epochStart/metachain/systemSCs_test.go	Updates test trie creation to pass a collapse manager (disabled).
common/interface.go	Introduces `common.TrieCollapseManager` interface.

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

Copilot

Pull request overview

Copilot reviewed 40 out of 40 changed files in this pull request and generated 5 comments.

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

…' into collapse-main-trie-leaves # Conflicts: # genesis/process/memoryComponents.go # state/accountsDB_test.go

codecov · 2026-03-17T10:35:58Z

Codecov Report

❌ Patch coverage is 84.73684% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.54%. Comparing base (a1758e0) to head (4054754).
⚠️ Report is 13 commits behind head on feat/collapse-trie-based-on-size.

Files with missing lines	Patch %	Lines
trie/patriciaMerkleTrie.go	57.50%	13 Missing and 4 partials ⚠️
trie/collapseManager/collapseManager.go	94.02%	2 Missing and 2 partials ⚠️
update/genesis/import.go	0.00%	4 Missing ⚠️
trie/factory/trieCreator.go	90.90%	1 Missing and 1 partial ⚠️
state/syncer/baseAccountsSyncer.go	0.00%	1 Missing ⚠️
update/factory/dataTrieFactory.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@                         Coverage Diff                          @@
##           feat/collapse-trie-based-on-size    #7373      +/-   ##
====================================================================
- Coverage                             77.55%   77.54%   -0.01%     
====================================================================
  Files                                   884      885       +1     
  Lines                                123834   123844      +10     
====================================================================
- Hits                                  96039    96035       -4     
- Misses                                21442    21450       +8     
- Partials                               6353     6359       +6

☔ 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-17T13:21:48Z

+	// TODO calibrate these values
+	numLeavesToCollapseSingleRun = 100
+	minNumLeavesToCollapseTrie   = 1000
+	minSizeInMemory              = 1048576 // 1 MB


maybe these values should be moved in the config.toml file ?

Moved only numLeavesToCollapseSingleRun in config, the other const are only checks that the existing config vals are ok.

miiu96 · 2026-03-17T13:34:44Z

+			for i := range current.children {
+				if current.children[i] != nil {
+					nextNodes = append(nextNodes, current.children[i])
+				}
+				if current.children[i] == nil && len(current.EncodedChildren[i]) != 0 {
+					count++
+				}
+			}


Suggested change

for i := range current.children {

if current.children[i] != nil {

nextNodes = append(nextNodes, current.children[i])

}

if current.children[i] == nil && len(current.EncodedChildren[i]) != 0 {

count++

}

}

for i := range current.children {

if current.children[i] != nil {

nextNodes = append(nextNodes, current.children[i])

} else if len(current.EncodedChildren[i]) != 0 {

count++

}

}

ssd04 · 2026-03-25T11:05:43Z

+	if cm.sizeInMemory+size < 0 {
+		log.Warn("trie size in memory is negative after adding size, resetting to 0", "size", size, "currentSize", cm.sizeInMemory)
+		cm.sizeInMemory = 0
+		return
+	}
+	cm.sizeInMemory += size


do we need mutex protection for sizeInMemory ?

and for accessedKeys?

Not right now. This component is used only by the trie, and is protected by the trie mutex.

ssd04 · 2026-03-25T11:10:56Z

+// GetCollapsibleLeaves returns a list of keys that can be collapsed to free memory
+func (cm *collapseManager) GetCollapsibleLeaves() ([][]byte, error) {
+	if uint64(cm.sizeInMemory) < cm.maxSizeInMem {
+		return nil, nil


Suggested change

return nil, nil

return make([][]byte, 0), nil

*optional: to avoid nil, nil confusion

ssd04 · 2026-03-25T11:19:17Z

+func (cm *collapseManager) ShouldCollapseTrie() bool {
+	// we collapse only if we are over the memory limit and there are not enough accessed keys to
+	// free memory by collapsing only leaves
+	if uint64(cm.sizeInMemory) > cm.maxSizeInMem && len(cm.accessedKeys) < minNumLeavesToCollapseTrie {


why second condition? maybe rename const minNumLeavesToCollapseTrie?

add a unit test case also for this?

There is a unit test for this: TestCollapseManager_ShouldCollapseTrie.
The second condition is ment for when the trie size is composed of mostly intermediary nodes and there are only a few leaves. Only leaves are collapsed so if we have only a few leaves left to collapse, the collapse will be triggered every commit. If we reach this state of a trie, we should recreate the trie. This will also collapse the intermediary nodes that were not used in a long time.

ssd04 · 2026-03-25T12:24:21Z

+	// an extension node can not have a leaf as child, so no need to check for that
+	_ = en.child.shouldCollapseChild(hexKey, tmc)
+	return false
+}


so this func will always return false?

Yes. Because we only collapse leaves and the child of an extension node can not be a leaf node, we never collapse the child of an extension node. I returned false instead of returning en.child.shouldCollapseChild(hexKey, tmc) directly just for this to be more obvious.

ssd04 · 2026-03-25T12:26:14Z

+	return nil
+}
+
+func (bn *branchNode) shouldCollapseChild(hexKey []byte, tmc MetricsCollector) bool {


intended to return false on all paths?

Yes. We only collapse leaf nodes, so only a leaf node can return true.

BeniaminDrasovean added 8 commits October 27, 2025 14:32

implement collapse manager

151d3d4

add unit tests and comments for the collapse manager implementations

04762c5

fix failing tests

8569c67

add unit tests and use ctx for collapsing

1987ca5

serialize collapsing

1bb6309

comments update

040f204

fix after merge

1b82639

BeniaminDrasovean requested a review from Copilot March 4, 2026 08:51

Copilot started reviewing on behalf of BeniaminDrasovean March 4, 2026 08:52 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

remove unused variable

ba75b3f

BeniaminDrasovean requested a review from Copilot March 5, 2026 10:50

Copilot started reviewing on behalf of BeniaminDrasovean March 5, 2026 10:51 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

Comment thread trie/patriciaMerkleTrie_test.go

Comment thread trie/collapseManager/collapseManager_test.go Outdated

Comment thread update/factory/dataTrieFactory.go

Comment thread update/genesis/import.go

Comment thread state/syncer/baseAccountsSyncer.go

Base automatically changed from collapse-trie-based-on-size to feat/collapse-trie-based-on-size March 16, 2026 14:30

Merge remote-tracking branch 'origin/feat/collapse-trie-based-on-size…

f48d88f

…' into collapse-main-trie-leaves # Conflicts: # genesis/process/memoryComponents.go # state/accountsDB_test.go

BeniaminDrasovean marked this pull request as ready for review March 17, 2026 10:21

miiu96 reviewed Mar 17, 2026

View reviewed changes

fix after review

3d38337

ssd04 reviewed Mar 25, 2026

View reviewed changes

fix after review

4054754

miiu96 approved these changes Apr 6, 2026

View reviewed changes

ssd04 approved these changes Apr 6, 2026

View reviewed changes

BeniaminDrasovean merged commit 05f59ea into feat/collapse-trie-based-on-size Apr 6, 2026
11 checks passed

BeniaminDrasovean deleted the collapse-main-trie-leaves branch April 6, 2026 10:32

Uh oh!

Conversation

BeniaminDrasovean commented Oct 27, 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!

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!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Mar 17, 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!

BeniaminDrasovean Mar 30, 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

BeniaminDrasovean commented Oct 27, 2025 •

edited

Loading

codecov Bot commented Mar 17, 2026 •

edited

Loading

BeniaminDrasovean Mar 30, 2026 •

edited

Loading