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 869316ECE3; Mon, 4 Jul 2022 17:21:09 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 869316ECE3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1656944469; bh=df+wILDBjVrh2gzVXcQU9E+RPm4LyPOizNbsJ3ONrCE=; 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=x6c3jeBYIH1uPryJfn1iVGUhxTOLaoqbjbDaKG4ndxzwqoWwfMofDpQchnSYkjcKS NcttLkYxL7vQWp9ttgWExXMM+XCDzHCbzvAPZbsFwA+0FSxziLs3RZkndj2st24b5z pMad2vYyfn3I3ncwfAb6CIocOp2k4AOSkJcfrglk= Received: from mail-lf1-f50.google.com (mail-lf1-f50.google.com [209.85.167.50]) (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 A80F96ECE3 for ; Mon, 4 Jul 2022 17:21:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A80F96ECE3 Received: by mail-lf1-f50.google.com with SMTP id e12so16046261lfr.6 for ; Mon, 04 Jul 2022 07:21:08 -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=/uWBg0Myv6LAjNq4lnA8M5iJtsMLldq2Wh2n5YSsYUI=; b=DeRh3oLo07uPXzeSiyZ+PijA8hNS6rHDtkSk9GBLYFHtZKn4Maa/XvC5YZO4mAdM6j mkllDLDJ+BCMsFG1cjtUQBrUr2vbmoD9TaJjXIjzwaDGoQr4aNfq01zE3qrLGZy/GCmZ hx260g5a9s0/b1xhNY2DDeyd2cBgG7rpuvdvRNIbsM7iXXHQEoV+rTMZJAIBgHb2g1hU OzqyN4F9Z+PuDM00V5ExX+nsspCWeKUK2S9g19Y4PELtLe9uWOWY4sA2iPE761ncs6Sa Mh2gE/HnxRBAyObEUl2squ9txCn9xnvm4nkcXO2L6PVw7GfkJJMwZMnQAxTdP8LlQMPE qq8w== X-Gm-Message-State: AJIora/LP6s+iermDEZCGpspyJhop4cfjrsdM92Xkr6IIIjEi74DjHSP SXz6x6aoUrvRnqdNXDBgimA= X-Google-Smtp-Source: AGRyM1s64uXDRBenc5p3AXKY4RREgQPEV8XnJLSH7KxhxuDpzx7md358xJ+mfJ9YPUlIO/dpqvNHEQ== X-Received: by 2002:a05:6512:1088:b0:481:2363:ee0b with SMTP id j8-20020a056512108800b004812363ee0bmr18714989lfg.55.1656944467890; Mon, 04 Jul 2022 07:21:07 -0700 (PDT) Received: from [192.168.43.168] ([178.176.74.241]) by smtp.gmail.com with ESMTPSA id g2-20020a056512118200b0047f701f6d09sm5157148lfr.184.2022.07.04.07.21.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 04 Jul 2022 07:21:07 -0700 (PDT) Message-ID: <721f8709-cf2b-b2df-0672-cfd6f9cf1efb@gmail.com> Date: Mon, 4 Jul 2022 17:21:05 +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> 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 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) > > >> -- >> 2.25.1 >>