-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[R-package] parallelize compilation in CMake-based builds #4525
Conversation
What happens on computers with fewer than 4 CPU thread? (cloud VMs, VPS, embedded devices) In R I remember we didn't allow to use more than 1 thread to compile LightGBM by default (the user had to manually add a parameter to perform multithreaded compilation), as to not overload those machines (still common in businesses). Previous solution 1, in a R configuration file:
Previous solution 2, in a prompt:
Previous solution 3, directly from R:
Is there a way to have something similar instead of an hardcoded 4 thread? |
Sure, we could expose a command-line argument that you'd pass like this: Rscript build_r.R \
--parallel-build-jobs 4 Similar to the way that other customizations have been exposed as command-line flags. Lines 44 to 51 in 86ead20
Do you think that added complexity is worth it?
Is it your opinion that the changes in #4462 should be reverted and that such customization should be exposed in a command-line argument in the Python package too? |
If this just requires adding In my opinion we should expose that parameter in the command line as |
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.
Can be merged to temporarily get Pythion and R compilation behaving similarly for the number of threads used.
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 believe passing args as a vector will be more correct, especially on Windows with processx
args Character vector, arguments to the command.
https://processx.r-lib.org/reference/run.html#arguments
Co-authored-by: Nikita Titov <[email protected]>
Sorry for taking so long to get back to this! I'd like to not merge this, and take @Laurae2 's suggestion of allowing for |
42f9a04
to
d4a8854
Compare
Ok I just updated this. Tested on my Mac, which has two physical CPUs and 4 logical CPUs (processor = "2.3 GHz Intel Core i5"). The results were what @Laurae2 said we should expect...going from single-threaded to double-threaded compilation gives a noticeable speedup, but beyond that, compilation doesn't get any faster.
@Laurae2 what do you think? |
.ci/test_r_package.sh
Outdated
@@ -112,7 +112,7 @@ cd ${BUILD_DIRECTORY} | |||
PKG_TARBALL="lightgbm_*.tar.gz" | |||
LOG_FILE_NAME="lightgbm.Rcheck/00check.log" | |||
if [[ $R_BUILD_TYPE == "cmake" ]]; then | |||
Rscript build_r.R --skip-install || exit -1 | |||
Rscript build_r.R -j2 --skip-install || exit -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.
Why j2
? We use j4
in all our CI jobs, e.g.
Line 105 in aafedd8
make -j4 || exit -1 |
Line 216 in aafedd8
make _lightgbm -j4 || exit -1 |
Refer to #1745.
Also, from the PR description:
Inspired by #4462, this proposes passing
-j4
when building the R package withCMake
, to parallelize compilation and make building faster.
#4525 (comment)
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.
Based on #4525 (comment) and the investigation I did last night (#4525 (comment)), I found that using more threads than you have available CPUs doesn't provide much speedup compared to using exactly as many threads as CPUs, but does lead to higher memory consumption.
According to https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources, the VMs we get from GitHub Actions come with a single 2-core CPU. This is why I chose -j2
.
All that said though, I think most of the benefit for CI comes from moving from single-threaded to multi-threaded, and the difference between -j2
and -j4
shouldn't be large in that environment. And I don't think the memory consumption for compilation is going to be a problem, given your point about all other jobs in this project's CI using -j4
already. I'll change these to -j4
for consistency with the other CI jobs.
Co-authored-by: Nikita Titov <[email protected]>
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.
LGTM! Thanks for making -j
flag publicly accessible.
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
Inspired by #4462, this proposes passing
-j4
when building the R package withCMake
, to parallelize compilation and make building faster.Ran the following on my Mac to benchmark and found that this did make compilation noticeably faster.
this PR
master
Notes for Reviewers
CI should pass here once #4524 is merged.