Add thread manager crate to agave #3890
Conversation
alexpyattaev
force-pushed
the
thread_manager
branch
3 times, most recently
from
da038b3 to
309350b
Compare
| @@ -48,11 +48,12 @@ fn main() -> anyhow::Result<()> { | |||
| println!("Running {exp}"); | |||
| let mut conffile = PathBuf::from(env!("CARGO_MANIFEST_DIR")); | |||
| conffile.push(exp); | |||
| let conffile = std::fs::File::open(conffile)?; | |||
There was a problem hiding this comment.
Typically, we don't specify configurations in files. Doing it in code instead for the example/test cases, or with command line flags for other cases. Sometimes use yml (for accounts information for example).
There was a problem hiding this comment.
I got you, you want to read thread-manager/examples/core_contention_contending_set.toml with it.
Yeah, could you maybe have a method that reads this file and used in both example and production environment in the future?
There was a problem hiding this comment.
this file will never be read in prod, its just for examples which are also benchmarks/tests. I'm not even sure we will use toml in prod for this yet, though it is likely.
| let cfg: RuntimeManagerConfig = serde_json::from_reader(conffile)?; | ||
| let mut buf = String::new(); | ||
| std::fs::File::open(conffile)?.read_to_string(&mut buf)?; | ||
| let cfg: RuntimeManagerConfig = toml::from_str(&buf)?; | ||
| //println!("Loaded config {}", serde_json::to_string_pretty(&cfg)?); |
There was a problem hiding this comment.
I would use log / tracing crate instead of print
| //println!("Loaded config {}", serde_json::to_string_pretty(&cfg)?); | ||
|
|
||
| let rtm = RuntimeManager::new(cfg).unwrap(); | ||
| let rtm = ThreadManager::new(cfg).unwrap(); |
There was a problem hiding this comment.
naming: rtm, cfg,tok is uncommon. we typically use the full descriptive names. Lile rtm -> thread_manager
|
I think you need to explain broader audience proposed changes in the context. Maybe by setting up a meeting with involved parties. |
|
Yes, a meeting would be necessary eventually, but I also need the code to be reasonably good before a million people look at it=) |
alexpyattaev
force-pushed
the
thread_manager
branch
from
df8b9d2 to
3c83c09
Compare
alexpyattaev
force-pushed
the
thread_manager
branch
from
2c6a9dd to
4e1d728
Compare
Problem
There exist a variety of perf issues related to unorganized thread pools that spawn far more threads than are useful on a given machine, this was identified by firedancer and is relatively easy to fix in agave.
Idea is to eventually address the needs of
Summary of Changes
Added new agave-thread-manager crate, which is to be gradually hooked into the agave itself. It will be used to centralize thread pool creation such that their core allocations can be controlled, and total thread count can gradually be reduced to match core count as closely as possible. Crate comes with its own benchmarks and tests to establish config policies.