search

rust-lang / crater

Run experiments across parts of the Rust ecosystem!
https://crater.rust-lang.org
639 stars 90 forks source link

Replace runner graph with single list #623

Closed Mark-Simulacrum closed 2 years ago

Mark-Simulacrum commented 2 years ago

Replace it with a single Vec which contains the list of crates to build; each worker just pops the next crate in the list and then executes all of the steps for that crate in series (reading configuration etc as necessary).

We can in theory separate out the toolchains such that each worker only handles one toolchain (which would likely save disk space), but that is extra work and does not seem particularly beneficial at this time. It would also not be that hard to just duplicate the crate list N times and have each worker created with a specific toolchain and keep this setup almost as-is.

Overall this is a much simpler approach, removing a considerable amount of complexity from the workers and essentially eliminating any worker thread synchronization (e.g., condvars, running/not running tasks, etc). Popping from the Vec of crates is fast and worker threads will terminate when the list is exhausted, but will otherwise just work on the next crate in queue. This nearly matches the current setup -- today a crate is likely scheduled in parallel for each toolchain across two worker threads -- but it shouldn't make a significant difference in overall runtimes.

Mark-Simulacrum commented 2 years ago

@bors r+

bors commented 2 years ago

:pushpin: Commit 81ae92cc166e5bfb3c26487a53e9203f95fa7a5f has been approved by Mark-Simulacrum

bors commented 2 years ago

:hourglass: Testing commit 81ae92cc166e5bfb3c26487a53e9203f95fa7a5f with merge 672143ef714d9f00b441065e6280b173a244a3a6...

bors commented 2 years ago

:sunny: Test successful - checks-actions Approved by: Mark-Simulacrum Pushing 672143ef714d9f00b441065e6280b173a244a3a6 to master...

pietroalbini commented 2 years ago

Thanks for this! The original plan for the task graph was to have dependencies between crates to better handle caching, but that never got implemented. Fully agree with the removal.