Skip to content

Conversation

@djc
Copy link
Contributor

@djc djc commented Oct 27, 2025

This is what I had in mind. It seems to compile but I've done no testing or benchmarking.

@FranciscoTGouveia maybe that's something you can have a look at?

Alternative to

Obsoletes

@FranciscoTGouveia
Copy link
Contributor

It compiles, but I believe there is an infinite loop; at least, I could not get any toolchain installation to finish as it seemed to hang on the download of the last component.

Additionally, I would like to note that when a component is being installed, although ongoing downloads continue normally, new component downloads cannot start.
As a result, during an installation (assuming RUSTUP_CONCURRENT_DOWNLOADS=2, which is the default), no more than one download is active at a time.

This isn’t an issue when RUSTUP_CONCURRENT_DOWNLOADS is configured to maximize concurrency, but otherwise, it can have a noticeable impact on performance (I wasn’t able to benchmark this due to the infinite loop).

Finally, I would like to point out that running both downloads and installations on the same thread prevents the progress bars for other downloads from updating (see here).

@djc
Copy link
Contributor Author

djc commented Oct 28, 2025

I've fixed this up so that it can at least pass basic manual tests. CI is failing because I changed the log output -- happy to fix this up if we decide that we want to move forward with this.

Copy link
Member

@rami3l rami3l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this works out pretty well during both code inspection and manual tests, so I consider this to be a somewhat faithful reproduction of the first step of #4471. Given that, I agree with merging this despite the fact that the project will suffer from temporary UX regressions.

I'd like to invite @FranciscoTGouveia do another round of review from his side to make sure everything is retained from the original solution.

@djc djc enabled auto-merge November 1, 2025 14:08
@djc
Copy link
Contributor Author

djc commented Nov 1, 2025

Here's what I think we could do next to solve the remaining UX problem and even further improve the UX:

  • Update the progress indicator during unpacking, using a ProgressBarIter wrapper
  • Update the progress indicator during installing, by making installation "async", maybe by calling yield_now() every 250ms or so from DirectoryPackage::install()

(A more principled approach might be to use tokio::fs but because there's a lot of layers in our install process that's probably a bunch of work, if not exactly hard.)

@djc djc added this pull request to the merge queue Nov 2, 2025
Merged via the queue into main with commit fcd02c0 Nov 2, 2025
29 checks passed
@djc djc deleted the async-install branch November 2, 2025 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants