feat: add cross-block cache size cli arg #14305
Conversation
fgimenez
added
C-perf
fgimenez
requested review from
mattsse,
Rjected,
onbjerg,
klkvr and
rkrasiuk
as code owners
crates/node/core/src/node_config.rs
Outdated
| /// Default size of cross-block cache. | ||
| pub const DEFAULT_CROSS_BLOCK_CACHE_SIZE: u64 = 4 * 1024 * 1024 * 1024; |
There was a problem hiding this comment.
this is in bytes?
can we mention this?
crates/node/core/src/args/engine.rs
Outdated
| /// Configure the size of cross-block cache | ||
| #[arg(long = "engine.cross-block-cache-size", default_value_t = DEFAULT_CROSS_BLOCK_CACHE_SIZE)] |
There was a problem hiding this comment.
this should mention the unit,
for other caches we expect that the input is MB because handling bytes is a bit useless:
for example:
reth/crates/node/core/src/args/rpc_server.rs
Lines 123 to 125 in b955551
There was a problem hiding this comment.
makes sense, done here 0918a5b PTAL
| @@ -65,7 +65,7 @@ impl Default for TreeConfig { | |||
| use_state_root_task: false, | |||
| always_compare_trie_updates: false, | |||
| use_caching_and_prewarming: false, | |||
| cross_block_cache_size: DEFAULT_CROSS_BLOCK_CACHE_SIZE, | |||
| cross_block_cache_size: DEFAULT_CROSS_BLOCK_CACHE_SIZE_MB, | |||
There was a problem hiding this comment.
hmm, not sure if we should mix this here, I lean towards handling this here in bytes as well as build_caches
wdyt @Rjected
There was a problem hiding this comment.
yep better to be consistent internally, i¡ve pushed changes to manage bytes from TreeConfig, done the change MB -> bytes in the cache size setter invocation, ptal
| builder.config.engine.state_root_task_compare_updates, | ||
| .with_always_compare_trie_updates(builder.config.engine.state_root_task_compare_updates) | ||
| .with_cross_block_cache_size( | ||
| builder.config.engine.cross_block_cache_size * 1024 * 1024, |
Closes #14300