From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 15133138334 for ; Thu, 11 Apr 2019 02:41:22 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id AB44BE08F4; Thu, 11 Apr 2019 02:41:20 +0000 (UTC) Received: from smtp.gentoo.org (mail.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 660B0E08F4 for ; Thu, 11 Apr 2019 02:41:20 +0000 (UTC) Received: from r6.ad.gaikai.biz (unknown [100.42.98.196]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: zmedico) by smtp.gentoo.org (Postfix) with ESMTPSA id BDEFC335D19; Thu, 11 Apr 2019 02:41:18 +0000 (UTC) From: Zac Medico To: gentoo-portage-dev@lists.gentoo.org Cc: Mike Frysinger , Douglas Anderson , Zac Medico Subject: [gentoo-portage-dev] [PATCH] Speed up testing against IUSE by not using regexp Date: Wed, 10 Apr 2019 19:40:57 -0700 Message-Id: <20190411024057.13636-1-zmedico@gentoo.org> X-Mailer: git-send-email 2.21.0 Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-portage-dev@lists.gentoo.org Reply-to: gentoo-portage-dev@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Archives-Salt: 275defcb-caca-4c49-aad7-c51866da2462 X-Archives-Hash: 379563f43a119166e0675fb96ce200ce From: Douglas Anderson 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 Tested-by: Douglas Anderson Reviewed-by: Douglas Anderson See: https://chromium.googlesource.com/chromiumos/third_party/portage_tool/+/82a0776602df5707606de2099b93b8b7b1cc34a1 Bug: https://bugs.gentoo.org/680810 Signed-off-by: Zac Medico --- bin/ebuild.sh | 4 ++-- bin/phase-helpers.sh | 6 +++--- .../ebuild/_config/special_env_vars.py | 7 ++++--- lib/portage/package/ebuild/config.py | 20 ++++++++++++++++--- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/bin/ebuild.sh b/bin/ebuild.sh index d3bf0fd29..20dff6f3f 100755 --- a/bin/ebuild.sh +++ b/bin/ebuild.sh @@ -763,8 +763,8 @@ else # If ${EBUILD_FORCE_TEST} == 1 and USE came from ${T}/environment # then it might not have USE=test like it's supposed to here. - if [[ ${EBUILD_PHASE} == test && ${EBUILD_FORCE_TEST} == 1 && - test =~ ${PORTAGE_IUSE} ]] && ! has test ${USE} ; then + if [[ ${EBUILD_PHASE} == test && ${EBUILD_FORCE_TEST} == 1 ]] && + ___in_portage_iuse test && ! has test ${USE} ; then export USE="${USE} test" fi declare -r USE diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh index ba3f27930..64a82b4b7 100644 --- a/bin/phase-helpers.sh +++ b/bin/phase-helpers.sh @@ -237,9 +237,9 @@ use() { # Make sure we have this USE flag in IUSE, but exempt binary # packages for API consumers like Entropy which do not require # a full profile with IUSE_IMPLICIT and stuff (see bug #456830). - elif [[ -n $PORTAGE_IUSE && -n $EBUILD_PHASE && - -n $PORTAGE_INTERNAL_CALLER ]] ; then - if [[ ! $u =~ $PORTAGE_IUSE ]] ; then + elif declare -f ___in_portage_iuse >/dev/null && + [[ -n ${EBUILD_PHASE} && -n ${PORTAGE_INTERNAL_CALLER} ]] ; then + if ! ___in_portage_iuse "${u}"; then if [[ ${EMERGE_FROM} != binary && ! ${EAPI} =~ ^(0|1|2|3|4|4-python|4-slot-abi)$ ]] ; then # This is only strict starting with EAPI 5, since implicit IUSE diff --git a/lib/portage/package/ebuild/_config/special_env_vars.py b/lib/portage/package/ebuild/_config/special_env_vars.py index f9a0c3c0e..e72049e33 100644 --- a/lib/portage/package/ebuild/_config/special_env_vars.py +++ b/lib/portage/package/ebuild/_config/special_env_vars.py @@ -14,8 +14,8 @@ import re # to enter the config instance from the external environment or # configuration files. env_blacklist = frozenset(( - "A", "AA", "BDEPEND", "BROOT", "CATEGORY", "DEPEND", "DESCRIPTION", - "DOCS", "EAPI", + "A", "AA", "BASH_FUNC____in_portage_iuse%%", "BDEPEND", "BROOT", + "CATEGORY", "DEPEND", "DESCRIPTION", "DOCS", "EAPI", "EBUILD_FORCE_TEST", "EBUILD_PHASE", "EBUILD_PHASE_FUNC", "EBUILD_SKIP_MANIFEST", "ED", "EMERGE_FROM", "EPREFIX", "EROOT", @@ -42,7 +42,8 @@ environ_whitelist = [] # environment in order to prevent sandbox from sourcing /etc/profile # in it's bashrc (causing major leakage). environ_whitelist += [ - "ACCEPT_LICENSE", "BASH_ENV", "BROOT", "BUILD_PREFIX", "COLUMNS", "D", + "ACCEPT_LICENSE", "BASH_ENV", "BASH_FUNC____in_portage_iuse%%", + "BROOT", "BUILD_PREFIX", "COLUMNS", "D", "DISTDIR", "DOC_SYMLINKS_DIR", "EAPI", "EBUILD", "EBUILD_FORCE_TEST", "EBUILD_PHASE", "EBUILD_PHASE_FUNC", "ECLASSDIR", "ECLASS_DEPTH", "ED", diff --git a/lib/portage/package/ebuild/config.py b/lib/portage/package/ebuild/config.py index 2981f7e31..cc2413989 100644 --- a/lib/portage/package/ebuild/config.py +++ b/lib/portage/package/ebuild/config.py @@ -1762,13 +1762,27 @@ class config(object): portage_iuse.update(built_use) self.configdict["pkg"]["IUSE_EFFECTIVE"] = \ " ".join(sorted(portage_iuse)) + + self.configdict["env"]["BASH_FUNC____in_portage_iuse%%"] = ( + "() { " + "if [[ ${#___PORTAGE_IUSE_HASH[@]} -lt 1 ]]; then " + " declare -gA ___PORTAGE_IUSE_HASH=(%s); " + "fi; " + "[[ -n ${___PORTAGE_IUSE_HASH[$1]} ]]; " + "}" ) % " ".join('["%s"]=1' % x for x in portage_iuse) else: portage_iuse = self._get_implicit_iuse() portage_iuse.update(explicit_iuse) - # PORTAGE_IUSE is not always needed so it's lazily evaluated. - self.configdict["env"].addLazySingleton( - "PORTAGE_IUSE", _lazy_iuse_regex, portage_iuse) + # The _get_implicit_iuse() returns a regular expression + # so we can't use the (faster) map. Fall back to + # implementing ___in_portage_iuse() the older/slower way. + + # PORTAGE_IUSE is not always needed so it's lazily evaluated. + self.configdict["env"].addLazySingleton( + "PORTAGE_IUSE", _lazy_iuse_regex, portage_iuse) + self.configdict["env"]["BASH_FUNC____in_portage_iuse%%"] = \ + "() { [[ $1 =~ ${PORTAGE_IUSE} ]]; }" ebuild_force_test = not restrict_test and \ self.get("EBUILD_FORCE_TEST") == "1" -- 2.21.0