Skip to content

Use std::thread::available_parallelism for determining the default number of jobs #1447

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

Merged
merged 2 commits into from
Apr 3, 2025

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Apr 2, 2025

The function was stabilized in 1.59, while the current MSRV is 1.63.

@Kobzol Kobzol force-pushed the available-parallelism branch 2 times, most recently from 041a0a8 to 1ef467d Compare April 2, 2025 18:15
@Kobzol Kobzol force-pushed the available-parallelism branch from 1ef467d to 3c8eb2d Compare April 3, 2025 06:42
@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 3, 2025

Changed the code, let me know what do you think :) I used ok() because getting an environment variable and parsing a number have different Error types, so they don't compose well.

Copy link
Collaborator

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

Looks fine after formatting nit, thanks!

@Kobzol Kobzol force-pushed the available-parallelism branch from 3c8eb2d to b94051a Compare April 3, 2025 10:48
@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 3, 2025

Reformatted.

@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 3, 2025

CI failure is unrelated, due to stable Rust being bumped :) Might be better to pin Rust versions on CI.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Apr 3, 2025

CI failure is unrelated, due to stable Rust being bumped :) Might be better to pin Rust versions on CI.

I think they are fixed on main, so if you rebase it should be fine?

If my memory is wrong and it isn't fixed, can you please fix it?

I prefer not to pin rust versions and use latest releases

@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 3, 2025

They don't look to be solved, I'll fix them in this PR then.

Kobzol added 2 commits April 3, 2025 13:00
…number of jobs

The function was stabilized in 1.59, while the current MSRV is 1.63.
@Kobzol Kobzol force-pushed the available-parallelism branch from b94051a to 23ffd6d Compare April 3, 2025 11:23
@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 3, 2025

I fixed most of the issues, but there is a problem with using the env stdlib function in the JobServer::new function. The project wants me to use Build instead, but it isn't clear to me how to thread a Build instance to this function. Should I just do Build::new().getenv()?

@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 3, 2025

Hmm, CI is green, but it failed locally 🤷‍♂️ Good enough for me I guess, maybe I ran Clippy in a different way than was expected.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Apr 3, 2025

I fixed most of the issues, but there is a problem with using the env stdlib function in the JobServer::new function. The project wants me to use Build instead, but it isn't clear to me how to thread a Build instance to this function. Should I just do Build::new().getenv()?

Ohh it's a clippy we setup for our project, if you run cargo clippy --no-deps it should fix it

Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thank you!

@NobodyXu NobodyXu merged commit 41f7c2e into rust-lang:main Apr 3, 2025
73 checks passed
@Kobzol Kobzol deleted the available-parallelism branch April 3, 2025 11:54
@github-actions github-actions bot mentioned this pull request Apr 4, 2025
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.

3 participants