-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parallelize initial Rust download in bootstrap #110427
Conversation
This make `_download_component_helper` "pure".
(rustbot has picked a reviewer for you, use r? to override) |
Apparently we want to be python 2 compatible here, I'll fix that tomorrow. |
I think we might be able to drop python2 compat in bootstrap.py proper now that the x.py shim reexecs itself with python3. |
I wasn't able to see this on my fast internet connection, but testing this on a slower one shows that the progress bars get pretty broken by the parallel download which is not ideal. |
4657f83
to
6287bc3
Compare
I made it Python 2.7 compatible and tested it manually using |
Is the progress bar mess fixed? |
Not really, I'm unsure about the best approach to it. I can try whether parallel downloads are faster on a fast internet connection (I wouldn't expect them to be faster on a slow one) and if no, just keep this part serial. If they are am advantage, I'll try find something. |
Inserting an with a fast (480MiB/s) internet connection with a less fast (60MiB/s) internet connection so the parallel download is actually worse for slow connections, probably because it reduces parallelism for the extraction phase since the downloads don't all complete at the same time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one question/nit/potential improvement.
38eb5a5
to
e829adc
Compare
Thanks for the PR! |
📌 Commit e829adc819c55c0736bcdce948c53f2920adf5b1 has been approved by It is now in the queue for this repository. |
⌛ Testing commit e829adc819c55c0736bcdce948c53f2920adf5b1 with merge 0a92164116464c739fa27fe5e2b820e598bdd7ca... |
@bors r- |
This is quite slow and embarassingly parallel, even in python. This speeds up the initial bootstrap build by about 5-10s.
e829adc
to
a98968e
Compare
I quickly tested it locally again and it worked. |
This comment has been minimized.
This comment has been minimized.
☀️ Test successful - checks-actions |
Finished benchmarking commit (9ecda8d): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. |
This seems to be causing bootstrap to get stuck in a loop and unable to proceed on some systems. I'm seeing problems on macOS, and #111046 reports similar problems on Windows. |
( If I edit |
Parallelize the initial download of Rust in
bootstrap.py
time ./x.py --help
afterrm -r build
Before: 33s
After: 27s