-
Notifications
You must be signed in to change notification settings - Fork 229
feat: cache data hash to optimize performance for empty blocks #2616
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
This optimization addresses the performance issue where hash calculation for empty blocks becomes costly when called repeatedly. The solution: - Adds private cachedHash field to Data struct - Uses thread-safe caching with RWMutex for concurrent access - Automatically invalidates cache when data changes via FromProto - Maintains backward compatibility with existing API Co-authored-by: Marko <[email protected]>
The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).
|
message Data { | ||
Metadata metadata = 1; | ||
repeated bytes txs = 2; | ||
bytes cached_hash = 3; // Optional cached hash for performance |
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.
Do we really need to add this? Just caching the last one in the manager is sufficient right? Sure it is less optimized that cache it for all, but the logic that omits the cache for the serialization can be confusing if you use the api directly .
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.
in the flow, every time we get we would rehash since we rarely to almost never keep the data struct alive for multiple checks on the hash after we get from the DB. This would make the hash cache nullified, evaluating if this cache is worth it if we remove it from the proto
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.
don't we only care about height-1 anyway?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2616 +/- ##
==========================================
- Coverage 72.41% 0 -72.42%
==========================================
Files 72 0 -72
Lines 7394 0 -7394
==========================================
- Hits 5354 0 -5354
+ Misses 1600 0 -1600
+ Partials 440 0 -440
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This optimization addresses the performance issue where hash calculation for empty blocks becomes costly when called repeatedly.
Changes
Performance Impact
Eliminates expensive marshaling + SHA256 operations for repeated hash calls on the same data, particularly beneficial for empty blocks.
Closes #2612
Generated with Claude Code