* [Tarantool-patches] [PATCH] build: configure parallel jobs @ 2022-07-01 14:36 Sergey Bronnikov via Tarantool-patches 2022-07-04 9:52 ` Sergey Kaplun via Tarantool-patches ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2022-07-01 14:36 UTC (permalink / raw) To: tarantool-patches lua-Harness and tarantool testsuites uses a prove(1) for running tests. prove(1) allows to run tests in parallel with option "--jobs" [1]. In CMake it is not possible to get a number of parallel jobs in CMake passed by user with option "-j", but it allows to pass a number of parallel jobs with environment variable CMAKE_BUILD_PARALLEL_LEVEL [2] on configuration phase. We use a value set by that environment variable and set it to a number of CPU threads when it was not specified by user. Number of CPU threads detected using builtin CMake function [3]. NOTE: CMAKE_BUILD_PARALLEL_LEVEL has been added in a version 3.12. 1. https://perldoc.perl.org/prove 2. https://cmake.org/cmake/help/latest/envvar/CMAKE_BUILD_PARALLEL_LEVEL.html 3. https://cmake.org/cmake/help/latest/module/ProcessorCount.html --- Branch: https://github.com/tarantool/luajit/tree/ligurio/prove-in-parallel CI: https://github.com/tarantool/luajit/commit/62acf609d9e1ea43d95cb27213e1b8f1331b57a7 test/CMakeLists.txt | 13 +++++++++++++ test/lua-Harness-tests/CMakeLists.txt | 1 + test/tarantool-tests/CMakeLists.txt | 1 + 3 files changed, 15 insertions(+) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index ba25af54..fc4ffc51 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -43,6 +43,19 @@ endif() set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]") separate_arguments(LUAJIT_TEST_COMMAND) +set(CMAKE_BUILD_PARALLEL_LEVEL $ENV{CMAKE_BUILD_PARALLEL_LEVEL}) +if(NOT CMAKE_BUILD_PARALLEL_LEVEL) + include(ProcessorCount) + ProcessorCount(N) + # Function ProcessorCount() is guaranteed to return a positive integer (>=1) + # if it succeeds. It returns 0 if there's a problem determining the processor + # count. + if(NOT N EQUAL 0) + set(CMAKE_BUILD_PARALLEL_LEVEL ${N}) + endif() +endif() +message(STATUS "Using CMAKE_BUILD_PARALLEL_LEVEL: ${CMAKE_BUILD_PARALLEL_LEVEL}") + add_subdirectory(LuaJIT-tests) add_subdirectory(PUC-Rio-Lua-5.1-tests) add_subdirectory(lua-Harness-tests) diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt index ec08a19b..12476171 100644 --- a/test/lua-Harness-tests/CMakeLists.txt +++ b/test/lua-Harness-tests/CMakeLists.txt @@ -28,6 +28,7 @@ add_custom_command(TARGET lua-Harness-tests LUA_PATH="${LUA_PATH}\;" ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR} --exec '${LUAJIT_TEST_COMMAND} -l profile_luajit21' + --jobs ${CMAKE_BUILD_PARALLEL_LEVEL} ${LUA_TEST_FLAGS} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt index 5708822e..ecda2e63 100644 --- a/test/tarantool-tests/CMakeLists.txt +++ b/test/tarantool-tests/CMakeLists.txt @@ -136,6 +136,7 @@ add_custom_command(TARGET tarantool-tests ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR} --exec 'env ${LUA_TEST_ENV_MORE} ${LUAJIT_TEST_COMMAND}' --ext ${LUA_TEST_SUFFIX} + --jobs ${CMAKE_BUILD_PARALLEL_LEVEL} ${LUA_TEST_FLAGS} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH] build: configure parallel jobs 2022-07-01 14:36 [Tarantool-patches] [PATCH] build: configure parallel jobs Sergey Bronnikov via Tarantool-patches @ 2022-07-04 9:52 ` Sergey Kaplun via Tarantool-patches 2022-07-04 10:06 ` Sergey Bronnikov via Tarantool-patches 2022-07-04 10:12 ` Igor Munkin via Tarantool-patches 2022-07-13 10:10 ` Igor Munkin via Tarantool-patches 2 siblings, 1 reply; 8+ messages in thread From: Sergey Kaplun via Tarantool-patches @ 2022-07-04 9:52 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: tarantool-patches Hi, Sergey! Thanks for the patch! LGTM, except a few nits regarding the commit message. On 01.07.22, Sergey Bronnikov wrote: > lua-Harness and tarantool testsuites uses a prove(1) for running tests. Typo: s/testsuites/test suites/ > prove(1) allows to run tests in parallel with option "--jobs" [1]. Typo: s/option/the option/ > > In CMake it is not possible to get a number of parallel jobs in CMake > passed by user with option "-j", but it allows to pass a number of > parallel jobs with environment variable CMAKE_BUILD_PARALLEL_LEVEL [2] > on configuration phase. We use a value set by that environment variable > and set it to a number of CPU threads when it was not specified by user. Typo: s/user/the user/ > Number of CPU threads detected using builtin CMake function [3]. > > NOTE: CMAKE_BUILD_PARALLEL_LEVEL has been added in a version 3.12. > > 1. https://perldoc.perl.org/prove > 2. https://cmake.org/cmake/help/latest/envvar/CMAKE_BUILD_PARALLEL_LEVEL.html > 3. https://cmake.org/cmake/help/latest/module/ProcessorCount.html > --- > > Branch: https://github.com/tarantool/luajit/tree/ligurio/prove-in-parallel > CI: https://github.com/tarantool/luajit/commit/62acf609d9e1ea43d95cb27213e1b8f1331b57a7 > > test/CMakeLists.txt | 13 +++++++++++++ > test/lua-Harness-tests/CMakeLists.txt | 1 + > test/tarantool-tests/CMakeLists.txt | 1 + > 3 files changed, 15 insertions(+) > <snipped> > -- > 2.25.1 > -- Best regards, Sergey Kaplun ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH] build: configure parallel jobs 2022-07-04 9:52 ` Sergey Kaplun via Tarantool-patches @ 2022-07-04 10:06 ` Sergey Bronnikov via Tarantool-patches 0 siblings, 0 replies; 8+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2022-07-04 10:06 UTC (permalink / raw) To: Sergey Kaplun; +Cc: tarantool-patches Thanks for review! Fixed and force-pushed. Sergey On 04.07.2022 12:52, Sergey Kaplun wrote: > Hi, Sergey! > > Thanks for the patch! > > LGTM, except a few nits regarding the commit message. > > On 01.07.22, Sergey Bronnikov wrote: >> lua-Harness and tarantool testsuites uses a prove(1) for running tests. > Typo: s/testsuites/test suites/ > >> prove(1) allows to run tests in parallel with option "--jobs" [1]. > Typo: s/option/the option/ > >> In CMake it is not possible to get a number of parallel jobs in CMake >> passed by user with option "-j", but it allows to pass a number of >> parallel jobs with environment variable CMAKE_BUILD_PARALLEL_LEVEL [2] >> on configuration phase. We use a value set by that environment variable >> and set it to a number of CPU threads when it was not specified by user. > Typo: s/user/the user/ > >> Number of CPU threads detected using builtin CMake function [3]. >> >> NOTE: CMAKE_BUILD_PARALLEL_LEVEL has been added in a version 3.12. >> >> 1. https://perldoc.perl.org/prove >> 2. https://cmake.org/cmake/help/latest/envvar/CMAKE_BUILD_PARALLEL_LEVEL.html >> 3. https://cmake.org/cmake/help/latest/module/ProcessorCount.html >> --- >> >> Branch: https://github.com/tarantool/luajit/tree/ligurio/prove-in-parallel >> CI: https://github.com/tarantool/luajit/commit/62acf609d9e1ea43d95cb27213e1b8f1331b57a7 >> >> test/CMakeLists.txt | 13 +++++++++++++ >> test/lua-Harness-tests/CMakeLists.txt | 1 + >> test/tarantool-tests/CMakeLists.txt | 1 + >> 3 files changed, 15 insertions(+) >> > <snipped> > >> -- >> 2.25.1 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH] build: configure parallel jobs 2022-07-01 14:36 [Tarantool-patches] [PATCH] build: configure parallel jobs Sergey Bronnikov via Tarantool-patches 2022-07-04 9:52 ` Sergey Kaplun via Tarantool-patches @ 2022-07-04 10:12 ` Igor Munkin via Tarantool-patches 2022-07-04 14:21 ` Sergey Bronnikov via Tarantool-patches 2022-07-13 10:10 ` Igor Munkin via Tarantool-patches 2 siblings, 1 reply; 8+ messages in thread From: Igor Munkin via Tarantool-patches @ 2022-07-04 10:12 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: tarantool-patches Sergey, Thanks for the patch! Please consider my several nits below. On 01.07.22, Sergey Bronnikov wrote: > lua-Harness and tarantool testsuites uses a prove(1) for running tests. Typo: s/testsuites/test suites/. Typo: s/a prove/prove/ since it's program name. > prove(1) allows to run tests in parallel with option "--jobs" [1]. Minor: It's worth to mention that this option requires an argument, so we can't rely on the default behaviour (since there is none). > > In CMake it is not possible to get a number of parallel jobs in CMake Typo: Let's leave only the latter "in CMake" occurrence. > passed by user with option "-j", but it allows to pass a number of Typo: s/"-j"/"--parallel"/, am I right? > parallel jobs with environment variable CMAKE_BUILD_PARALLEL_LEVEL [2] > on configuration phase. We use a value set by that environment variable Minor: It's better (but not obligatory) use the passive voice here, to get something like "The value set by that environment variable is used as a number of CPU threads when it was not specified by user." > and set it to a number of CPU threads when it was not specified by user. > Number of CPU threads detected using builtin CMake function [3]. > > NOTE: CMAKE_BUILD_PARALLEL_LEVEL has been added in a version 3.12. Minor: It's also worth to mention whether this envvar is just ignored or the error is raised. > > 1. https://perldoc.perl.org/prove > 2. https://cmake.org/cmake/help/latest/envvar/CMAKE_BUILD_PARALLEL_LEVEL.html > 3. https://cmake.org/cmake/help/latest/module/ProcessorCount.html > --- > > Branch: https://github.com/tarantool/luajit/tree/ligurio/prove-in-parallel > CI: https://github.com/tarantool/luajit/commit/62acf609d9e1ea43d95cb27213e1b8f1331b57a7 > > test/CMakeLists.txt | 13 +++++++++++++ > test/lua-Harness-tests/CMakeLists.txt | 1 + > test/tarantool-tests/CMakeLists.txt | 1 + > 3 files changed, 15 insertions(+) > > diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt > index ba25af54..fc4ffc51 100644 > --- a/test/CMakeLists.txt > +++ b/test/CMakeLists.txt > @@ -43,6 +43,19 @@ endif() > set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]") > separate_arguments(LUAJIT_TEST_COMMAND) > I guess all code below can be moved to a function somewhere in cmake/ and its usage -- to the root CMakeLists.txt. Thoughts? > +set(CMAKE_BUILD_PARALLEL_LEVEL $ENV{CMAKE_BUILD_PARALLEL_LEVEL}) > +if(NOT CMAKE_BUILD_PARALLEL_LEVEL) > + include(ProcessorCount) > + ProcessorCount(N) > + # Function ProcessorCount() is guaranteed to return a positive integer (>=1) > + # if it succeeds. It returns 0 if there's a problem determining the processor > + # count. > + if(NOT N EQUAL 0) > + set(CMAKE_BUILD_PARALLEL_LEVEL ${N}) I believe we need else branch here to set CMAKE_BUILD_PARALLEL_LEVEL, if ProcessorCount yields zero. > + endif() > +endif() > +message(STATUS "Using CMAKE_BUILD_PARALLEL_LEVEL: ${CMAKE_BUILD_PARALLEL_LEVEL}") > + > add_subdirectory(LuaJIT-tests) > add_subdirectory(PUC-Rio-Lua-5.1-tests) > add_subdirectory(lua-Harness-tests) <snipped> > -- > 2.25.1 > -- Best regards, IM ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH] build: configure parallel jobs 2022-07-04 10:12 ` Igor Munkin via Tarantool-patches @ 2022-07-04 14:21 ` Sergey Bronnikov via Tarantool-patches 2022-07-05 21:22 ` Igor Munkin via Tarantool-patches 0 siblings, 1 reply; 8+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2022-07-04 14:21 UTC (permalink / raw) To: Igor Munkin; +Cc: tarantool-patches Igor, thanks for review! On 04.07.2022 13:12, Igor Munkin wrote: > Sergey, > > Thanks for the patch! Please consider my several nits below. > > On 01.07.22, Sergey Bronnikov wrote: >> lua-Harness and tarantool testsuites uses a prove(1) for running tests. > Typo: s/testsuites/test suites/. Already spotted by Sergey and fixed. > > Typo: s/a prove/prove/ since it's program name. Fixed. > >> prove(1) allows to run tests in parallel with option "--jobs" [1]. > Minor: It's worth to mention that this option requires an argument, so > we can't rely on the default behaviour (since there is none). Added. >> In CMake it is not possible to get a number of parallel jobs in CMake > Typo: Let's leave only the latter "in CMake" occurrence. Fixed. > >> passed by user with option "-j", but it allows to pass a number of > Typo: s/"-j"/"--parallel"/, am I right? Yes and no :) "-j" is for build tools like make and ninja and "--parallel" is for cmake. Updated to make it clear. > >> parallel jobs with environment variable CMAKE_BUILD_PARALLEL_LEVEL [2] >> on configuration phase. We use a value set by that environment variable > Minor: It's better (but not obligatory) use the passive voice here, to > get something like "The value set by that environment variable is used > as a number of CPU threads when it was not specified by user." > Updated, thanks. >> and set it to a number of CPU threads when it was not specified by user. >> Number of CPU threads detected using builtin CMake function [3]. >> >> NOTE: CMAKE_BUILD_PARALLEL_LEVEL has been added in a version 3.12. > Minor: It's also worth to mention whether this envvar is just ignored or > the error is raised. Ignore. Added it to the sentence. >> 1. https://perldoc.perl.org/prove >> 2. https://cmake.org/cmake/help/latest/envvar/CMAKE_BUILD_PARALLEL_LEVEL.html >> 3. https://cmake.org/cmake/help/latest/module/ProcessorCount.html >> --- >> >> Branch: https://github.com/tarantool/luajit/tree/ligurio/prove-in-parallel >> CI: https://github.com/tarantool/luajit/commit/62acf609d9e1ea43d95cb27213e1b8f1331b57a7 >> >> test/CMakeLists.txt | 13 +++++++++++++ >> test/lua-Harness-tests/CMakeLists.txt | 1 + >> test/tarantool-tests/CMakeLists.txt | 1 + >> 3 files changed, 15 insertions(+) >> >> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt >> index ba25af54..fc4ffc51 100644 >> --- a/test/CMakeLists.txt >> +++ b/test/CMakeLists.txt >> @@ -43,6 +43,19 @@ endif() >> set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]") >> separate_arguments(LUAJIT_TEST_COMMAND) >> > I guess all code below can be moved to a function somewhere in cmake/ > and its usage -- to the root CMakeLists.txt. Thoughts? Agree. Initial idea was to parallelize tests only, but with CMAKE_BUILD_PARALLEL_LEVEL it works for building process too. Moved hunk to the root CMakeLists.txt right after building compile options and flags and before adding subdirectories, because code in lower levels can depend on this option. >> +set(CMAKE_BUILD_PARALLEL_LEVEL $ENV{CMAKE_BUILD_PARALLEL_LEVEL}) >> +if(NOT CMAKE_BUILD_PARALLEL_LEVEL) >> + include(ProcessorCount) >> + ProcessorCount(N) >> + # Function ProcessorCount() is guaranteed to return a positive integer (>=1) >> + # if it succeeds. It returns 0 if there's a problem determining the processor >> + # count. >> + if(NOT N EQUAL 0) >> + set(CMAKE_BUILD_PARALLEL_LEVEL ${N}) > I believe we need else branch here to set CMAKE_BUILD_PARALLEL_LEVEL, if > ProcessorCount yields zero. I would do it without additional branch. Like this: set(CMAKE_BUILD_PARALLEL_LEVEL 1) if(NOT N EQUAL 0) set(CMAKE_BUILD_PARALLEL_LEVEL ${N}) endif() Are you ok with this? > >> + endif() >> +endif() >> +message(STATUS "Using CMAKE_BUILD_PARALLEL_LEVEL: ${CMAKE_BUILD_PARALLEL_LEVEL}") >> + >> add_subdirectory(LuaJIT-tests) >> add_subdirectory(PUC-Rio-Lua-5.1-tests) >> add_subdirectory(lua-Harness-tests) > <snipped> > >> -- >> 2.25.1 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH] build: configure parallel jobs 2022-07-04 14:21 ` Sergey Bronnikov via Tarantool-patches @ 2022-07-05 21:22 ` Igor Munkin via Tarantool-patches 2022-07-06 10:32 ` Sergey Bronnikov via Tarantool-patches 0 siblings, 1 reply; 8+ messages in thread From: Igor Munkin via Tarantool-patches @ 2022-07-05 21:22 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: tarantool-patches Sergey, Thanks for the fixes. LGTM, with a couple more nits. On 04.07.22, Sergey Bronnikov wrote: > Igor, thanks for review! > > On 04.07.2022 13:12, Igor Munkin wrote: > > Sergey, > > > > Thanks for the patch! Please consider my several nits below. > > > > On 01.07.22, Sergey Bronnikov wrote: <snipped> > >> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt > >> index ba25af54..fc4ffc51 100644 > >> --- a/test/CMakeLists.txt > >> +++ b/test/CMakeLists.txt > >> @@ -43,6 +43,19 @@ endif() > >> set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]") > >> separate_arguments(LUAJIT_TEST_COMMAND) > >> > > I guess all code below can be moved to a function somewhere in cmake/ > > and its usage -- to the root CMakeLists.txt. Thoughts? > > Agree. Initial idea was to parallelize tests only, > > but with CMAKE_BUILD_PARALLEL_LEVEL it works for building process too. > > Moved hunk to the root CMakeLists.txt right after building compile options > > and flags and before adding subdirectories, because code in lower levels > can depend on this option. I still believe this should be moved to a separate module in cmake/ directory (like it's done for SetVersion). Please, also move it's usage to "Fine-tuning cmake environment" section in root CMakeLists.txt to group most of includes there. > > >> +set(CMAKE_BUILD_PARALLEL_LEVEL $ENV{CMAKE_BUILD_PARALLEL_LEVEL}) > >> +if(NOT CMAKE_BUILD_PARALLEL_LEVEL) > >> + include(ProcessorCount) > >> + ProcessorCount(N) > >> + # Function ProcessorCount() is guaranteed to return a positive integer (>=1) > >> + # if it succeeds. It returns 0 if there's a problem determining the processor > >> + # count. > >> + if(NOT N EQUAL 0) > >> + set(CMAKE_BUILD_PARALLEL_LEVEL ${N}) > > I believe we need else branch here to set CMAKE_BUILD_PARALLEL_LEVEL, if > > ProcessorCount yields zero. > > I would do it without additional branch. Like this: > > set(CMAKE_BUILD_PARALLEL_LEVEL 1) > if(NOT N EQUAL 0) > set(CMAKE_BUILD_PARALLEL_LEVEL ${N}) > endif() > > Are you ok with this? Great, thanks! > > <snipped> > >> -- > >> 2.25.1 > >> All in all, if you've introduced the machinery to control parallel level of CMake jobs, we can also fix our workflows in a separate commit by changing --parallel parameter with CMAKE_BUILD_PARALLEL_LEVEL envvar. So the following approach... | $ cmake . | $ cmake . --build --parallel $(nproc) --target test ... is replaced with the next one... | $ CMAKE_BUILD_PARALLEL_LEVEL=$(nproc) cmake . | $ cmake . --build --parallel ... considering the comment[1] in CMAKE_BUILD_PARALLEL_LEVEL manual. [1]: https://cmake.org/cmake/help/latest/envvar/CMAKE_BUILD_PARALLEL_LEVEL.html -- Best regards, IM ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH] build: configure parallel jobs 2022-07-05 21:22 ` Igor Munkin via Tarantool-patches @ 2022-07-06 10:32 ` Sergey Bronnikov via Tarantool-patches 0 siblings, 0 replies; 8+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2022-07-06 10:32 UTC (permalink / raw) To: Igor Munkin; +Cc: tarantool-patches Igor, thanks for review! On 06.07.2022 00:22, Igor Munkin wrote: > Sergey, > > Thanks for the fixes. LGTM, with a couple more nits. > > On 04.07.22, Sergey Bronnikov wrote: >> Igor, thanks for review! >> >> On 04.07.2022 13:12, Igor Munkin wrote: >>> Sergey, >>> >>> Thanks for the patch! Please consider my several nits below. >>> >>> On 01.07.22, Sergey Bronnikov wrote: > <snipped> > >>>> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt >>>> index ba25af54..fc4ffc51 100644 >>>> --- a/test/CMakeLists.txt >>>> +++ b/test/CMakeLists.txt >>>> @@ -43,6 +43,19 @@ endif() >>>> set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]") >>>> separate_arguments(LUAJIT_TEST_COMMAND) >>>> >>> I guess all code below can be moved to a function somewhere in cmake/ >>> and its usage -- to the root CMakeLists.txt. Thoughts? >> Agree. Initial idea was to parallelize tests only, >> >> but with CMAKE_BUILD_PARALLEL_LEVEL it works for building process too. >> >> Moved hunk to the root CMakeLists.txt right after building compile options >> >> and flags and before adding subdirectories, because code in lower levels >> can depend on this option. > I still believe this should be moved to a separate module in cmake/ > directory (like it's done for SetVersion). Please, also move it's usage > to "Fine-tuning cmake environment" section in root CMakeLists.txt to > group most of includes there. Moved to a separate cmake module cmake/SetBuildParallelLevel.cmake. diff --git a/CMakeLists.txt b/CMakeLists.txt index 659ee879..1a7f62aa 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,10 +30,13 @@ endif() set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") include(LuaJITUtils) +include(SetBuildParallelLevel) include(SetVersion) # --- Variables to be exported to child scopes --------------------------------- +SetBuildParallelLevel() +message(STATUS "[CMakeBuildParallelLevel] CMAKE_BUILD_PARALLEL_LEVEL is ${CMAKE_BUILD_PARALLEL_LEVEL}") SetVersion( LUAJIT_VERSION LUAJIT_VERSION_MAJOR diff --git a/cmake/SetBuildParallelLevel.cmake b/cmake/SetBuildParallelLevel.cmake new file mode 100644 index 00000000..97d7b7b0 --- /dev/null +++ b/cmake/SetBuildParallelLevel.cmake @@ -0,0 +1,14 @@ +function(SetBuildParallelLevel) + set(CMAKE_BUILD_PARALLEL_LEVEL $ENV{CMAKE_BUILD_PARALLEL_LEVEL} PARENT_SCOPE) + if(NOT CMAKE_BUILD_PARALLEL_LEVEL) + set(CMAKE_BUILD_PARALLEL_LEVEL 1 PARENT_SCOPE) + include(ProcessorCount) + ProcessorCount(N) + # Function ProcessorCount() is guaranteed to return a positive integer (>=1) + # if it succeeds. It returns 0 if there's a problem determining the processor + # count. + if(NOT N EQUAL 0) + set(CMAKE_BUILD_PARALLEL_LEVEL ${N} PARENT_SCOPE) + endif() + endif() +endfunction() diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt index ec08a19b..12476171 100644 --- a/test/lua-Harness-tests/CMakeLists.txt +++ b/test/lua-Harness-tests/CMakeLists.txt @@ -28,6 +28,7 @@ add_custom_command(TARGET lua-Harness-tests LUA_PATH="${LUA_PATH}\;" ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR} --exec '${LUAJIT_TEST_COMMAND} -l profile_luajit21' + --jobs ${CMAKE_BUILD_PARALLEL_LEVEL} ${LUA_TEST_FLAGS} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt index 5708822e..ecda2e63 100644 --- a/test/tarantool-tests/CMakeLists.txt +++ b/test/tarantool-tests/CMakeLists.txt @@ -136,6 +136,7 @@ add_custom_command(TARGET tarantool-tests ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR} --exec 'env ${LUA_TEST_ENV_MORE} ${LUAJIT_TEST_COMMAND}' --ext ${LUA_TEST_SUFFIX} + --jobs ${CMAKE_BUILD_PARALLEL_LEVEL} ${LUA_TEST_FLAGS} WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ) >>>> +set(CMAKE_BUILD_PARALLEL_LEVEL $ENV{CMAKE_BUILD_PARALLEL_LEVEL}) >>>> +if(NOT CMAKE_BUILD_PARALLEL_LEVEL) >>>> + include(ProcessorCount) >>>> + ProcessorCount(N) >>>> + # Function ProcessorCount() is guaranteed to return a positive integer (>=1) >>>> + # if it succeeds. It returns 0 if there's a problem determining the processor >>>> + # count. >>>> + if(NOT N EQUAL 0) >>>> + set(CMAKE_BUILD_PARALLEL_LEVEL ${N}) >>> I believe we need else branch here to set CMAKE_BUILD_PARALLEL_LEVEL, if >>> ProcessorCount yields zero. >> I would do it without additional branch. Like this: >> >> set(CMAKE_BUILD_PARALLEL_LEVEL 1) >> if(NOT N EQUAL 0) >> set(CMAKE_BUILD_PARALLEL_LEVEL ${N}) >> endif() >> >> Are you ok with this? > Great, thanks! > >> > <snipped> > >>>> -- >>>> 2.25.1 >>>> > All in all, if you've introduced the machinery to control parallel level > of CMake jobs, we can also fix our workflows in a separate commit by > changing --parallel parameter with CMAKE_BUILD_PARALLEL_LEVEL envvar. So > the following approach... > | $ cmake . > | $ cmake . --build --parallel $(nproc) --target test > ... is replaced with the next one... > | $ CMAKE_BUILD_PARALLEL_LEVEL=$(nproc) cmake . > | $ cmake . --build --parallel > ... considering the comment[1] in CMAKE_BUILD_PARALLEL_LEVEL manual. > > [1]: https://cmake.org/cmake/help/latest/envvar/CMAKE_BUILD_PARALLEL_LEVEL.html Added in a separate commit. Also added a commit with fixup with replacing hw.ncpu to hw.logicalcpu. > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH] build: configure parallel jobs 2022-07-01 14:36 [Tarantool-patches] [PATCH] build: configure parallel jobs Sergey Bronnikov via Tarantool-patches 2022-07-04 9:52 ` Sergey Kaplun via Tarantool-patches 2022-07-04 10:12 ` Igor Munkin via Tarantool-patches @ 2022-07-13 10:10 ` Igor Munkin via Tarantool-patches 2 siblings, 0 replies; 8+ messages in thread From: Igor Munkin via Tarantool-patches @ 2022-07-13 10:10 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: tarantool-patches Sergey, I've checked the patch into all long-term branches in tarantool/luajit and bumped a new version in master, 2.10 and 1.10. On 01.07.22, Sergey Bronnikov wrote: > lua-Harness and tarantool testsuites uses a prove(1) for running tests. > prove(1) allows to run tests in parallel with option "--jobs" [1]. > > In CMake it is not possible to get a number of parallel jobs in CMake > passed by user with option "-j", but it allows to pass a number of > parallel jobs with environment variable CMAKE_BUILD_PARALLEL_LEVEL [2] > on configuration phase. We use a value set by that environment variable > and set it to a number of CPU threads when it was not specified by user. > Number of CPU threads detected using builtin CMake function [3]. > > NOTE: CMAKE_BUILD_PARALLEL_LEVEL has been added in a version 3.12. > > 1. https://perldoc.perl.org/prove > 2. https://cmake.org/cmake/help/latest/envvar/CMAKE_BUILD_PARALLEL_LEVEL.html > 3. https://cmake.org/cmake/help/latest/module/ProcessorCount.html > --- > > Branch: https://github.com/tarantool/luajit/tree/ligurio/prove-in-parallel > CI: https://github.com/tarantool/luajit/commit/62acf609d9e1ea43d95cb27213e1b8f1331b57a7 > > test/CMakeLists.txt | 13 +++++++++++++ > test/lua-Harness-tests/CMakeLists.txt | 1 + > test/tarantool-tests/CMakeLists.txt | 1 + > 3 files changed, 15 insertions(+) > <snipped> > -- > 2.25.1 > -- Best regards, IM ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-07-13 10:10 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-01 14:36 [Tarantool-patches] [PATCH] build: configure parallel jobs Sergey Bronnikov via Tarantool-patches 2022-07-04 9:52 ` Sergey Kaplun via Tarantool-patches 2022-07-04 10:06 ` Sergey Bronnikov via Tarantool-patches 2022-07-04 10:12 ` Igor Munkin via Tarantool-patches 2022-07-04 14:21 ` Sergey Bronnikov via Tarantool-patches 2022-07-05 21:22 ` Igor Munkin via Tarantool-patches 2022-07-06 10:32 ` Sergey Bronnikov via Tarantool-patches 2022-07-13 10:10 ` Igor Munkin via Tarantool-patches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox