Skip to content
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 tidy #82347

Merged
merged 3 commits into from
Apr 4, 2021
Merged

Parallelize tidy #82347

merged 3 commits into from
Apr 4, 2021

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Feb 20, 2021

Split off from #81833

While that PR brings wall time of x.py test tidy down to 0m2.847s adding this one on top should bring it down to 0m1.673s.

r? @Mark-Simulacrum

Previous concerns can be found at #81833 (comment) and #81833 (comment)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 20, 2021
@Mark-Simulacrum
Copy link
Member

I think I'd be happy to merge with #81833 (comment), presuming that seems reasonable to you. I think for the parallelism in main I'm not too worried, it should continue to be relatively straightforward to add more passes.

@Mark-Simulacrum Mark-Simulacrum 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 Feb 20, 2021
@rust-log-analyzer

This comment has been minimized.

@the8472
Copy link
Member Author

the8472 commented Feb 21, 2021

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot 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 21, 2021
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=iffy

Thanks. I think this looks good to me, and we can always back it out if it causes trouble.

@bors
Copy link
Contributor

bors commented Mar 1, 2021

📌 Commit 5e306f5e0106d9e2862afd4369a88c746c6c5ce6 has been approved by Mark-Simulacrum

@bors 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 Mar 1, 2021
@bors
Copy link
Contributor

bors commented Mar 1, 2021

☔ The latest upstream changes (presumably #82654) made this pull request unmergeable. Please resolve the merge conflicts.

@bors 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 Mar 1, 2021
@the8472
Copy link
Member Author

the8472 commented Mar 1, 2021

@rustbot label -S-waiting-on-author +S-waiting-on-bors

@rustbot rustbot 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 1, 2021
@the8472
Copy link
Member Author

the8472 commented Mar 9, 2021

@Mark-Simulacrum looks like the approval got lost

@shengsheng
Copy link

@rustbot label -S-waiting-on-bors +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 12, 2021
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 22, 2021

📌 Commit 45d953d8190b68556af42b5483bbf5bb9f613104 has been approved by Mark-Simulacrum

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 22, 2021
@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 3, 2021
@JohnTitor
Copy link
Member

@bors retry

@bors 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 Apr 3, 2021
@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented Apr 4, 2021

⌛ Testing commit e312c41435d6a2b701f73102ea6272789319a031 with merge 1d0beb04badf38ce3fec6df02b5c32663571b07d...

@bors
Copy link
Contributor

bors commented Apr 4, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 4, 2021
@rust-log-analyzer

This comment has been minimized.

this avoids concurrent write attempts to the output directory
@the8472
Copy link
Member Author

the8472 commented Apr 4, 2021

I did forget to update one of the stub functions for windows, that should be fixed now. x.py doesn't have any magic to at least run check for all platforms, does it?

@Mark-Simulacrum
Copy link
Member

x.py check --target foo may work without too much effort to cross-compile, but "all platforms" isn't really a well defined set (and I'm not sure if we've pulled away from C / native dependencies enough to not need a linker even for check).

@bors r+

@bors
Copy link
Contributor

bors commented Apr 4, 2021

📌 Commit 0513ba4 has been approved by Mark-Simulacrum

@bors 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 Apr 4, 2021
@the8472
Copy link
Member Author

the8472 commented Apr 4, 2021

Nope, fails immediately

$ ./x.py check --target x86_64-pc-windows-msvc
Updating only changed submodules
Submodules updated in 0.02 seconds
    Finished dev [unoptimized + debuginfo] target(s) in 0.13s
thread 'main' panicked at '
cmake does not support Visual Studio generators.

This is likely due to it being an msys/cygwin build of cmake,
rather than the required windows version, built using MinGW
or Visual Studio.

Maybe some wine-based docker build could work.

@Mark-Simulacrum
Copy link
Member

I know we run the mingw one on the mingw-check builder (which is linux host), to try to get some coverage.

@bors
Copy link
Contributor

bors commented Apr 4, 2021

⌛ Testing commit 0513ba4 with merge f98135b...

@bors
Copy link
Contributor

bors commented Apr 4, 2021

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing f98135b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 4, 2021
@bors bors merged commit f98135b into rust-lang:master Apr 4, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants