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

Powershell: Use WaitForExit instead of -Wait #106216

Merged
merged 2 commits into from
Dec 29, 2022

Conversation

ChrisDenton
Copy link
Member

@ChrisDenton ChrisDenton commented Dec 28, 2022

Using the method WaitForExit instead of the parameter -Wait results in a notable speed up of the x.ps1 script (~350ms, fairly consistently).

Results:

milliseconds before: 1127.7576
milliseconds after:   779.0467

I think there are opportunities for further speed up by calling Get-Command only once with the pattern py* then filtering the returned list.

But I'll leave that for another time (or someone else).

r? @jyn514

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 28, 2022
@jyn514
Copy link
Member

jyn514 commented Dec 28, 2022

Why is this faster? 😕

thank you for looking into it!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 28, 2022

📌 Commit 874cefa has been approved by jyn514

It is now in the queue for this repository.

@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 Dec 28, 2022
@ChrisDenton
Copy link
Member Author

Why is this faster? 😕

I'm not exactly sure but the reason I tried it is because -PassThru allows us to more directly call methods on the .NET object instead of going through the overhead of whatever powershell does with the -Wait argument.

@albertlarsan68
Copy link
Member

Can it also be done for the alternate python detection just below please ?

@ChrisDenton
Copy link
Member Author

Oh good point! I missed that there were now two places. I'll create a little wrapper func to help avoid this mistake in the future.

@ChrisDenton
Copy link
Member Author

@bors r-

@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 Dec 28, 2022
@jyn514
Copy link
Member

jyn514 commented Dec 28, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Dec 28, 2022

📌 Commit 96501bd has been approved by jyn514

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 28, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 29, 2022
Powershell: Use `WaitForExit` instead of `-Wait`

Using the method `WaitForExit` instead of the parameter `-Wait` results in a notable speed up of the `x.ps1` script (~350ms, fairly consistently).

Results:
```
milliseconds before: 1127.7576
milliseconds after:   779.0467
```

I think there are opportunities for further speed up by calling `Get-Command` only once with the pattern `py*` then filtering the returned list.

But I'll leave that for another time (or someone else).

r? `@jyn514`
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 29, 2022
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#106208 (Make trait/impl `where` clause mismatch on region error a bit more actionable)
 - rust-lang#106216 (Powershell: Use `WaitForExit` instead of `-Wait`)
 - rust-lang#106217 (rustdoc: remove unnecessary `.tooltip::after { text-align: center }`)
 - rust-lang#106218 (Migrate css var scraped examples)
 - rust-lang#106221 (Rename `Rptr` to `Ref` in AST and HIR)
 - rust-lang#106223 (On unsized locals with explicit types suggest `&`)
 - rust-lang#106225 (Remove CraftSpider from review rotation)
 - rust-lang#106229 (update Miri)
 - rust-lang#106242 (Detect diff markers in the parser)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e782314 into rust-lang:master Dec 29, 2022
@rustbot rustbot added this to the 1.68.0 milestone Dec 29, 2022
@ChrisDenton ChrisDenton deleted the ps-go-faster branch December 29, 2022 19:57
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#106208 (Make trait/impl `where` clause mismatch on region error a bit more actionable)
 - rust-lang#106216 (Powershell: Use `WaitForExit` instead of `-Wait`)
 - rust-lang#106217 (rustdoc: remove unnecessary `.tooltip::after { text-align: center }`)
 - rust-lang#106218 (Migrate css var scraped examples)
 - rust-lang#106221 (Rename `Rptr` to `Ref` in AST and HIR)
 - rust-lang#106223 (On unsized locals with explicit types suggest `&`)
 - rust-lang#106225 (Remove CraftSpider from review rotation)
 - rust-lang#106229 (update Miri)
 - rust-lang#106242 (Detect diff markers in the parser)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants