Re: [edk2-devel] [Patch 0/9] Enable multiple process AutoGen
On 07/22/19 11:11, Feng, Bob C wrote: > Hi Laszlo, > > Thanks for your detailed testing and comments. > > I send out patch serial V2 to resolve comment 1#. V2 also fixed some issues > found by others and was generated based on master latest version. > > For 3#, I can reproduce the issue, I'll fix it in patch serial V3. My > understanding is that when I fix 3#, 2# will be resolved accordingly. Right? Yes. I first discovered the problem with (2), but then I wanted to reproduce the issue in isolation from QEMU. That is section (3). So if you fix (3), I expect the build will work just fine as part of the QEMU tree (2) as well. > For 5#-2, When using "-n 1", Autogen should start one worker subprocess to > generate autogen files that will be the same as run in serial. > I'll make the change in patches V3 OK, sounds good. Thank you, Laszlo > > For the keyboard interrupt handling mentioned in 4# and 5#-1, I'll do more > testing on it. And do the fix in V3. > Before sending the patches, I tested Ctrl+C to stop the build but not the > "pick up". > > Thanks, > Bob > > -Original Message- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Sunday, July 21, 2019 8:26 PM > To: Feng, Bob C > Cc: devel@edk2.groups.io; Philippe Mathieu-Daudé > Subject: Re: [edk2-devel] [Patch 0/9] Enable multiple process AutoGen > > Hi Bob, > > On 07/18/19 08:14, Bob Feng wrote: >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875 >> >> In order to improve the build performance, we implemented >> multiple-processes AutoGen. This change will reduce 20% time for >> AutoGen phase. >> >> The design document can be got from: >> https://edk2.groups.io/g/devel/files/Designs/2019/0627/Multiple-thread >> -AutoGen.pdf >> >> This patch serial pass the build of Ovmf, MinKabylake, MinPurley, >> packages under Edk2 repository and intel client and server platforms. > > I've done some basic regression-testing with this set, applied on top of > current master (= commit 8ff68cd5e4c9, "ShellPkg: acpiview: DBG2: Remove > redundant forward declarations", 2019-07-19). > > (1) The build process now seems to produce files called > > GlobalVar_37d7e986-f7e9-45c2-8067-e371421a626c_AARCH64.bin > GlobalVar_37d7e986-f7e9-45c2-8067-e371421a626c_ARM.bin > GlobalVar_5a9e7754-d81b-49ea-85ad-69eaa7b1539b_IA32.bin > GlobalVar_5a9e7754-d81b-49ea-85ad-69eaa7b1539b_X64.bin > > in the project root directory. (The GUIDs match the PLATFORM_GUID > values from ArmVirtQemu.dsc and OvmfPkg*.dsc.) > > They seem to come from patch "BaseTools: Enable Multiple Process > AutoGen". > > Can we place these files under the Build/ directory somewhere, > instead? > > (2) The QEMU project now bundles edk2 (as a git submodule for the edk2 > tree, and some firmware binaries built from that). The build scripts > carried by QEMU set PYTHON_COMMAND to python2, in order to work > around TianoCore BZ#1607. > > For regression-testing this set, I ran > > $ make -C roms efi > > with QEMU's edk2 submodule advanced to commit 8ff68cd5e4c9, plus > this set applied (see above), and with QEMU itself being at > e2b47666fe15. > > The build itself passes fine, so that's good. > > But, some (not all) of the firmware binaries are *misbuilt*. I'll > elaborate below (independently of QEMU). > > (3) Consider the following test. (Note that this is independent of the > QEMU project entirely.) > > (3a) Set up a pristine build environment: > > # Open a new shell, then: > $ git clean -ffdx > $ git reset --hard > $ git submodule deinit --force --all > $ git checkout master > $ git submodule update --init > $ export PYTHON_COMMAND=python2 > $ source edksetup.sh > $ nice make -C "$EDK_TOOLS_PATH" \ > -j $(getconf _NPROCESSORS_ONLN) > > (3b) Build OvmfPkgIA32 with one set of features, and capture a > checksum of the resultant firmware binary: > > $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ > -t GCC48 > $ md5sum Build/OvmfIa32/NOOPT_GCC48/FV/OVMF_CODE.fd > 8d56575dd3b5c89ac6f1829ae4f41b55 > > (3c) *Re*build the same platform with a different set of features, > and capture a checksum again: > > $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ > -t GCC48 -D SECURE_BOOT_ENABLE -D SMM_REQUIRE > $ md5sum Build/OvmfIa32/NOO
Re: [edk2-devel] [Patch 0/9] Enable multiple process AutoGen
Hi Laszlo, Thanks for your detailed testing and comments. I send out patch serial V2 to resolve comment 1#. V2 also fixed some issues found by others and was generated based on master latest version. For 3#, I can reproduce the issue, I'll fix it in patch serial V3. My understanding is that when I fix 3#, 2# will be resolved accordingly. Right? For 5#-2, When using "-n 1", Autogen should start one worker subprocess to generate autogen files that will be the same as run in serial. I'll make the change in patches V3 For the keyboard interrupt handling mentioned in 4# and 5#-1, I'll do more testing on it. And do the fix in V3. Before sending the patches, I tested Ctrl+C to stop the build but not the "pick up". Thanks, Bob -Original Message- From: Laszlo Ersek [mailto:ler...@redhat.com] Sent: Sunday, July 21, 2019 8:26 PM To: Feng, Bob C Cc: devel@edk2.groups.io; Philippe Mathieu-Daudé Subject: Re: [edk2-devel] [Patch 0/9] Enable multiple process AutoGen Hi Bob, On 07/18/19 08:14, Bob Feng wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875 > > In order to improve the build performance, we implemented > multiple-processes AutoGen. This change will reduce 20% time for > AutoGen phase. > > The design document can be got from: > https://edk2.groups.io/g/devel/files/Designs/2019/0627/Multiple-thread > -AutoGen.pdf > > This patch serial pass the build of Ovmf, MinKabylake, MinPurley, > packages under Edk2 repository and intel client and server platforms. I've done some basic regression-testing with this set, applied on top of current master (= commit 8ff68cd5e4c9, "ShellPkg: acpiview: DBG2: Remove redundant forward declarations", 2019-07-19). (1) The build process now seems to produce files called GlobalVar_37d7e986-f7e9-45c2-8067-e371421a626c_AARCH64.bin GlobalVar_37d7e986-f7e9-45c2-8067-e371421a626c_ARM.bin GlobalVar_5a9e7754-d81b-49ea-85ad-69eaa7b1539b_IA32.bin GlobalVar_5a9e7754-d81b-49ea-85ad-69eaa7b1539b_X64.bin in the project root directory. (The GUIDs match the PLATFORM_GUID values from ArmVirtQemu.dsc and OvmfPkg*.dsc.) They seem to come from patch "BaseTools: Enable Multiple Process AutoGen". Can we place these files under the Build/ directory somewhere, instead? (2) The QEMU project now bundles edk2 (as a git submodule for the edk2 tree, and some firmware binaries built from that). The build scripts carried by QEMU set PYTHON_COMMAND to python2, in order to work around TianoCore BZ#1607. For regression-testing this set, I ran $ make -C roms efi with QEMU's edk2 submodule advanced to commit 8ff68cd5e4c9, plus this set applied (see above), and with QEMU itself being at e2b47666fe15. The build itself passes fine, so that's good. But, some (not all) of the firmware binaries are *misbuilt*. I'll elaborate below (independently of QEMU). (3) Consider the following test. (Note that this is independent of the QEMU project entirely.) (3a) Set up a pristine build environment: # Open a new shell, then: $ git clean -ffdx $ git reset --hard $ git submodule deinit --force --all $ git checkout master $ git submodule update --init $ export PYTHON_COMMAND=python2 $ source edksetup.sh $ nice make -C "$EDK_TOOLS_PATH" \ -j $(getconf _NPROCESSORS_ONLN) (3b) Build OvmfPkgIA32 with one set of features, and capture a checksum of the resultant firmware binary: $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ -t GCC48 $ md5sum Build/OvmfIa32/NOOPT_GCC48/FV/OVMF_CODE.fd 8d56575dd3b5c89ac6f1829ae4f41b55 (3c) *Re*build the same platform with a different set of features, and capture a checksum again: $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ -t GCC48 -D SECURE_BOOT_ENABLE -D SMM_REQUIRE $ md5sum Build/OvmfIa32/NOOPT_GCC48/FV/OVMF_CODE.fd ed79052e9add242174ccefc78a5bddb3 Okay, now repeat all three steps from zero, but with the present feature patch set applied: (3d) Repeat (3a), but rather than checking out the master branch with "git checkout", check out the topic branch that is the same "master" commit, *plus* the present series applied on top. Don't forget to start a new shell first! (3e) Repeat (3b): $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ -t GCC48 $ md5sum Build/OvmfIa32/NOOPT_GCC48/FV/OVMF_CODE.fd 8d56575dd3b5c89ac6f1829ae4f41b55 Notice that the checksum is identical to that from (3b), so all's good. (3f) Repeat (3c): $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \
Re: [edk2-devel] [Patch 0/9] Enable multiple process AutoGen
> This is the problem: the checksum does not match the one from > (3c). And, the binary from (3f) is in fact mis-built, it > crashes immediately when launched on QEMU/KVM, with "emulation > failure". It looks an incremental build error. This multi-process build tool fails to re-generate the new autogen code (CreateCodeFile) and makefile (CreateMakeFile). The new build tool wrongly skip these steps in incremental build. Thanks Steven Shi > -Original Message- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Laszlo Ersek > Sent: Sunday, July 21, 2019 8:26 PM > To: Feng, Bob C > Cc: devel@edk2.groups.io; Philippe Mathieu-Daudé > Subject: Re: [edk2-devel] [Patch 0/9] Enable multiple process AutoGen > > Hi Bob, > > On 07/18/19 08:14, Bob Feng wrote: > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875 > > > > In order to improve the build performance, we implemented > > multiple-processes AutoGen. This change will reduce 20% time > > for AutoGen phase. > > > > The design document can be got from: > > https://edk2.groups.io/g/devel/files/Designs/2019/0627/Multiple-thread- > AutoGen.pdf > > > > This patch serial pass the build of Ovmf, MinKabylake, MinPurley, > > packages under Edk2 repository and intel client and server platforms. > > I've done some basic regression-testing with this set, applied on top of > current master (= commit 8ff68cd5e4c9, "ShellPkg: acpiview: DBG2: Remove > redundant forward declarations", 2019-07-19). > > (1) The build process now seems to produce files called > > GlobalVar_37d7e986-f7e9-45c2-8067-e371421a626c_AARCH64.bin > GlobalVar_37d7e986-f7e9-45c2-8067-e371421a626c_ARM.bin > GlobalVar_5a9e7754-d81b-49ea-85ad-69eaa7b1539b_IA32.bin > GlobalVar_5a9e7754-d81b-49ea-85ad-69eaa7b1539b_X64.bin > > in the project root directory. (The GUIDs match the PLATFORM_GUID > values from ArmVirtQemu.dsc and OvmfPkg*.dsc.) > > They seem to come from patch "BaseTools: Enable Multiple Process > AutoGen". > > Can we place these files under the Build/ directory somewhere, > instead? > > (2) The QEMU project now bundles edk2 (as a git submodule for the edk2 > tree, and some firmware binaries built from that). The build scripts > carried by QEMU set PYTHON_COMMAND to python2, in order to work > around TianoCore BZ#1607. > > For regression-testing this set, I ran > > $ make -C roms efi > > with QEMU's edk2 submodule advanced to commit 8ff68cd5e4c9, plus > this set applied (see above), and with QEMU itself being at > e2b47666fe15. > > The build itself passes fine, so that's good. > > But, some (not all) of the firmware binaries are *misbuilt*. I'll > elaborate below (independently of QEMU). > > (3) Consider the following test. (Note that this is independent of the > QEMU project entirely.) > > (3a) Set up a pristine build environment: > > # Open a new shell, then: > $ git clean -ffdx > $ git reset --hard > $ git submodule deinit --force --all > $ git checkout master > $ git submodule update --init > $ export PYTHON_COMMAND=python2 > $ source edksetup.sh > $ nice make -C "$EDK_TOOLS_PATH" \ > -j $(getconf _NPROCESSORS_ONLN) > > (3b) Build OvmfPkgIA32 with one set of features, and capture a > checksum of the resultant firmware binary: > > $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ > -t GCC48 > $ md5sum Build/OvmfIa32/NOOPT_GCC48/FV/OVMF_CODE.fd > 8d56575dd3b5c89ac6f1829ae4f41b55 > > (3c) *Re*build the same platform with a different set of features, > and capture a checksum again: > > $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ > -t GCC48 -D SECURE_BOOT_ENABLE -D SMM_REQUIRE > $ md5sum Build/OvmfIa32/NOOPT_GCC48/FV/OVMF_CODE.fd > ed79052e9add242174ccefc78a5bddb3 > > Okay, now repeat all three steps from zero, but with the present > feature patch set applied: > > (3d) Repeat (3a), but rather than checking out the master branch > with "git checkout", check out the topic branch that is the > same "master" commit, *plus* the present series applied on top. > > Don't forget to start a new shell first! > > (3e) Repeat (3b): > > $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ > -t GCC48 &
Re: [edk2-devel] [Patch 0/9] Enable multiple process AutoGen
Hi Bob, On 07/18/19 08:14, Bob Feng wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875 > > In order to improve the build performance, we implemented > multiple-processes AutoGen. This change will reduce 20% time > for AutoGen phase. > > The design document can be got from: > https://edk2.groups.io/g/devel/files/Designs/2019/0627/Multiple-thread-AutoGen.pdf > > This patch serial pass the build of Ovmf, MinKabylake, MinPurley, > packages under Edk2 repository and intel client and server platforms. I've done some basic regression-testing with this set, applied on top of current master (= commit 8ff68cd5e4c9, "ShellPkg: acpiview: DBG2: Remove redundant forward declarations", 2019-07-19). (1) The build process now seems to produce files called GlobalVar_37d7e986-f7e9-45c2-8067-e371421a626c_AARCH64.bin GlobalVar_37d7e986-f7e9-45c2-8067-e371421a626c_ARM.bin GlobalVar_5a9e7754-d81b-49ea-85ad-69eaa7b1539b_IA32.bin GlobalVar_5a9e7754-d81b-49ea-85ad-69eaa7b1539b_X64.bin in the project root directory. (The GUIDs match the PLATFORM_GUID values from ArmVirtQemu.dsc and OvmfPkg*.dsc.) They seem to come from patch "BaseTools: Enable Multiple Process AutoGen". Can we place these files under the Build/ directory somewhere, instead? (2) The QEMU project now bundles edk2 (as a git submodule for the edk2 tree, and some firmware binaries built from that). The build scripts carried by QEMU set PYTHON_COMMAND to python2, in order to work around TianoCore BZ#1607. For regression-testing this set, I ran $ make -C roms efi with QEMU's edk2 submodule advanced to commit 8ff68cd5e4c9, plus this set applied (see above), and with QEMU itself being at e2b47666fe15. The build itself passes fine, so that's good. But, some (not all) of the firmware binaries are *misbuilt*. I'll elaborate below (independently of QEMU). (3) Consider the following test. (Note that this is independent of the QEMU project entirely.) (3a) Set up a pristine build environment: # Open a new shell, then: $ git clean -ffdx $ git reset --hard $ git submodule deinit --force --all $ git checkout master $ git submodule update --init $ export PYTHON_COMMAND=python2 $ source edksetup.sh $ nice make -C "$EDK_TOOLS_PATH" \ -j $(getconf _NPROCESSORS_ONLN) (3b) Build OvmfPkgIA32 with one set of features, and capture a checksum of the resultant firmware binary: $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ -t GCC48 $ md5sum Build/OvmfIa32/NOOPT_GCC48/FV/OVMF_CODE.fd 8d56575dd3b5c89ac6f1829ae4f41b55 (3c) *Re*build the same platform with a different set of features, and capture a checksum again: $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ -t GCC48 -D SECURE_BOOT_ENABLE -D SMM_REQUIRE $ md5sum Build/OvmfIa32/NOOPT_GCC48/FV/OVMF_CODE.fd ed79052e9add242174ccefc78a5bddb3 Okay, now repeat all three steps from zero, but with the present feature patch set applied: (3d) Repeat (3a), but rather than checking out the master branch with "git checkout", check out the topic branch that is the same "master" commit, *plus* the present series applied on top. Don't forget to start a new shell first! (3e) Repeat (3b): $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ -t GCC48 $ md5sum Build/OvmfIa32/NOOPT_GCC48/FV/OVMF_CODE.fd 8d56575dd3b5c89ac6f1829ae4f41b55 Notice that the checksum is identical to that from (3b), so all's good. (3f) Repeat (3c): $ nice build -a IA32 -b NOOPT -p OvmfPkg/OvmfPkgIA32.dsc \ -t GCC48 -D SECURE_BOOT_ENABLE -D SMM_REQUIRE $ md5sum Build/OvmfIa32/NOOPT_GCC48/FV/OVMF_CODE.fd baf08c5a9ecb0adc754f5b48410b6476 This is the problem: the checksum does not match the one from (3c). And, the binary from (3f) is in fact mis-built, it crashes immediately when launched on QEMU/KVM, with "emulation failure". Thus: - Without the patch set, if I build a platform, then *rebuild* it with different -D flags, then both builds produce proper outputs. - With the patch series applied however, only the initial build produces good results. The *rebuild*, with different -D flags, is "poisoned" by the artifacts still laying around from the first build. In other words, this is a "stale cache" problem. (4) Not a show stopper, but a divergence from previous behavior: When one presses Ctrl-S (the "stop" character) in a terminal, the terminal output / autoscrolling is suspended, until Ctrl-Q (the "start" character) is pressed. This is generally used for two things: scrolling back the
[edk2-devel] [Patch 0/9] Enable multiple process AutoGen
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1875 In order to improve the build performance, we implemented multiple-processes AutoGen. This change will reduce 20% time for AutoGen phase. The design document can be got from: https://edk2.groups.io/g/devel/files/Designs/2019/0627/Multiple-thread-AutoGen.pdf This patch serial pass the build of Ovmf, MinKabylake, MinPurley, packages under Edk2 repository and intel client and server platforms. Feng, Bob C (9): BaseTools: Singleton the object to handle build conf file BaseTools: Split WorkspaceAutoGen._InitWorker into multiple functions BaseTools: Add functions to get platform scope build options BaseTools: Decouple AutoGen Objects BaseTools: Enable Multiple Process AutoGen BaseTools: Add shared data for processes BaseTools: Add LogAgent to support multiple process Autogen BaseTools: Move BuildOption parser out of build.py BaseTools: Add the support for python 2 BaseTools/Source/Python/AutoGen/AutoGen.py| 4227 + .../Source/Python/AutoGen/AutoGenWorker.py| 220 + .../Source/Python/AutoGen/BuildEngine.py | 22 + BaseTools/Source/Python/AutoGen/DataPipe.py | 153 + BaseTools/Source/Python/AutoGen/GenC.py |6 +- .../Source/Python/AutoGen/ModuleAutoGen.py| 1883 .../Python/AutoGen/ModuleAutoGenHelper.py | 616 +++ .../Source/Python/AutoGen/PlatformAutoGen.py | 1483 ++ .../Source/Python/AutoGen/WorkspaceAutoGen.py | 902 BaseTools/Source/Python/Common/EdkLogger.py | 117 +- BaseTools/Source/Python/Common/Misc.py|1 - .../Python/Common/TargetTxtClassObject.py | 28 +- .../Python/Common/ToolDefClassObject.py |6 +- BaseTools/Source/Python/GenFds/GenFds.py |4 +- .../Python/GenFds/GenFdsGlobalVariable.py | 54 +- .../Python/PatchPcdValue/PatchPcdValue.py |1 - .../Source/Python/Workspace/DscBuildData.py | 38 +- .../Source/Python/Workspace/InfBuildData.py | 39 + .../Python/Workspace/WorkspaceCommon.py |4 + .../Python/Workspace/WorkspaceDatabase.py |3 + BaseTools/Source/Python/build/BuildReport.py |4 +- BaseTools/Source/Python/build/build.py| 322 +- BaseTools/Source/Python/build/buildoptions.py | 92 + 23 files changed, 5789 insertions(+), 4436 deletions(-) create mode 100644 BaseTools/Source/Python/AutoGen/AutoGenWorker.py create mode 100644 BaseTools/Source/Python/AutoGen/DataPipe.py create mode 100644 BaseTools/Source/Python/AutoGen/ModuleAutoGen.py create mode 100644 BaseTools/Source/Python/AutoGen/ModuleAutoGenHelper.py create mode 100644 BaseTools/Source/Python/AutoGen/PlatformAutoGen.py create mode 100644 BaseTools/Source/Python/AutoGen/WorkspaceAutoGen.py create mode 100644 BaseTools/Source/Python/build/buildoptions.py -- 2.20.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#43907): https://edk2.groups.io/g/devel/message/43907 Mute This Topic: https://groups.io/mt/32512451/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
