Re: [PATCH v2] initscripts: populate-volatiles: Speed up processing

[Mike Looijmans <mike.looijmans@topic.nl>]

On 12-10-18 18:24, Joshua Watt wrote:
> Checking the requirements for each volatiles file in the
> populate-volatiles script can be very slow when there are a large number
> of volatiles files, easily consuming over 80% of the processing time.
> These checks don't usually uncover any problems so concatenate all the
> volatiles files together and process them as one large file for a "fast
> path" option. This ensures that the penalty for checking the
> requirements is only incurred once. In the event that checking the
> requirements for the unified file fails, fall back to the slow process
> of checking each one individually so that the offending one can be
> skipped.
> 
> The core file is handled separately because it is responsible for
> creating the temp directory used by check_requirements and thus must
> always run first and without having its requirements checked.
> 
> [YOCTO #12949]
> 
> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> ---
>   .../initscripts-1.0/populate-volatile.sh      | 37 +++++++++++++++++--
>   1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/meta/recipes-core/initscripts/initscripts-1.0/populate-volatile.sh b/meta/recipes-core/initscripts/initscripts-1.0/populate-volatile.sh
> index 35316ec2baa..824f8f3a6ba 100755
> --- a/meta/recipes-core/initscripts/initscripts-1.0/populate-volatile.sh
> +++ b/meta/recipes-core/initscripts/initscripts-1.0/populate-volatile.sh
> @@ -112,7 +112,6 @@ check_requirements() {
>   	}
>   
>   	CFGFILE="$1"
> -	[ `basename "${CFGFILE}"` = "${COREDEF}" ] && return 0
>   
>   	TMP_INTERMED="${TMPROOT}/tmp.$$"
>   	TMP_DEFINED="${TMPROOT}/tmpdefined.$$"
> @@ -154,8 +153,11 @@ check_requirements() {
>   
>   apply_cfgfile() {
>   	CFGFILE="$1"
> +	SKIP_REQUIREMENTS="$2"
>   
> -	check_requirements "${CFGFILE}" || {
> +	[ "${VERBOSE}" != "no" ] && echo "Applying ${CFGFILE}"
> +
> +	[ "${SKIP_REQUIREMENTS}" == "yes" ] || check_requirements "${CFGFILE}" || {
>   		echo "Skipping ${CFGFILE}"
>   		return 1
>   	}
> @@ -231,10 +233,37 @@ then
>   	sh ${ROOT_DIR}/etc/volatile.cache
>   else
>   	rm -f ${ROOT_DIR}/etc/volatile.cache ${ROOT_DIR}/etc/volatile.cache.build
> -	for file in `ls -1 "${CFGDIR}" | sort`; do
> -		apply_cfgfile "${CFGDIR}/${file}"
> +
> +	# Apply the core file with out checking requirements. ${TMPROOT} is
> +	# needed by check_requirements but is setup by this file, so it must be
> +	# processed first and without being checked.
> +	[ -e "${CFGDIR}/${COREDEF}" ] && apply_cfgfile "${CFGDIR}/${COREDEF}" "yes"
> +
> +	# Fast path: check_requirements is slow and most of the time doesn't
> +	# find any problems. If there are a lot of config files, it is much
> +	# faster to to concatenate them all together and process them once to
> +	# avoid the overhead of calling check_requirements repeatedly
> +	TMP_FILE="${TMPROOT}/tmp_volatile.$$"
> +	rm -f "$TMP_FILE"
> +
> +	CFGFILES="`ls -1 "${CFGDIR}" | grep -v "^${COREDEF}\$" | sort`"
> +	for file in ${CFGFILES}; do
> +		cat "${CFGDIR}/${file}" >> "$TMP_FILE"
>   	done

You can replace that loop with this:

cat `ls -1 "${CFGDIR}" | grep -v "^${COREDEF}\$" | sort` > "$TMP_FILE"

(And the ">" instead of ">>" makes that you can remove the "rm" command as well.)

>   
> +	if check_requirements "$TMP_FILE"
> +	then
> +		apply_cfgfile "$TMP_FILE" "yes"
> +	else
> +		# Slow path: One or more config files failed requirements.
> +		# Process each one individually so the offending one can be
> +		# skipped
> +		for file in ${CFGFILES}; do
> +			apply_cfgfile "${CFGDIR}/${file}"
> +		done
> +	fi
> +	rm "$TMP_FILE"
> +
>   	[ -e ${ROOT_DIR}/etc/volatile.cache.build ] && sync && mv ${ROOT_DIR}/etc/volatile.cache.build ${ROOT_DIR}/etc/volatile.cache
>   fi
>   
>