Skip to content

Conversation

@rami3l
Copy link
Member

@rami3l rami3l commented Nov 11, 2025

Don't merge: this PR is for evaluation purposes only.

From my local testing it seems that DirectoryPackage::install() is not a good candidate for the said optimization in https://rust-lang.zulipchat.com/#narrow/channel/490103-t-rustup/topic/Shipping.20fully.20concurrent.20downloads.20of.20rust.20components/near/554900068. On my machine, this has slowed down the installation of the beta toolchain with default profile by ~2x.

@rami3l rami3l marked this pull request as draft November 11, 2025 13:17
let manifest = utils::read_file("package manifest", &root.join("manifest.in"))?;
let mut builder = target.add(name, tx);

let yield_timeout = Duration::from_millis(50);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is pretty short -- have you tried with longer intervals, like 250/500/1000ms?

Copy link
Member Author

Choose a reason for hiding this comment

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

No significant changes on my side with either of those values: the installation is always ~2x slower than latest.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, that seems very surprising.

@djc
Copy link
Contributor

djc commented Nov 11, 2025

(IMO [DON'T MERGE] is really not helpful when the PR is draft anyway.)

@rami3l rami3l changed the title [DON'T MERGE] refactor(dist/component/package): make DirectoryPackage::install() async refactor(dist/component/package): make DirectoryPackage::install() async Nov 11, 2025
@rami3l
Copy link
Member Author

rami3l commented Nov 11, 2025

(IMO [DON'T MERGE] is really not helpful when the PR is draft anyway.)

The intention is that it will never be merged instead of passing through the normal state.

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.

2 participants