Mostly configurable canonical cache size #2516
Conversation
rphmeier
added
A0-pleasereview 🤓
|
looks reasonable |
|
lgtm, but haven't dived deeply into logic. will let @arkpar review before merge. |
| let cap = jump_dests.capacity(); | ||
|
|
||
| // grow the cache as necessary; it operates on amount of items | ||
| // but we're working based on memory usage. |
There was a problem hiding this comment.
Would be better to create an a type of LRUCache that manages that and reuse it for the state cache as well. E.g LRUHeapSizeCache that requires HeapSizeOf. Can be done in a future PR though
There was a problem hiding this comment.
I might make an upstream PR to implement that. Contributing back to the community and all :)
There was a problem hiding this comment.
I've filed an issue at contain-rs/lru-cache#38, I intend to implement and incorporate to the follow-up PR where we'll want this in more than one place.
| @@ -211,6 +211,8 @@ usage! { | |||
| or |c: &Config| otry!(c.footprint).cache_size_blocks.clone(), | |||
| flag_cache_size_queue: u32 = 50u32, | |||
| or |c: &Config| otry!(c.footprint).cache_size_queue.clone(), | |||
| flag_cache_size_state: u32 = 25u32, | |||
There was a problem hiding this comment.
Should be at least 64MB by default
There was a problem hiding this comment.
I don't agree. 64MB is quite large and I find that there are diminishing returns after a certain point. With the default size, I find that the heavy blocks only process twice as slowly as with an unrestricted cache.
I think keeping the footprint light by default should be a priority. We want Parity to be able to run on an rPi. Miners have both the resources and the technical knowledge to increase the cache size using the flag.
arkpar
added
A5-grumble 🔥
rphmeier
added
A0-pleasereview 🤓
arkpar
added
A8-looksgood 🦄
Account storage caches (within the cached account entries) aren't currently configurable, but the jump tables and account cache itself are through the
--cache-size-stateflag. This will change in a future PR