-
Notifications
You must be signed in to change notification settings - Fork 146
fix(l1): make function inner for InMemoryTrieDB #5199
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
Conversation
Lines of code reportTotal lines added: Detailed view |
Benchmark for bc435bbClick to view benchmark
|
Benchmark for 1b3c4fdClick to view benchmark
|
Benchmark for dc5c8e1Click to view benchmark
|
Benchmark for ac0e238Click to view benchmark
|
This reverts commit 6b665f8.
Benchmark for 35ca949Click to view benchmark
|
Benchmark for 9828dbaClick to view benchmark
|
Benchmark for 92f657cClick to view benchmark
|
put_batch for TrieWrapper
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.
Pull Request Overview
This PR refactors the trie cache synchronization mechanism to use RwLock instead of a Read-Copy-Update (RCU) pattern with Mutex. The key motivation is to improve concurrency by allowing multiple readers while maintaining write safety. Additionally, it implements the previously unimplemented put_batch method in TrieWrapper.
Key changes:
- Replaced
Arc<Mutex<Arc<TrieLayerCache>>>withArc<RwLock<TrieLayerCache>>to eliminate the double-Arc RCU pattern - Implemented
put_batchfunctionality inTrieWrapperwith proper state root management - Added a new
inner()method toInMemoryTrieDBfor exposing the internalNodeMap
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/storage/trie_db/layering.rs | Wrapped TrieWrapper fields in RwLock and implemented put_batch with state root computation and prefix application |
| crates/storage/store_db/rocksdb.rs | Refactored trie cache from RCU pattern to direct RwLock usage, simplified layer management code |
| crates/storage/store_db/in_memory.rs | Updated in-memory store to use RwLock for trie cache consistency with rocksdb implementation |
| crates/common/trie/db.rs | Added inner() accessor method to InMemoryTrieDB |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Maybe we don't want this, so I will address Copilot comments after we decide if we actually want this or not. Otherwise we may have to solve it in a different way. Edit: yep, I made a mistake, we didn't want this and it wasn't even necessary |
put_batch for TrieWrapper
Benchmark for d28953eClick to view benchmark
|
Benchmark for 4fa4235Click to view benchmark
|
avilagaston9
left a comment
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.
Maybe it’s better to clarify in the comments that we need these methods for ethrex-replay
|
Benchmark for 78dfcafClick to view benchmark
|
**Motivation** <!-- Why does this pull request exist? What are its goals? --> - Make necessary changes in order for ethrex-replay to work fine (we make use of these things there) **Description** - Added a method to `InMemoryTrieDB` to use it in ethrex-replay
Motivation
Description
InMemoryTrieDBto use it in ethrex-replay