initscripts: populate-volatiles: Speed up processing

PATCH Superseded Joshua Watt 3 participants 5 messages Oct 11, 2018 – Oct 19, 2018

Share & tools

Joshua Watt

author

2018-10-11 18:08 UTC #0 lore raw

PATCH 1/1 initscripts: populate-volatiles: Speed up processing

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.

[YOCTO #12949]

Signed-off-by Joshua Watt <[email protected]>

---
 .../initscripts-1.0/populate-volatile.sh      | 31 +++++++++++++++++--
 1 file changed, 28 insertions(+), 3 deletions(-)

meta/recipes-core/initscripts/initscripts-1.0/populate-volatile.sh +28 -3

index 35316ec2baa..51eef0ca713 100755
@@ -154,8 +154,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 +234,32 @@ 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}"
+
+	# 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}" | sort`"
+	for file in ${CFGFILES}; do
+		cat "${CFGDIR}/${file}" >> "$TMP_FILE"
 	done
 
+	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

Signature · 1 line

2.17.1

Richard Purdie

2018-10-12 07:33 UTC #1 lore raw

Quoted 15 lines · Joshua Watt

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.

Quoted 3 lines

[YOCTO #12949]
Signed-off-by Joshua Watt <[email protected]>

As yet I little evidence for it but I think this caused:

https://autobuilder.yoctoproject.org/typhoon/#/builders/35/builds/69/steps/7/logs/step6c

You can see the config in:

https://autobuilder.yoctoproject.org/typhoon/#/builders/35/builds/69/steps/7/logs/stdio

the key bits for 6c are:

DISTRO_FEATURES_append = ' systemd'
VIRTUAL-RUNTIME_init_manager = 'sysvinit'

Cheers,

Richard

Joshua Watt

author

2018-10-12 15:39 UTC #2 lore raw

Quoted 17 lines · Richard Purdie

Quoted 16 lines · Joshua Watt
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.

Quoted 5 lines

Quoted 3 lines
[YOCTO #12949]
Signed-off-by Joshua Watt <[email protected]>
As yet I little evidence for it but I think this caused:

Yep, this was caused by my change. Turns out that for whatever reason this configuration doesn't have /var/volatile/tmp created before populate-volatiles.sh is run... not sure exactly *why* that is different, but this is actually the reason that the 00_core volatile file requirements aren't checked (e.g. there is no tmp directory to put the files until it is done).

Anyway, the fix is fairly simple. V2 is coming shortly.

Quoted 20 lines

https://autobuilder.yoctoproject.org/typhoon/#/builders/35/builds/69/
steps/7/logs/step6c
You can see the config in:
https://autobuilder.yoctoproject.org/typhoon/#/builders/35/builds/69/
steps/7/logs/stdio
the key bits for 6c are:
DISTRO_FEATURES_append = ' systemd'
VIRTUAL-RUNTIME_init_manager = 'sysvinit'
Cheers,
Richard

Signature · 1 line

Joshua Watt <[email protected]>

Joshua Watt

author

2018-10-12 16:24 UTC #3 lore raw

PATCH v2 1/1 initscripts: populate-volatiles: Speed up processing

In reply to Joshua Watt

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 <[email protected]>

---
 .../initscripts-1.0/populate-volatile.sh      | 37 +++++++++++++++++--
 1 file changed, 33 insertions(+), 4 deletions(-)

meta/recipes-core/initscripts/initscripts-1.0/populate-volatile.sh +33 -4

index 35316ec2baa..824f8f3a6ba 100755
@@ -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
 
+	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

Signature · 1 line

2.17.1

Mike Looijmans

2018-10-19 00:37 UTC #4 lore raw

Quoted 60 lines · Joshua Watt

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 <[email protected]>
---
  .../initscripts-1.0/populate-volatile.sh      | 37 +++++++++++++++++--
  1 file changed, 33 insertions(+), 4 deletions(-)
meta/recipes-core/initscripts/initscripts-1.0/populate-volatile.sh +11 -4
index 35316ec2baa..824f8f3a6ba 100755
@@ -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

meta/recipes-core/initscripts/initscripts-1.0/populate-volatile.sh then · line 252

Quoted 10 lines

meta/recipes-core/initscripts/initscripts-1.0/populate-volatile.sh +9 -0

+	# 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.)

Quoted 18 lines

meta/recipes-core/initscripts/initscripts-1.0/populate-volatile.sh +13 -0

+	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