Improve --help performance for x.py #48569

Phlosioneer · 2018-02-27T01:54:41Z

Since compiling the bootstrap command doesn't require any submodules,
we can skip updating submodules when a --help command is passed in.
On my machine, this saves 1 minute if the submodules are already
downloaded, and 10 minutes if run on a clean repo.

This commit also adds a message before compiling/downloading anything
when a --help command is passed in, to tell the user WHY --help
takes so long to complete. It also points the user to the bootstrap
README.md for faster help.

Finally, this fixes one warning message that still referenced using
make instead of x.py, even though x.py is now the standard way of
building rust.

Closes #37305

Since compiling the bootstrap command doesn't require any submodules, we can skip updating submodules when a --help command is passed in. On my machine, this saves 1 minute if the submodules are already downloaded, and 10 minutes if run on a clean repo. This commit also adds a message before compiling/downloading anything when a --help command is passed in, to tell the user WHY --help takes so long to complete. It also points the user to the bootstrap README.md for faster help. Finally, this fixes one warning message that still referenced using make instead of x.py, even though x.py is now the standard way of building rust.

rust-highfive · 2018-02-27T01:54:53Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

estebank · 2018-02-27T02:30:50Z

@bors r+

bors · 2018-02-27T02:30:51Z

📌 Commit ffb6291 has been approved by estebank

petrochenkov · 2018-02-27T08:31:07Z

src/bootstrap/bootstrap.py

+        if help_triggered:
+            print("NOTE: Downloading and compiling bootstrap requirements before processing")
+            print("      --help command. See src/bootstrap/README.md for help with common")
+            print("      commands.")


We should get this message only if we actually going to download something or do other long operation.

Actually there's already print_what_bootstrap_means that is supposed to print this exact information, but maybe it doesn't fire for some reason?

I decided to print the message every time, because we always rebuild the build tool. So it will never be instant. Also, doing anything more complicated than printing this at the beginning means that we have to go through the whole process to gather information, setup environment variables, and check if anything needs to be done. It would require a lot of logic spread through a lot of the build script, and that complexity simply isn't worth it.

I have never noticed print_what_bootstrap_means. Looking back at it, it only triggers if a download happens, and is immediately followed by a lot of download messages and loading bars. Also, prior to this fix, the very first thing the build script did was update_submodules, so print_what_bootstrap_means would be printed after the most time consuming part of the build script.

The message also doesn't reference --help at all; it states why stuff needs to download before running build commands, but the user will not expect --help to be part of that. My message makes that perfectly clear.

Basically, we either have to print this whenever --help happens, regardless of what we actually need to do; OR we have to wait until the build script sets up enough to know if it needs to download, at which point seconds have already passed and the tool will feel sluggish (asking the user to wait after the user has already waited). And if we want to do the second option, because of how the script is organized, we would either have to do a lot of refactoring, or litter the code with checks and print statements.

Ok, sounds good.
Could you remove print_what_bootstrap_means (and also self.printed) then?

petrochenkov · 2018-02-27T08:31:27Z

@bors r-

alexcrichton · 2018-02-27T19:26:15Z

r? @petrochenkov

petrochenkov · 2018-03-01T15:43:05Z

src/bootstrap/bootstrap.py

-        bootstrap()
+        # If the user is asking for help, let them know that the whole download-and-build
+        # process has to happen before anything is printed out.
+        if help_triggered:


Could you move this into bootstrap()? Top level main is for catching and handling exceptions only.

petrochenkov · 2018-03-01T15:52:40Z

src/bootstrap/bootstrap.py

+        # If the user is asking for help, let them know that the whole download-and-build
+        # process has to happen before anything is printed out.
+        if help_triggered:
+            print("NOTE: Downloading and compiling bootstrap requirements before processing")


Is the word "requirements" used in this meaning? "Downloading requirements" sounds weird.
Maybe "Downloading and building bootstrap and its dependencies ..."?

It was an existing solution to tell the user why a --help command takes a long time to process. However, it would only print if the stage0 rust compiler needed to be downloaded, it came after update_submodules (which took a long time), and it was immediately followed by download messages and loading bars, meaning users could easily gloss over the message. This commit also moves the help message out of main(), and instead puts it at the top of bootstrap(). main() is intended to be minimal, only handling error messages.

petrochenkov · 2018-03-02T08:56:01Z

Thanks!
@bors r+

bors · 2018-03-02T08:56:01Z

📌 Commit 2269ff5 has been approved by petrochenkov

petrochenkov · 2018-03-02T08:56:28Z

@bors rollup

@nrc

… r=petrochenkov Fixes rust-lang#47311. r? @nrc

@nrc

… r=petrochenkov Fixes rust-lang#47311. r? @nrc

… r=petrochenkov Improve --help performance for x.py Since compiling the bootstrap command doesn't require any submodules, we can skip updating submodules when a --help command is passed in. On my machine, this saves 1 minute if the submodules are already downloaded, and 10 minutes if run on a clean repo. This commit also adds a message before compiling/downloading anything when a --help command is passed in, to tell the user WHY --help takes so long to complete. It also points the user to the bootstrap README.md for faster help. Finally, this fixes one warning message that still referenced using make instead of x.py, even though x.py is now the standard way of building rust. Closes rust-lang#37305

Rollup of 8 pull requests - Successful merges: #48283, #48466, #48569, #48629, #48637, #48680, #48513, #48664 - Failed merges:

rust-highfive assigned alexcrichton Feb 27, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 27, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 27, 2018

petrochenkov reviewed Feb 27, 2018

View reviewed changes

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 27, 2018

rust-highfive assigned petrochenkov and unassigned alexcrichton Feb 27, 2018

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 28, 2018

petrochenkov reviewed Mar 1, 2018

View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 1, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 2, 2018

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 2, 2018

Rollup merge of rust-lang#48569 - Phlosioneer:x-py-help-optimization,…

03f32b9

… r=petrochenkov Fixes rust-lang#47311. r? @nrc

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 3, 2018

Rollup merge of rust-lang#48569 - Phlosioneer:x-py-help-optimization,…

bffea5d

… r=petrochenkov Fixes rust-lang#47311. r? @nrc

Manishearth mentioned this pull request Mar 3, 2018

Rollup of 8 pull requests #48687

Closed

kennytm mentioned this pull request Mar 3, 2018

Rollup of 8 pull requests #48694

Merged

bors added a commit that referenced this pull request Mar 3, 2018

Auto merge of #48694 - kennytm:rollup, r=kennytm

e026b59

Rollup of 8 pull requests - Successful merges: #48283, #48466, #48569, #48629, #48637, #48680, #48513, #48664 - Failed merges:

bors merged commit 2269ff5 into rust-lang:master Mar 3, 2018

ehuss mentioned this pull request Mar 27, 2018

Added submodule fetch instructions. #49338

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve --help performance for x.py #48569

Improve --help performance for x.py #48569

Phlosioneer commented Feb 27, 2018

rust-highfive commented Feb 27, 2018

estebank commented Feb 27, 2018

bors commented Feb 27, 2018

petrochenkov Feb 27, 2018 •

edited

Loading

Phlosioneer Feb 27, 2018

petrochenkov Mar 1, 2018

petrochenkov commented Feb 27, 2018

alexcrichton commented Feb 27, 2018

petrochenkov Mar 1, 2018

petrochenkov Mar 1, 2018 •

edited

Loading

petrochenkov commented Mar 2, 2018

bors commented Mar 2, 2018

petrochenkov commented Mar 2, 2018

Improve --help performance for x.py #48569

Improve --help performance for x.py #48569

Conversation

Phlosioneer commented Feb 27, 2018

rust-highfive commented Feb 27, 2018

estebank commented Feb 27, 2018

bors commented Feb 27, 2018

petrochenkov Feb 27, 2018 • edited Loading

Choose a reason for hiding this comment

Phlosioneer Feb 27, 2018

Choose a reason for hiding this comment

petrochenkov Mar 1, 2018

Choose a reason for hiding this comment

petrochenkov commented Feb 27, 2018

alexcrichton commented Feb 27, 2018

petrochenkov Mar 1, 2018

Choose a reason for hiding this comment

petrochenkov Mar 1, 2018 • edited Loading

Choose a reason for hiding this comment

petrochenkov commented Mar 2, 2018

bors commented Mar 2, 2018

petrochenkov commented Mar 2, 2018

petrochenkov Feb 27, 2018 •

edited

Loading

petrochenkov Mar 1, 2018 •

edited

Loading