feat: introduce global thread pool and rename `ShellOtps` to `GlobalOpts`

foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.

https://getfoundry.sh

Apache License 2.0

8.31k stars 1.75k forks source link

feat: introduce global thread pool and rename `ShellOtps` to `GlobalOpts` #9313

Open zerosnacks opened 2 days ago

zerosnacks commented 2 days ago

Motivation

Closes: https://github.com/foundry-rs/foundry/issues/8408

Solution

Introduces global Rayon thread pool spawned by passing the --jobs flag. If not passed it is not spawned. If passed as --jobs 0 it will use the default Rayon configuration (number of logical CPUs).
Renames ShellOpts to GlobalOpts.
Only selectively passes down GlobalOpts where required, this can be the case when wanting to indicate a conflicts_with a global argument or you need access to the thread pool. In most cases when checking against the shell configuration it is probably easier to use the e.g. shell::is_json variant over passing it all the way down.

zerosnacks commented 1 day ago

impl looks good! there's a regression re the way rayon thread pool is spawned though, a command like

Thanks, good catch!

I think we should keep the current behavior, that is if --jobs flag not passed then we use rayon default (the number of CPUs available) instead single thread.

I had implemented it this way to avoid spawning the global thread pool as the vast majority of commands are not multi-threaded, cc @DaniPopes should we spawn by default?

grandizzy commented 9 hours ago

wonder how (if) this would also affect the threadpools spawned in deps (and if we want to or not) like compilers

https://github.com/foundry-rs/compilers/blob/034ecd611eef030217c4b363a794226cd50b4b9e/crates/compilers/src/compile/project.rs#L529-L530

zerosnacks commented 1 hour ago

wonder how (if) this would also affect the threadpools spawned in deps (and if we want to or not) like compilers

https://github.com/foundry-rs/compilers/blob/034ecd611eef030217c4b363a794226cd50b4b9e/crates/compilers/src/compile/project.rs#L529-L530

Good point, whenever Rayon is first accessed it will spawn a global thread pool with default settings (unless configured).

Right now it will attempt to spawn for a thread for each job here:

https://github.com/foundry-rs/foundry/blob/9d7557fcf0f758ea0e8ef5d2db853bd1e1d660dc/crates/common/src/compile.rs#L144

I'm not sure what we are optimizing for - not running with the default settings of available logical CPUs? It seems desirable, unless specifically required to be limited. A possible motivation would be to pay the cost of spawning upfront.

cc @DaniPopes