| Summary: | sys-apps/portage: optimize bash PORTAGE_IUSE variable to use associative array instead of regular expression | ||
|---|---|---|---|
| Product: | Portage Development | Reporter: | Zac Medico <zmedico> |
| Component: | Core - Ebuild Support | Assignee: | Portage team <dev-portage> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | kevinmbecause |
| Priority: | Normal | Keywords: | InVCS |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| URL: | https://chromium-review.googlesource.com/c/chromiumos/third_party/portage_tool/+/1524641 | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 835380, 683434 | ||
|
Description
Zac Medico
2019-03-17 21:57:02 UTC
Patch posted for review: https://archives.gentoo.org/gentoo-portage-dev/message/379563f43a119166e0675fb96ce200ce https://github.com/gentoo/portage/pull/415 The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/portage.git/commit/?id=9cac3bfa782fe7eaa27103821a980cadf9299421 commit 9cac3bfa782fe7eaa27103821a980cadf9299421 Author: Douglas Anderson <dianders@chromium.org> AuthorDate: 2019-03-14 15:35:06 +0000 Commit: Zac Medico <zmedico@gentoo.org> CommitDate: 2019-04-15 23:01:56 +0000 Speed up testing against IUSE by not using regexp When trying to figure out why it took so long to do a no-op kernel build (re-build when nothing changed) on Chrome OS, I tracked down one slowdown to cros-kernel2_src_configure(). This function was taking ~900 ms to execute. The bulk of that slowdown was in iterating over the list of config fragments, specifically the "use ${fragment}" test. We currently have 77 fragments so we were effectively calling the "use" function 77 times. Digging through the portage code, the slow part of the "use" function was the block of code to confirm that you specified each USE flag in your IUSE. Commenting out the whole "elif" block of code there sped things up so that the entire cros-kernel2_src_configure() was now taking ~130 ms. This means that each call to the "use" function was taking about 10 ms. The specific part of the test that was slow was testing against the regular expression. It was specifically slow in the Chrome OS kernel build because we inherit the "cros-board" eclass which populates a huge number of boards in the USE flag, making the regular expression totally unwieldly. One way to speed this whole thing up is to use a bash associative array. Unfortunately arrays can't come in through environment variables, so we'll write a function that declares the array the first time it's needed. With this version of the code cros-kernel2_src_configure() now takes ~190 ms which seems like it's OK. AKA 77 checks against IUSE took 60 ms or less than 1 ms per check. NOTE: to keep EAPI 4 and older working, we keep doing the regular expression tests there, though we now do it in the __in_portage_iuse() function. In at least one test the extra overhead of the function made testing USE flags on EAPI 4 ~15% slower, but presumably this is OK as we want to encourage folks to move to the newer EAPIs. BUG=chromium:767073 TEST=Time some builds; confirm bad use flags still caught. Change-Id: Ic74fa49bdf002399ba0d6c41f42d4632b07127a9 Reviewed-on: https://chromium-review.googlesource.com/1524641 Commit-Ready: Douglas Anderson <dianders@chromium.org> Tested-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Douglas Anderson <dianders@chromium.org> See: https://chromium.googlesource.com/chromiumos/third_party/portage_tool/+/82a0776602df5707606de2099b93b8b7b1cc34a1 Bug: https://bugs.gentoo.org/680810 Signed-off-by: Zac Medico <zmedico@gentoo.org> bin/ebuild.sh | 4 ++-- bin/phase-helpers.sh | 6 +++--- .../package/ebuild/_config/special_env_vars.py | 7 ++++--- lib/portage/package/ebuild/config.py | 20 +++++++++++++++++--- 4 files changed, 26 insertions(+), 11 deletions(-) While this is a nifty feature, it breaks certain builds when /bin/sh doesn't grok this magical BASH_FUNC____in_portage_iuse%%. You see messages like
/bin/sh: line 4: `BASH_FUNC____in_portage_iuse%%': not a valid identifier
all over the place, and unfortunately it breaks for packages like e.g. bison like this:
LC_ALL=C tests/bison --version >doc/bison.help.tmp
LC_ALL=C tests/bison --help | \
sed -e 's,^Usage: .*/bison \[OPTION\],Usage: bison [OPTION],g' \
-e '/translation bugs/d' >>doc/bison.help.tmp
./build-aux/move-if-change doc/bison.help.tmp doc/bison.help
/bin/sh: line 4: `BASH_FUNC____in_portage_iuse%%': not a valid identifier
make[2]: *** [Makefile:6857: doc/bison.help] Error 2
make[2]: Leaving directory '/Volumes/Scratch/Gentoo/bootstrap32-20190603/tmp/var/tmp/portage/sys-devel/bison-3.4.1/work/bison-3.4.1'
make[1]: *** [Makefile:5613: all-recursive] Error 1
make[1]: Leaving directory '/Volumes/Scratch/Gentoo/bootstrap32-20190603/tmp/var/tmp/portage/sys-devel/bison-3.4.1/work/bison-3.4.1'
make: *** [Makefile:3124: all] Error 2
* ERROR: sys-devel/bison-3.4.1::gentoo_prefix failed (compile phase):
* emake failed
/bin/sh is called by many things, such as python's plaform.system(), hence the frequent repetition of the error.
I assume this variable is only interesting for Portage's own env (which uses bash, of a certain version that groks this), so when it bleeds through it is basically not necessary. I was wondering if you have suggestions/ideas on how to limit the scope of this variable to ebuild.sh and friends only?
Note that I tried replacing os.system with subprocess.call in lib/portage/__init__.py, but that isn't enough, since basically anything that forks the /bin/sh shell like this is affected.
(In reply to Fabian Groffen from comment #3) > /bin/sh: line 4: `BASH_FUNC____in_portage_iuse%%': not a valid identifier Hopefully a patch like this will correct the problem: > diff --git a/bin/ebuild.sh b/bin/ebuild.sh > index 20dff6f3f..50a0330f3 100755 > --- a/bin/ebuild.sh > +++ b/bin/ebuild.sh > @@ -8,6 +8,7 @@ unalias -a > > # Make sure this isn't exported to scripts we execute. > unset BASH_COMPAT > +export -n -f ___in_portage_iuse > > source "${PORTAGE_BIN_PATH}/isolated-functions.sh" || exit (In reply to Zac Medico from comment #4) > (In reply to Fabian Groffen from comment #3) > > /bin/sh: line 4: `BASH_FUNC____in_portage_iuse%%': not a valid identifier > > Hopefully a patch like this will correct the problem: > > > diff --git a/bin/ebuild.sh b/bin/ebuild.sh > > index 20dff6f3f..50a0330f3 100755 > > --- a/bin/ebuild.sh > > +++ b/bin/ebuild.sh > > @@ -8,6 +8,7 @@ unalias -a > > > > # Make sure this isn't exported to scripts we execute. > > unset BASH_COMPAT > > +export -n -f ___in_portage_iuse > > > > source "${PORTAGE_BIN_PATH}/isolated-functions.sh" || exit Thanks, that fix makes the error go away and make otherwise failing packages compile again! The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/portage.git/commit/?id=7f1aac1113203cb43d2a44bfafd6b8deb045efea commit 7f1aac1113203cb43d2a44bfafd6b8deb045efea Author: Zac Medico <zmedico@gentoo.org> AuthorDate: 2019-06-05 20:28:52 +0000 Commit: Zac Medico <zmedico@gentoo.org> CommitDate: 2019-06-05 20:32:29 +0000 ebuild.sh: unexport ___in_portage_iuse function (bug 680810) The exported BASH_FUNC____in_portage_iuse%% variable can trigger problems for some shells, so do not export it. Reported-by: Fabian Groffen <grobian@gentoo.org> Bug: https://bugs.gentoo.org/680810 Fixes: 9cac3bfa782f ("Speed up testing against IUSE by not using regexp") Signed-off-by: Zac Medico <zmedico@gentoo.org> bin/ebuild.sh | 1 + 1 file changed, 1 insertion(+) (In reply to Larry the Git Cow from comment #6) > The bug has been referenced in the following commit(s): > > https://gitweb.gentoo.org/proj/portage.git/commit/ > ?id=7f1aac1113203cb43d2a44bfafd6b8deb045efea > > commit 7f1aac1113203cb43d2a44bfafd6b8deb045efea > Author: Zac Medico <zmedico@gentoo.org> > AuthorDate: 2019-06-05 20:28:52 +0000 > Commit: Zac Medico <zmedico@gentoo.org> > CommitDate: 2019-06-05 20:32:29 +0000 > > ebuild.sh: unexport ___in_portage_iuse function (bug 680810) > > The exported BASH_FUNC____in_portage_iuse%% variable can trigger > problems for some shells, so do not export it. > > Reported-by: Fabian Groffen <grobian@gentoo.org> > Bug: https://bugs.gentoo.org/680810 > Fixes: 9cac3bfa782f ("Speed up testing against IUSE by not using regexp") > Signed-off-by: Zac Medico <zmedico@gentoo.org> > > bin/ebuild.sh | 1 + > 1 file changed, 1 insertion(+) This can trigger a bunch of eix-update noise like: > /usr/lib/portage/python3.6/ebuild.sh: line 11: export: ___in_portage_iuse: not a function > /usr/lib/portage/python3.6/ebuild.sh: line 11: export: ___in_portage_iuse: not a function > /usr/lib/portage/python3.6/ebuild.sh: line 11: export: ___in_portage_iuse: not a function > /usr/lib/portage/python3.6/ebuild.sh: line 11: export: ___in_portage_iuse: not a function (In reply to Zac Medico from comment #7) > This can trigger a bunch of eix-update noise like: > > > /usr/lib/portage/python3.6/ebuild.sh: line 11: export: ___in_portage_iuse: not a function > > /usr/lib/portage/python3.6/ebuild.sh: line 11: export: ___in_portage_iuse: not a function > > /usr/lib/portage/python3.6/ebuild.sh: line 11: export: ___in_portage_iuse: not a function > > /usr/lib/portage/python3.6/ebuild.sh: line 11: export: ___in_portage_iuse: not a function Noted in bug 689128 comment 0. *** Bug 689126 has been marked as a duplicate of this bug. *** The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/portage.git/commit/?id=f4aa49bc1ba210a1257ae6291a60d0944c32691d commit f4aa49bc1ba210a1257ae6291a60d0944c32691d Author: Zac Medico <zachary.medico@sony.com> AuthorDate: 2019-07-03 21:20:00 +0000 Commit: Zac Medico <zmedico@gentoo.org> CommitDate: 2019-07-03 21:26:25 +0000 ebuild.sh: suppress export error messages for eix-update Suppress export error messages like this for eix-update: /usr/lib/portage/python3.6/ebuild.sh: line 11: export: ___in_portage_iuse: not a function Fixes: 7f1aac111320 ("ebuild.sh: unexport ___in_portage_iuse function (bug 680810)") Bug: https://bugs.gentoo.org/680810 Bug: https://bugs.gentoo.org/689128 Signed-off-by: Zac Medico <zmedico@gentoo.org> bin/ebuild.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=fb6dd4afe2540de3ae7de4b6d7ffdd776b14712b commit fb6dd4afe2540de3ae7de4b6d7ffdd776b14712b Author: Zac Medico <zachary.medico@sony.com> AuthorDate: 2019-07-03 21:40:27 +0000 Commit: Zac Medico <zmedico@gentoo.org> CommitDate: 2019-07-03 21:49:28 +0000 sys-apps/portage: revbump to 2.3.68-r1 Suppress export error messages like this for eix-update: /usr/lib/portage/python3.6/ebuild.sh: line 11: export: ___in_portage_iuse: not a function Bug: https://bugs.gentoo.org/680810#c10 Bug: https://bugs.gentoo.org/689128#c3 Package-Manager: Portage-2.3.68, Repoman-2.3.16 Signed-off-by: Zac Medico <zmedico@gentoo.org> sys-apps/portage/{portage-2.3.68.ebuild => portage-2.3.68-r1.ebuild} | 4 ++++ 1 file changed, 4 insertions(+) |