-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve Trie Node Retrieve and Save #1569
Conversation
@@ -55,9 +60,17 @@ public TrieStoreImpl(KeyValueDataSource store) { | |||
*/ | |||
@Override | |||
public void save(Trie trie) { | |||
noRetrieves = 0; | |||
noSaves = 0; | |||
noNoSaves = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the no
prefix confusing, noNoSaves
sounds like a double negation. Consider changing for numberNoSaves
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefixed those fields with numOf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Kudos, SonarCloud Quality Gate passed! |
pipeline: run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
It removes a trie node set, add an internal boolean flag to each trie node to knows if the node was already saved, improve the retrieval of the node (no get hash calculation is needed for adding the retrieved node to the removed set), improve save trie without visiting unloaded child nodes
Originated from #1526
Motivation and Context
It solves #1509 and #1510
How Has This Been Tested?
Additionally to the usual code tests, the execution of blocks from a testnet node was executed:
java -cp rskj-core-3.0.0-triefix-all.jar -Dlogback.configurationFile= -Dblockchain.flushNumberOfBlocks=1000 -Dcache.states.max-elements=1000000 co.rsk.cli.tools.ExecuteBlocks 1672391 1673139 --testnet
There are new logs, to obtain some additional information from TrieStoreImpl and DataSourceWithCache. The above command was executed with
Only for testing purpose.
Some additional logs obtained:
2021-05-07-16:52:33.317 TRACE [triestore] Start saving trie root.
2021-05-07-16:52:33.318 TRACE [triestore] Start saving trie, level : 0
2021-05-07-16:52:57.923 TRACE [triestore] End saving trie, level : 0, already saved.
2021-05-07-16:52:57.924 TRACE [triestore] End saving trie root. No. Retrieves: 396. No. Saves: 0. No. No Saves: 1
2021-05-07-16:52:57.924 TRACE [triestore] End process block. No. Retrieves: 968. No. Saves: 0. No. No Saves: 1
2021-05-07-16:52:57.925 TRACE [datasourcewithcache] Activity: No. Gets: 969. No. Puts: 0. No. Gets from Store: 967
With this additional logs we have more information about the effectivity of having a cache (and the Trie getHash issue #1511 was discovered using these logs)
Types of changes
Checklist: