-
Couldn't load subscription status.
- Fork 1.6k
Use nproc-1 to set parallelism for builds and tests #5939
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
base: develop
Are you sure you want to change the base?
Conversation
| cd "${BUILD_DIR}" | ||
| cmake -Donly_docs=ON .. | ||
| cmake --build . --target docs --parallel $(nproc) | ||
| cmake --build . --target docs --parallel $(( $(nproc) - 1 )) |
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.
You can let nproc do the math.
Probably use 2 to be safe.
nproc --ignore 2
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.
I love it!
https://github.com/XRPLF/clio/blob/develop/.github/actions/get-threads-number/action.yml |
I like that too, although it doesn't account for Windows. In our case all runners have nproc installed. I can create something similar. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5939 +/- ##
=======================================
Coverage 78.5% 78.5%
=======================================
Files 817 817
Lines 69017 69017
Branches 8276 8276
=======================================
Hits 54197 54197
Misses 14820 14820 🚀 New features to boost your workflow:
|
@legleux I created XRPLF/actions#17. |
|
Windoooooooooows! 👊 I can't believe all the years I've spent messing around shell math with nproc this was there the whole time? All it needs is a short flag. |
High Level Overview of Change
This change reduces the number of cores used to build and test by one.
Context of Change
Using all cores may be contributing to occasional build failures. Only the Conan profile currently specifies that the number of cores minus one should be used for the level of parallelism, but that does not affect the building and testing as those are currently set to
$(nproc).Although it would be nice to determine the number of cores to use in a single location, and then pass that value to all places that need it, it is very unwieldy to do so. Therefore this PR replaces
$(nproc)by$(( $(nproc) - 1 ))in the three locations where it is used; note that the ctest invocation will soon be changed in #5932).Type of Change
.gitignore, formatting, dropping support for older tooling)