Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 680810 - sys-apps/portage: optimize bash PORTAGE_IUSE variable to use associative array instead of regular expression
Summary: sys-apps/portage: optimize bash PORTAGE_IUSE variable to use associative arra...
Status: RESOLVED FIXED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core - Ebuild Support (show other bugs)
Hardware: All All
: Normal normal (vote)
Assignee: Portage team
URL: https://chromium-review.googlesource....
Whiteboard:
Keywords: InVCS
: 689126 (view as bug list)
Depends on:
Blocks: 835380 683434
  Show dependency tree
 
Reported: 2019-03-17 21:57 UTC by Zac Medico
Modified: 2023-05-20 05:12 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Zac Medico gentoo-dev 2019-03-17 21:57:02 UTC
We can conditionally enable regular expressions for EAPI 4 and earlier, since regular expressions are used in the _get_implicit_iuse method. For EAPI 5 and later (with IUSE_EFFECTIVE), we can use a bash associative array as shown here:

https://chromium-review.googlesource.com/c/chromiumos/third_party/portage_tool/+/1524641
Comment 2 Larry the Git Cow gentoo-dev 2019-04-15 23:02:27 UTC
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(-)
Comment 3 Fabian Groffen gentoo-dev 2019-06-04 15:39:10 UTC
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.
Comment 4 Zac Medico gentoo-dev 2019-06-04 18:53:08 UTC
(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
Comment 5 Fabian Groffen gentoo-dev 2019-06-05 15:47:19 UTC
(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!
Comment 6 Larry the Git Cow gentoo-dev 2019-06-05 20:33:38 UTC
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(+)
Comment 7 Zac Medico gentoo-dev 2019-07-02 01:29:08 UTC
(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
Comment 8 Zac Medico gentoo-dev 2019-07-02 01:56:07 UTC
(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.
Comment 9 kevinmbecause 2019-07-02 08:18:58 UTC
*** Bug 689126 has been marked as a duplicate of this bug. ***
Comment 10 Larry the Git Cow gentoo-dev 2019-07-03 21:27:52 UTC
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(-)
Comment 11 Larry the Git Cow gentoo-dev 2019-07-03 21:49:38 UTC
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(+)