[gentoo-dev] [PATCH v2 0/5] cargo.eclass: optimizations

[gentoo-dev] [PATCH v2 0/5] cargo.eclass: optimizations

[Michał Górny]

Hi,

Changes from v1: `@` is used to separate crate names and versions
rather than `/`.  Thanks to Denis Lisov for the suggestion.

-- 
Best regards,
Michał Górny


Michał Górny (5):
  eclass/tests: Add a minimal benchmark for cargo.eclass
  cargo.eclass: Add variable alternative to $(cargo_crate_uris)
  cargo.eclass: Optimize GIT_CRATES check
  cargo.eclass: Support separating crate names/versions via `@`
  cargo.eclass: Mark GIT_CRATES as pre-inherit

 eclass/cargo.eclass         | 127 ++++++++++++++++++++++--------------
 eclass/tests/cargo-bench.sh | 114 ++++++++++++++++++++++++++++++++
 2 files changed, 192 insertions(+), 49 deletions(-)
 create mode 100755 eclass/tests/cargo-bench.sh

-- 
2.41.0

[gentoo-dev] [PATCH v2 1/5] eclass/tests: Add a minimal benchmark for cargo.eclass

[Michał Górny]

The initial results on my machine are:

```
real  252 it/s
user  289 it/s
```

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/tests/cargo-bench.sh | 107 ++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100755 eclass/tests/cargo-bench.sh

diff --git a/eclass/tests/cargo-bench.sh b/eclass/tests/cargo-bench.sh
new file mode 100755
index 000000000000..cdc5e4431c14
--- /dev/null
+++ b/eclass/tests/cargo-bench.sh
@@ -0,0 +1,107 @@
+#!/bin/bash
+# Copyright 2023 Gentoo Authors
+# Distributed under the terms of the GNU General Public License v2
+
+EAPI=8
+source tests-common.sh || exit
+
+export LC_ALL=C
+
+ITERATIONS=1000
+RUNS=3
+
+doit() {
+	for (( i = 0; i < ITERATIONS; i++ )); do
+		SRC_URI="
+			$(cargo_crate_uris)
+		"
+	done
+}
+
+timeit() {
+	local real=()
+	local user=()
+	local x vr avg
+
+	for (( x = 0; x < RUNS; x++ )); do
+		while read tt tv; do
+			case ${tt} in
+				real) real+=( ${tv} );;
+				user) user+=( ${tv} );;
+			esac
+		done < <( ( time -p doit ) 2>&1 )
+	done
+
+	[[ ${#real[@]} == ${RUNS} ]] || die "Did not get ${RUNS} real times"
+	[[ ${#user[@]} == ${RUNS} ]] || die "Did not get ${RUNS} user times"
+
+	local xr avg
+	for x in real user; do
+		xr="${x}[*]"
+		avg=$(dc -S 3 -e "${ITERATIONS} ${RUNS} * ${!xr} + + / p")
+
+		printf '%s %4.0f it/s\n' "${x}" "${avg}"
+	done
+}
+
+# taken from cryptograpy-41.0.1
+CRATES="
+	Inflector-0.11.4
+	aliasable-0.1.3
+	asn1-0.15.2
+	asn1_derive-0.15.2
+	autocfg-1.1.0
+	base64-0.13.1
+	bitflags-1.3.2
+	cc-1.0.79
+	cfg-if-1.0.0
+	foreign-types-0.3.2
+	foreign-types-shared-0.1.1
+	indoc-1.0.9
+	libc-0.2.144
+	lock_api-0.4.9
+	memoffset-0.8.0
+	once_cell-1.17.2
+	openssl-0.10.54
+	openssl-macros-0.1.1
+	openssl-sys-0.9.88
+	ouroboros-0.15.6
+	ouroboros_macro-0.15.6
+	parking_lot-0.12.1
+	parking_lot_core-0.9.7
+	pem-1.1.1
+	pkg-config-0.3.27
+	proc-macro-error-1.0.4
+	proc-macro-error-attr-1.0.4
+	proc-macro2-1.0.59
+	pyo3-0.18.3
+	pyo3-build-config-0.18.3
+	pyo3-ffi-0.18.3
+	pyo3-macros-0.18.3
+	pyo3-macros-backend-0.18.3
+	quote-1.0.28
+	redox_syscall-0.2.16
+	scopeguard-1.1.0
+	smallvec-1.10.0
+	syn-1.0.109
+	syn-2.0.18
+	target-lexicon-0.12.7
+	unicode-ident-1.0.9
+	unindent-0.1.11
+	vcpkg-0.2.15
+	version_check-0.9.4
+	windows-sys-0.45.0
+	windows-targets-0.42.2
+	windows_aarch64_gnullvm-0.42.2
+	windows_aarch64_msvc-0.42.2
+	windows_i686_gnu-0.42.2
+	windows_i686_msvc-0.42.2
+	windows_x86_64_gnu-0.42.2
+	windows_x86_64_gnullvm-0.42.2
+	windows_x86_64_msvc-0.42.2
+"
+
+inherit cargo
+timeit
+
+texit
-- 
2.41.0

[gentoo-dev] [PATCH v2 2/5] cargo.eclass: Add variable alternative to $(cargo_crate_uris)

[Michał Górny]

Add a helper function that sets ${CARGO_CRATE_URIS} variable to make
it possible to set SRC_URI without subshells.  This gives a slight
speedup (~20%):

```
real  300 it/s
user  324 it/s
```

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/cargo.eclass         | 48 +++++++++++++++++++++++++------------
 eclass/tests/cargo-bench.sh |  4 +++-
 2 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
index 991e808d453f..4e0cd1e4de70 100644
--- a/eclass/cargo.eclass
+++ b/eclass/cargo.eclass
@@ -68,7 +68,7 @@ ECARGO_VENDOR="${ECARGO_HOME}/gentoo"
 # "
 # inherit cargo
 # ...
-# SRC_URI="$(cargo_crate_uris)"
+# SRC_URI="${CARGO_CRATE_URIS}"
 # @CODE
 
 # @ECLASS_VARIABLE: GIT_CRATES
@@ -162,31 +162,31 @@ ECARGO_VENDOR="${ECARGO_HOME}/gentoo"
 # group, and then switch over to building with FEATURES=userpriv.
 # Or vice-versa.
 
-# @FUNCTION: cargo_crate_uris
+# @ECLASS_VARIABLE: CARGO_CRATE_URIS
+# @OUTPUT_VARIABLE
+# @DESCRIPTION:
+# List of URIs to put in SRC_URI created from CRATES variable.
+
+# @FUNCTION: _cargo_set_crate_uris
+# @USAGE: <crates>
 # @DESCRIPTION:
 # Generates the URIs to put in SRC_URI to help fetch dependencies.
 # Constructs a list of crates from its arguments.
 # If no arguments are provided, it uses the CRATES variable.
-cargo_crate_uris() {
+# The value is set as CARGO_CRATE_URIS.
+_cargo_set_crate_uris() {
 	local -r regex='^([a-zA-Z0-9_\-]+)-([0-9]+\.[0-9]+\.[0-9]+.*)$'
-	local crate crates
-
-	if [[ -n ${@} ]]; then
-		crates="$@"
-	elif [[ -n ${CRATES} ]]; then
-		crates="${CRATES}"
-	else
-		eerror "CRATES variable is not defined and nothing passed as argument"
-		die "Can't generate SRC_URI from empty input"
-	fi
+	local crates=${1}
+	local crate
 
+	CARGO_CRATE_URIS=
 	for crate in ${crates}; do
 		local name version url
 		[[ $crate =~ $regex ]] || die "Could not parse name and version from crate: $crate"
 		name="${BASH_REMATCH[1]}"
 		version="${BASH_REMATCH[2]}"
 		url="https://crates.io/api/v1/crates/${name}/${version}/download -> ${crate}.crate"
-		echo "${url}"
+		CARGO_CRATE_URIS+="${url} "
 	done
 
 	local git_crates_type
@@ -214,12 +214,30 @@ cargo_crate_uris() {
 				;;
 			esac
 
-			printf -- '%s -> %s\n' "${crate_uri//%commit%/${commit}}" "${repo_name}-${commit}${repo_ext}.tar.gz"
+			CARGO_CRATE_URIS+="${crate_uri//%commit%/${commit}} -> ${repo_name}-${commit}${repo_ext}.tar.gz "
 		done
 	elif [[ -n ${git_crates_type} ]]; then
 		die "GIT_CRATE must be declared as an associative array"
 	fi
 }
+_cargo_set_crate_uris "${CRATES}"
+
+# @FUNCTION: cargo_crate_uris
+# @USAGE: [<crates>...]
+# @DESCRIPTION:
+# Generates the URIs to put in SRC_URI to help fetch dependencies.
+# Constructs a list of crates from its arguments.
+# If no arguments are provided, it uses the CRATES variable.
+cargo_crate_uris() {
+	local crates=${*-${CRATES}}
+	if [[ -z ${crates} ]]; then
+		eerror "CRATES variable is not defined and nothing passed as argument"
+		die "Can't generate SRC_URI from empty input"
+	fi
+
+	_cargo_set_crate_uris "${crates}"
+	echo "${CARGO_CRATE_URIS}"
+}
 
 # @FUNCTION: cargo_gen_config
 # @DESCRIPTION:
diff --git a/eclass/tests/cargo-bench.sh b/eclass/tests/cargo-bench.sh
index cdc5e4431c14..11b740f8dfcd 100755
--- a/eclass/tests/cargo-bench.sh
+++ b/eclass/tests/cargo-bench.sh
@@ -12,8 +12,9 @@ RUNS=3
 
 doit() {
 	for (( i = 0; i < ITERATIONS; i++ )); do
+		_cargo_set_crate_uris "${CRATES}"
 		SRC_URI="
-			$(cargo_crate_uris)
+			${CARGO_CRATE_URIS}
 		"
 	done
 }
@@ -102,6 +103,7 @@ CRATES="
 "
 
 inherit cargo
+
 timeit
 
 texit
-- 
2.41.0

[gentoo-dev] [PATCH v2 3/5] cargo.eclass: Optimize GIT_CRATES check

[Michał Górny]

Optimize the GIT_CRATES check to call `declare -p` only if the variable
is actually set.  In the vast majority of ebuilds using cargo.eclass,
it's not set, so the subshell-first approach is slowing things down.
With this change, the speed improves by another ~20%:

```
real  363 it/s
user  365 it/s
```

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/cargo.eclass | 58 ++++++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
index 4e0cd1e4de70..d97bb0df9348 100644
--- a/eclass/cargo.eclass
+++ b/eclass/cargo.eclass
@@ -189,35 +189,35 @@ _cargo_set_crate_uris() {
 		CARGO_CRATE_URIS+="${url} "
 	done
 
-	local git_crates_type
-	git_crates_type="$(declare -p GIT_CRATES 2>&-)"
-	if [[ ${git_crates_type} == "declare -A "* ]]; then
-		local crate commit crate_uri crate_dir repo_ext feat_expr
-
-		for crate in "${!GIT_CRATES[@]}"; do
-			IFS=';' read -r crate_uri commit crate_dir <<< "${GIT_CRATES[${crate}]}"
-
-			case "${crate_uri}" in
-				https://github.com/*)
-					repo_ext=".gh"
-					repo_name="${crate_uri##*/}"
-					crate_uri="${crate_uri%/}/archive/%commit%.tar.gz"
-				;;
-				https://gitlab.com/*)
-					repo_ext=".gl"
-					repo_name="${crate_uri##*/}"
-					crate_uri="${crate_uri%/}/-/archive/%commit%/${repo_name}-%commit%.tar.gz"
-				;;
-				*)
-					repo_ext=
-					repo_name="${crate}"
-				;;
-			esac
-
-			CARGO_CRATE_URIS+="${crate_uri//%commit%/${commit}} -> ${repo_name}-${commit}${repo_ext}.tar.gz "
-		done
-	elif [[ -n ${git_crates_type} ]]; then
-		die "GIT_CRATE must be declared as an associative array"
+	if declare -p GIT_CRATES &>/dev/null; then
+		if [[ $(declare -p GIT_CRATES) == "declare -A"* ]]; then
+			local crate commit crate_uri crate_dir repo_ext feat_expr
+
+			for crate in "${!GIT_CRATES[@]}"; do
+				IFS=';' read -r crate_uri commit crate_dir <<< "${GIT_CRATES[${crate}]}"
+
+				case "${crate_uri}" in
+					https://github.com/*)
+						repo_ext=".gh"
+						repo_name="${crate_uri##*/}"
+						crate_uri="${crate_uri%/}/archive/%commit%.tar.gz"
+					;;
+					https://gitlab.com/*)
+						repo_ext=".gl"
+						repo_name="${crate_uri##*/}"
+						crate_uri="${crate_uri%/}/-/archive/%commit%/${repo_name}-%commit%.tar.gz"
+					;;
+					*)
+						repo_ext=
+						repo_name="${crate}"
+					;;
+				esac
+
+				CARGO_CRATE_URIS+="${crate_uri//%commit%/${commit}} -> ${repo_name}-${commit}${repo_ext}.tar.gz "
+			done
+		else
+			die "GIT_CRATE must be declared as an associative array"
+		fi
 	fi
 }
 _cargo_set_crate_uris "${CRATES}"
-- 
2.41.0

[gentoo-dev] [PATCH v2 4/5] cargo.eclass: Support separating crate names/versions via `@`

[Michał Górny]

Support specifying crate names and versions separated by `@` character
rather than `-`.  Since `@` are not valid in crate names, this
makes splitting the tokens trivial and free of regular expressions.
Effectively, the `@` variant is roughly 180% faster:

```
 * CRATES with '@' separator
real  952 it/s
user  952 it/s
 * CRATES with '-' separator
real  339 it/s
user  339 it/s
```

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/cargo.eclass         |  24 +++++---
 eclass/tests/cargo-bench.sh | 111 +++++++++++++++++++-----------------
 2 files changed, 75 insertions(+), 60 deletions(-)

diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
index d97bb0df9348..8618c90bd986 100644
--- a/eclass/cargo.eclass
+++ b/eclass/cargo.eclass
@@ -59,12 +59,16 @@ ECARGO_VENDOR="${ECARGO_HOME}/gentoo"
 # Bash string containing all crates that are to be downloaded.
 # It is used by cargo_crate_uris.
 #
+# Ideally, crate names and versions should be separated by a `@`
+# character.  A legacy syntax using hyphen is also supported but it is
+# much slower.
+#
 # Example:
 # @CODE
 # CRATES="
-# metal-1.2.3
-# bar-4.5.6
-# iron_oxide-0.0.1
+# metal@1.2.3
+# bar@4.5.6
+# iron_oxide@0.0.1
 # "
 # inherit cargo
 # ...
@@ -182,10 +186,16 @@ _cargo_set_crate_uris() {
 	CARGO_CRATE_URIS=
 	for crate in ${crates}; do
 		local name version url
-		[[ $crate =~ $regex ]] || die "Could not parse name and version from crate: $crate"
-		name="${BASH_REMATCH[1]}"
-		version="${BASH_REMATCH[2]}"
-		url="https://crates.io/api/v1/crates/${name}/${version}/download -> ${crate}.crate"
+		if [[ ${crate} == *@* ]]; then
+			name=${crate%@*}
+			version=${crate##*@}
+		else
+			[[ ${crate} =~ ${regex} ]] ||
+				die "Could not parse name and version from crate: ${crate}"
+			name="${BASH_REMATCH[1]}"
+			version="${BASH_REMATCH[2]}"
+		fi
+		url="https://crates.io/api/v1/crates/${name}/${version}/download -> ${name}-${version}.crate"
 		CARGO_CRATE_URIS+="${url} "
 	done
 
diff --git a/eclass/tests/cargo-bench.sh b/eclass/tests/cargo-bench.sh
index 11b740f8dfcd..d30b04569905 100755
--- a/eclass/tests/cargo-bench.sh
+++ b/eclass/tests/cargo-bench.sh
@@ -47,63 +47,68 @@ timeit() {
 
 # taken from cryptograpy-41.0.1
 CRATES="
-	Inflector-0.11.4
-	aliasable-0.1.3
-	asn1-0.15.2
-	asn1_derive-0.15.2
-	autocfg-1.1.0
-	base64-0.13.1
-	bitflags-1.3.2
-	cc-1.0.79
-	cfg-if-1.0.0
-	foreign-types-0.3.2
-	foreign-types-shared-0.1.1
-	indoc-1.0.9
-	libc-0.2.144
-	lock_api-0.4.9
-	memoffset-0.8.0
-	once_cell-1.17.2
-	openssl-0.10.54
-	openssl-macros-0.1.1
-	openssl-sys-0.9.88
-	ouroboros-0.15.6
-	ouroboros_macro-0.15.6
-	parking_lot-0.12.1
-	parking_lot_core-0.9.7
-	pem-1.1.1
-	pkg-config-0.3.27
-	proc-macro-error-1.0.4
-	proc-macro-error-attr-1.0.4
-	proc-macro2-1.0.59
-	pyo3-0.18.3
-	pyo3-build-config-0.18.3
-	pyo3-ffi-0.18.3
-	pyo3-macros-0.18.3
-	pyo3-macros-backend-0.18.3
-	quote-1.0.28
-	redox_syscall-0.2.16
-	scopeguard-1.1.0
-	smallvec-1.10.0
-	syn-1.0.109
-	syn-2.0.18
-	target-lexicon-0.12.7
-	unicode-ident-1.0.9
-	unindent-0.1.11
-	vcpkg-0.2.15
-	version_check-0.9.4
-	windows-sys-0.45.0
-	windows-targets-0.42.2
-	windows_aarch64_gnullvm-0.42.2
-	windows_aarch64_msvc-0.42.2
-	windows_i686_gnu-0.42.2
-	windows_i686_msvc-0.42.2
-	windows_x86_64_gnu-0.42.2
-	windows_x86_64_gnullvm-0.42.2
-	windows_x86_64_msvc-0.42.2
+	Inflector@0.11.4
+	aliasable@0.1.3
+	asn1@0.15.2
+	asn1_derive@0.15.2
+	autocfg@1.1.0
+	base64@0.13.1
+	bitflags@1.3.2
+	cc@1.0.79
+	cfg-if@1.0.0
+	foreign-types@0.3.2
+	foreign-types-shared@0.1.1
+	indoc@1.0.9
+	libc@0.2.144
+	lock_api@0.4.9
+	memoffset@0.8.0
+	once_cell@1.17.2
+	openssl@0.10.54
+	openssl-macros@0.1.1
+	openssl-sys@0.9.88
+	ouroboros@0.15.6
+	ouroboros_macro@0.15.6
+	parking_lot@0.12.1
+	parking_lot_core@0.9.7
+	pem@1.1.1
+	pkg-config@0.3.27
+	proc-macro-error@1.0.4
+	proc-macro-error-attr@1.0.4
+	proc-macro2@1.0.59
+	pyo3@0.18.3
+	pyo3-build-config@0.18.3
+	pyo3-ffi@0.18.3
+	pyo3-macros@0.18.3
+	pyo3-macros-backend@0.18.3
+	quote@1.0.28
+	redox_syscall@0.2.16
+	scopeguard@1.1.0
+	smallvec@1.10.0
+	syn@1.0.109
+	syn@2.0.18
+	target-lexicon@0.12.7
+	unicode-ident@1.0.9
+	unindent@0.1.11
+	vcpkg@0.2.15
+	version_check@0.9.4
+	windows-sys@0.45.0
+	windows-targets@0.42.2
+	windows_aarch64_gnullvm@0.42.2
+	windows_aarch64_msvc@0.42.2
+	windows_i686_gnu@0.42.2
+	windows_i686_msvc@0.42.2
+	windows_x86_64_gnu@0.42.2
+	windows_x86_64_gnullvm@0.42.2
+	windows_x86_64_msvc@0.42.2
 "
 
 inherit cargo
 
+einfo "CRATES with '@' separator"
+timeit
+
+einfo "CRATES with '-' separator"
+CRATES=${CRATES//@/-}
 timeit
 
 texit
-- 
2.41.0

[gentoo-dev] [PATCH v2 5/5] cargo.eclass: Mark GIT_CRATES as pre-inherit

[Michał Górny]

The variable needs to be set before inherit in order for
${CARGO_CRATE_URIS} to be set correctly.  Currently all ebuilds using
GIT_CRATES except for one define it pre-inherit anyway, and this makes
it consistent with CRATES.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/cargo.eclass | 1 +
 1 file changed, 1 insertion(+)

diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass
index 8618c90bd986..2ff1f042ba79 100644
--- a/eclass/cargo.eclass
+++ b/eclass/cargo.eclass
@@ -77,6 +77,7 @@ ECARGO_VENDOR="${ECARGO_HOME}/gentoo"
 
 # @ECLASS_VARIABLE: GIT_CRATES
 # @DEFAULT_UNSET
+# @PRE_INHERIT
 # @DESCRIPTION:
 # Bash associative array containing all of the crates that are to be
 # fetched via git.  It is used by cargo_crate_uris.
-- 
2.41.0