[Tarantool-patches] [PATCH] build: configure parallel jobs
Sergey Bronnikov
estetus at gmail.com
Wed Jul 6 13:32:08 MSK 2022
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.
>
More information about the Tarantool-patches
mailing list