fix(store): Fix refcount logic slowness #3123
Conversation
Use rocksdb merge operator for ColState. No longer need to atomically read + write on update. Fixes #3065 Test plan --------- sanity test manually check performance
mikhailOK
requested review from
evgenykuzyakov and
MaksymZavershynskyi
as code owners
|
|
| cursor.into_inner() | ||
| } | ||
|
|
||
| fn decode_trie_node_with_rc(bytes: &[u8]) -> Result<(&[u8], u32), StorageError> { | ||
| fn decode_trie_node_with_rc(bytes: &[u8]) -> Result<(&[u8], i32), StorageError> { |
There was a problem hiding this comment.
Update writes encoded positive or negative refcount delta, and merge adds up updates. Records/partial sum can be negative, but the result from db.get() should always be positive (if it is <= 0, there is a bug in gc). Probably need to add a new assert, this change deleted the one from apply_deletions.
Co-authored-by: Bowen Wang <bowenwang1996@users.noreply.github.com>
Co-authored-by: Bowen Wang <bowenwang1996@users.noreply.github.com>
|
In #3065 the bottleneck was calling |
|
Applying a block should become faster |
Codecov Report
@@ Coverage Diff @@
## master #3123 +/- ##
=======================================
Coverage 87.25% 87.26%
=======================================
Files 212 212
Lines 41424 41515 +91
=======================================
+ Hits 36145 36226 +81
- Misses 5279 5289 +10
Continue to review full report at Codecov.
|
| @@ -395,21 +396,21 @@ impl RawTrieNodeWithSize { | |||
| } | |||
| } | |||
|
|
|||
| fn encode_trie_node_with_rc(data: &[u8], rc: u32) -> Vec<u8> { | |||
| fn encode_trie_node_with_rc(data: &[u8], rc: i32) -> Vec<u8> { | |||
There was a problem hiding this comment.
Should the naming reflect that this is now working with delta values instead of absolute values?
There was a problem hiding this comment.
It's actually both: we write deltas, but read absolute values (aggregation happens in merge operator). Makes sense to do a refactor to move refcount logic from Trie to Store, but we can do it separately
Co-authored-by: Vlad Frolov <frolvlad@gmail.com>
Co-authored-by: Vlad Frolov <frolvlad@gmail.com>
mikhailOK
force-pushed
the
merge_operator
branch
from
7d906c9
to
a788980
Compare
|
Manually tested restarting with this change on old storage |
Use rocksdb merge operator for ColState. No longer need to atomically
read + write on update.
Fixes #3065
Test plan
sanity test
manually check performance