core/state: reduce lock contention in triePrefetcher.used()#2049
core/state: reduce lock contention in triePrefetcher.used()#2049
Conversation
The used() method previously held a global write lock while doing a map lookup, a blocking fetcher.wait(), and slice appends — serializing all N parallel IntermediateRoot goroutines behind a single mutex. Fix this by downgrading to a read lock for the map lookup and moving slice appends behind a per-subfetcher usedLock via a new appendUsed() method.
b768d72 to
8c20442
Compare
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2049 +/- ##
===========================================
+ Coverage 50.15% 50.20% +0.04%
===========================================
Files 871 871
Lines 150614 150621 +7
===========================================
+ Hits 75546 75624 +78
+ Misses 70023 69952 -71
Partials 5045 5045
... and 20 files with indirect coverage changes
🚀 New features to boost your workflow:
|
lucca30
left a comment
There was a problem hiding this comment.
Besides the lint fix, the code looks pretty good. Nice finding on replacing a global lock by a more granular one.
|
|
Misclicked on CoPilot. |
There was a problem hiding this comment.
Pull request overview
This pull request optimizes the triePrefetcher.used() method to reduce lock contention when multiple goroutines update usage statistics for different subfetchers concurrently. The optimization is particularly beneficial during IntermediateRoot processing, where multiple goroutines operate on independent storage tries.
Changes:
- Refactored
triePrefetcher.used()to use a read lock instead of a write lock for map lookups, allowing concurrent access by multiple goroutines - Introduced per-subfetcher
usedLockto protectusedAddrandusedSlotslice appends, replacing the previous global write lock - Added comprehensive tests to verify correctness of concurrent access and validate that the optimization actually provides parallelism
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| core/state/trie_prefetcher.go | Refactored used() method to use read lock; added appendUsed() method with per-subfetcher locking; updated report() to acquire usedLock when reading usage data |
| core/state/trie_prefetcher_test.go | Added three new tests: TestConcurrentUsed (correctness), TestConcurrentUsedParallelism (performance), and TestUsedStateCorrectAfterReport (integration with report()) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* core/state: reduce lock contention in triePrefetcher.used() The used() method previously held a global write lock while doing a map lookup, a blocking fetcher.wait(), and slice appends — serializing all N parallel IntermediateRoot goroutines behind a single mutex. Fix this by downgrading to a read lock for the map lookup and moving slice appends behind a per-subfetcher usedLock via a new appendUsed() method. * fix linter
Description
The used() method previously held a global write lock while doing a map lookup, a blocking fetcher.wait(), and slice appends — serializing all N parallel IntermediateRoot goroutines behind a single mutex. Fix this by downgrading to a read lock for the map lookup and moving slice appends behind a per-subfetcher usedLock via a new appendUsed() method.
Changes
Breaking changes
Please complete this section if any breaking changes have been made, otherwise delete it
Nodes audience
In case this PR includes changes that must be applied only to a subset of nodes, please specify how you handled it (e.g. by adding a flag with a default value...)
Checklist
Cross repository changes
Testing
Manual tests
Please complete this section with the steps you performed if you ran manual tests for this functionality, otherwise delete it
Additional comments
Please post additional comments in this section if you have them, otherwise delete it