Add --partition M/N to distribute builds #253
Conversation
| progress.count = new_count; | ||
| if skip { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| if cx.clean_per_run { |
There was a problem hiding this comment.
(unrelated to this pr: i don't think cargo_clean(...) is needed if cx.print_command_list == true..right?)
There was a problem hiding this comment.
Yeah. Well, both are optional and I don't know what the use case of using --clean-per-run together with -pr-int-command-list in the first place, though...
There was a problem hiding this comment.
thanks for quick reply. I'll create a small pr after this pr to fix this.
src/main.rs
Outdated
| let mut msg = String::new(); | ||
| if term::verbose() { | ||
| write!(msg, "skipping {line}").unwrap(); | ||
| } else { | ||
| write!(msg, "skipping {line} on {}", cx.packages(id).name).unwrap(); | ||
| } | ||
| write!(msg, " ({}/{})", new_count, progress.total).unwrap(); |
There was a problem hiding this comment.
this may be subjective, but i liked explicitly printing messages about being skipped.
src/main.rs
Outdated
| let new_count = progress.count + 1; | ||
| let mut skip = false; | ||
| if let Some(partition) = &cx.partition { | ||
| if progress.count % partition.count != partition.index - 1 { |
There was a problem hiding this comment.
this again subjective as well. but it's more intuitive not to see adjusted progress (i.e. {X/N}/{Y/N}) after partitioning while showing the skip messages.
ryoqun
force-pushed
the
partition
branch
4 times, most recently
from
fa2214e to
64b44ff
Compare
|
@taiki-e hey, me again at different places. :) Could you review this in your free time? |
src/main.rs
Outdated
| if progress.count % partition.count != partition.index - 1 { | ||
| let mut msg = String::new(); |
There was a problem hiding this comment.
IIUC this would be something like:
1/4 run
2/4 skipped
3/4 run
4/4 skipped
I think this is fine for the nextest use, I guess it maybe a bit inefficient for cargo-hack use, since the feature combinations that would reduce the amount of recompilation are often before or after the current combination.
There was a problem hiding this comment.
thanks for nice suggestion. I've switched from interleaved partitioning to chunked partitioning. and seems our ci got quicker to finish: https://buildkite.com/anza/agave/builds/7952#0190d3f9-52af-4b75-9e69-e408f4a1a57e
|
Hey, sorry for delayed reply... I got some time to work on this.
really thanks. :)
sorry about that. I added tests. |
|
@taiki-e hi, friendly ping. :) |
|
@taiki-e i guess, maybe this pr can't be merged soon? this pr is blocking our ci fix: anza-xyz/agave#2127, it seems that we have to install |
|
@taiki-e just saw thank you very much! |
if there are many workspace members, it takes very long to execute all runs. So, introduce
--partition M/Nto distribute runs across machines.This pr is in draft still. If this addition of new flag sounds acceptable, I'll finish up it (maybe, writing tests and improve cli handling (edit: DONE), update docs?)
demo: https://buildkite.com/anza/agave/builds/7618#0190b45e-b9e3-4b64-a172-4d9fc6b52e0f
related: #180, #248 (... in the sense this is about accelerating the whole step of
cargo hackin CIs)the exact behavior is moderately mirrored from the counted partitioning of nextest: https://nexte.st/docs/ci-features/partitioning/#counted-partitioning
context: rust-lang/cargo#10958 (comment) and rust-lang/cargo#8379 (comment)