From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 6950B6ECE3; Wed, 6 Jul 2022 13:32:13 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6950B6ECE3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1657103533; bh=6OPapHXa8Sf9m4sCha9Ym0PqKrtd3Z5rn6+0eI2bnao=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=fIv8GdtfQ4phB0J38s90ZqgeQBSTnCQyO79XPrt2exSuSEN/vZnOsVYgraQf3edXK X1LWn+2p5BTCcV93+IDBJx9Hur8biOIB0aNtAjsZ+NOxiP4DnAdGqVeadpvgi5KcBs ymOtzpWQB4jPc+xnUsC54YEh3MBwaw118bgFmC2g= Received: from mail-lf1-f54.google.com (mail-lf1-f54.google.com [209.85.167.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 125486ECE3 for ; Wed, 6 Jul 2022 13:32:11 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 125486ECE3 Received: by mail-lf1-f54.google.com with SMTP id z25so7598092lfr.2 for ; Wed, 06 Jul 2022 03:32:11 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=ifGcOdrSLYFTWv9YZa4RPzxAQdvscKGWPSjQP+UwHyA=; b=sE22xOlupFZL5QhilWeuBOpcNN9qVM2p6b8m6wLUXv9GAR0KexqMRpMybirtGif6DQ E/NRUKpNKM+p447JkckzqDqoH2SrRJpHr55iwZTA/p4fzhXpFe/mJZMCLyVMhjpJLWdh PIFz2rj133yBeTZ6NUF3H2elwE/Rui4zzkf+3BxgdHjAYUzlKm9CPcMxO7LtmN6LA2Y4 XD8j0wpNM2GV+85JO/2u8If69dfOjF+iRRYDHNli0j0FaLLlvMv8t2CeFNh6Iz+82WZJ LEnpX6OZKca391eH5s2Z6cK/WfSRxWDrfIawjCsGrYj2Bq4uRm48cxHNAtLKJF1nK3PP Dd1w== X-Gm-Message-State: AJIora+q+agjuGfHwI5GDoWy5zudVvBbneRTnMwYiu/XyFv2J9KfDZju p9uskHPXQf004zmCCVcAcQpo+MH1dcc= X-Google-Smtp-Source: AGRyM1vWkCnPlXGAeXdEESDM5qofYi/TOTMtAq21vQcvkEx6el3NK4v7xq8iJXb97LWBLnlZhwD2jg== X-Received: by 2002:a05:6512:ea3:b0:485:a515:5e27 with SMTP id bi35-20020a0565120ea300b00485a5155e27mr2183070lfb.257.1657103530414; Wed, 06 Jul 2022 03:32:10 -0700 (PDT) Received: from [192.168.43.168] ([178.176.74.160]) by smtp.gmail.com with ESMTPSA id v1-20020a056512348100b004793b9c2c12sm610359lfr.124.2022.07.06.03.32.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 06 Jul 2022 03:32:09 -0700 (PDT) Message-ID: <94c11226-df97-ffc2-4d86-b474319335f3@gmail.com> Date: Wed, 6 Jul 2022 13:32:08 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Content-Language: en-US To: Igor Munkin References: <62acf609d9e1ea43d95cb27213e1b8f1331b57a7.1656685987.git.sergeyb@tarantool.org> <721f8709-cf2b-b2df-0672-cfd6f9cf1efb@gmail.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Tarantool-patches] [PATCH] build: configure parallel jobs X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "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: > > >>>> 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! > >> > > >>>> -- >>>> 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. >