From mboxrd@z Thu Jan  1 00:00:00 1970
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com)
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
 by groups.io with SMTP; Mon, 22 Jul 2019 13:06:05 -0700
Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22])
	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
	(No client certificate requested)
	by mx1.redhat.com (Postfix) with ESMTPS id 2750430821AE;
	Mon, 22 Jul 2019 20:06:05 +0000 (UTC)
Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-23.ams2.redhat.com [10.36.117.23])
	by smtp.corp.redhat.com (Postfix) with ESMTP id 291621001B33;
	Mon, 22 Jul 2019 20:06:03 +0000 (UTC)
Subject: Re: [edk2-devel] [PATCH] OvmfPkg: enable multiprocessor builds when using build.sh
To: devel@edk2.groups.io, jordan.l.justen@intel.com,
 Ard Biesheuvel <ard.biesheuvel@linaro.org>, Rebecca Cran <rebecca@bsdio.com>
References: <20190722005816.96146-1-rebecca@bsdio.com>
 <156377947230.31344.9139528030621143554@jljusten-skl>
From: "Laszlo Ersek" <lersek@redhat.com>
Message-ID: <5313f05d-cd57-3fd0-9a44-4290b64db5a9@redhat.com>
Date: Mon, 22 Jul 2019 22:06:03 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <156377947230.31344.9139528030621143554@jljusten-skl>
X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Mon, 22 Jul 2019 20:06:05 +0000 (UTC)
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit

On 07/22/19 09:11, Jordan Justen wrote:
> Maybe a commit message tweak would be:
> 
> OvmfPkg/build.sh: enable multithreaded build by default
> 
> On 2019-07-21 17:58:16, Rebecca Cran wrote:
>> When building both BaseTools and OvmfPkg, enable multiprocessor builds,
>> using up to the number of cores available in the system. This can
>> drastically reduce build times.
>> For example, on a modern ThreadRipper system the
>> time required to build decreases from 3 minutes to 1 minute.
>>
>> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
>> ---
>>  OvmfPkg/build.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
>> index 4fcbdd2bc9..5d3a672bd2 100755
>> --- a/OvmfPkg/build.sh
>> +++ b/OvmfPkg/build.sh
>> @@ -40,7 +40,7 @@ ARCH_X64=no
>>  BUILDTARGET=DEBUG
>>  BUILD_OPTIONS=
>>  PLATFORMFILE=
>> -THREADNUMBER=1
>> +THREADNUMBER=$(getconf _NPROCESSORS_ONLN)
> 
> Based on OvmfPkg/build.sh --help, I think initializing THREADNUMBER to
> 0 might have the same effect, but not depend on getconf. Does that
> work?

My understanding is the same. The zero value causes the edk2 "build"
utility to fetch the logical CPU count with Python's
multiprocessing.cpu_count() method:

[Source/Python/build/build.py]

        if self.ThreadNumber == 0:
            try:
                self.ThreadNumber = multiprocessing.cpu_count()
            except (ImportError, NotImplementedError):
                self.ThreadNumber = 1

> I'm not sure why I defaulted this to single threaded build way back in
> 578630802e.

Your commit 578630802ee5 ("accept "-n THREADNUMBER" in OvmfPkg build
script", 2012-07-10) predates the now-default invocation of
multiprocessing.cpu_count() in "build.py".

The latter was added in 29af38b0f8f2 ("BaseTools: Enable
MAX_CONCURRENT_THREAD_NUMBER = 0 feature", 2018-01-15).

Therefore, your patch was consistent with the BaseTools behavior, at the
time you wrote it. We can consider Rebecca's patch to restore the
consistency between "OvmfPkg/build.sh" and the BaseTools default behavior.

> It looks like if we tweaked things more, and omitted adding the -n
> parameter to the build command by default, then it would use the
> Conf/target.txt value, which by default appears to also be 0, so this
> could accomplish the same thing, but also let a user set it in
> target.txt.

I assume that users prefer passing a simple command line parameter to
editing a text file. IOW I believe "THREADNUMBER=0" would be the best
solution.

Thanks
Laszlo

> 
> -Jordan
> 
>>  LAST_ARG=
>>  RUN_QEMU=no
>>  ENABLE_FLASH=no
>> -- 
>> 2.22.0
>>
> 
> 
>