Run a parallel pre-check before updating toolchains#4752
Run a parallel pre-check before updating toolchains#4752FranciscoTGouveia wants to merge 1 commit intorust-lang:mainfrom
Conversation
src/cli/common.rs
Outdated
| future::join_all(channels.iter().map(|(_, d)| d.show_dist_version())) | ||
| .await | ||
| .into_iter() | ||
| .map(|v| v.map_or(true, |opt| opt.is_some())) | ||
| .collect() | ||
| }; |
There was a problem hiding this comment.
Is there a cleaner way to achieve this?
There was a problem hiding this comment.
Am I reading it right that now for every toolchain that needs to be updated, the update is checked twice?
There was a problem hiding this comment.
Yes, I forgot to add a comment about this, sorry.
I see two possible ways to proceed:
- Accept the cost of performing a double check. In practice, since the first check is done concurrently in a batch, the overhead should be relatively small.
- Do a small refactor, starting in
DistOptions::install_into(), to allow skipping the second check.
There was a problem hiding this comment.
@FranciscoTGouveia I think we should try to avoid double checking if the changes involved are not super disruptive 🙏
| // Run a pre-check to determine which channels have updates available. | ||
| // This step is skipped when force-updating. |
There was a problem hiding this comment.
Feedback on the wording of the comment is welcome :)
FranciscoTGouveia
force-pushed
the
update-performance
branch
from
18df18c to
d6cc274
Compare
| join_all(channels.iter().map(|(_, d)| d.show_dist_version())) | ||
| .await | ||
| .into_iter() | ||
| .map(|v| v.map_or(true, |opt| opt.is_some())) |
There was a problem hiding this comment.
I'm confused by this logic. show_dist_version() seems to merely yield the latest available version, but that a DistributableToolchain has a manifest with a rust component with a version is_some() doesn't mean that it's newer than the installed version?
3 participants
Fixes #4747.
Instead of sequentially checking each toolchain for updates,
rustup updatenow first checks all toolchains concurrently and only then performs the updates (if needed).This brings two advantages:
rustup updateis now ~2x faster (see below);nightlyneeds updating.Benchmarks were run using
hyperfine(20 iterations on a 50 Mbps connection) on a setup with 11 toolchains, and no regressions were observed.For the no-op
rustup update, this patch was 2.2x faster; for cases where a single toolchain was updated, there was no measurable difference.CC @epage