-
Notifications
You must be signed in to change notification settings - Fork 190
[Storage] Optimize memory cache key creation and key format for some stores #7391
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 speeds up key creation by ~372x and eliminates all memory allocs (used by key creation) for some stores. It also optimizes the cache key format to be smaller. Previously, memory cache key format for transaction results was inefficient and creates needless allocations by encoding to hex. Same issue exists for transaction result index cache and block cache. Light transaction results used by AN also use the same cache key format. This commit optimizes cache key format for multiple stores: - cache key format: - old: hex(blockID) + hex(txID) - new: blockID+txID - index cache key format: - old hex(blockID) + hex(txIndex) - new blockID+txIndex - block cache key format: - old hex(blockID) - new blockID Updated stores include: - TransactionResults - TransactionResultErrorMessages - LightTransactionResults Since cache keys are created for every read or insert of transaction results, etc., it is good to reduce memory allocs when it doesn't sacrifice speed. Benchmark stats for creating cache key: │ old.log │ new.log │ │ sec/op │ sec/op vs base │ TransactionResultCacheKey/_create_cache_key-12 882.000n ± 8% 2.368n ± 0% -99.73% (p=0.000 n=10) │ old.log │ new.log │ │ B/op │ B/op vs base │ TransactionResultCacheKey/_create_cache_key-12 576.0 ± 0% 0.0 ± 0% -100.00% (p=0.000 n=10) │ old.log │ new.log │ │ allocs/op │ allocs/op vs base │ TransactionResultCacheKey/_create_cache_key-12 9.000 ± 0% 0.000 ± 0% -100.00% (p=0.000 n=10)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7391 +/- ##
=======================================
Coverage 41.10% 41.11%
=======================================
Files 2207 2207
Lines 193498 193436 -62
=======================================
- Hits 79544 79532 -12
+ Misses 107325 107290 -35
+ Partials 6629 6614 -15
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:
|
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.
Nice optimization 👍 Thanks Faye.
Updates #7242
This speeds up key creation by ~372x and eliminates all memory allocs (used by key creation) for some stores. It also optimizes the cache key format to be smaller.
I found this "low-hanging fruit" optimization last week while working on a bugfix PR:
Previously, memory cache key format for transaction results was inefficient and creates needless allocations by encoding to hex. Same issue exists for transaction result index cache and block cache. Light transaction results used by AN also use the same cache key format.
This PR optimizes cache key format for multiple stores:
hex(blockID) + hex(txID)
blockID + txID
hex(blockID) + hex(txIndex)
blockID + txIndex
hex(blockID)
blockID
Updated stores include:
TransactionResults
TransactionResultErrorMessages
LightTransactionResults
Since cache keys are created for every read or insert of transaction results, etc., it is good to reduce memory allocs when it doesn't sacrifice speed.
Benchmark stats for creating cache key: