-
Notifications
You must be signed in to change notification settings - Fork 198
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
Trie sync optimization #2888
Trie sync optimization #2888
Conversation
iulianpascalau
commented
Mar 4, 2021
•
edited
edited
- created a new optimized trie syncer
- added the possibility to switch from the 2 implementations from the config lines
- made the trie sync integration tests use real file-based storers
- added the possibility to switch from the 2 implementations from the config lines
…e-sync-optimization
Codecov Report
@@ Coverage Diff @@
## feat/trie-sync-fixes #2888 +/- ##
========================================================
+ Coverage 74.91% 74.97% +0.06%
========================================================
Files 613 615 +2
Lines 58645 58857 +212
========================================================
+ Hits 43933 44130 +197
- Misses 10783 10789 +6
- Partials 3929 3938 +9
Continue to review full report at Codecov.
|
- made the trie sync tests use real life storers
data/trie/doubleListSync.go
Outdated
|
||
delete(dlts.marginMissing, hash) | ||
|
||
dlts.marginExisting[string(n.getHash())] = n |
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.
why margin ? it could be simply existing and missing
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.
it better explains that when we are traversing the trie we are holding and processing a virtual "margin" of nodes. The nodes closer to the root node are already processed and not present in the lists, the nodes closer to the leaves will be included in the margin lists and will be processed. After that they will be removed. For clarity I would keep the "margin" term.
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.
margin is clear for you. it does not necessary mean for clarity for others 🗡️
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.
renamed
741cd9e
# Conflicts: # data/syncer/userAccountsSyncer.go # data/trie/branchNode.go # data/trie/branchNode_test.go # data/trie/extensionNode.go # data/trie/extensionNode_test.go # data/trie/interface.go # data/trie/leafNode.go # data/trie/sync.go
59cbcaa