Patchwork [U-Boot,RFC] Allow for parallel builds and saved output

login
register
mail settings
Submitter Andy Fleming
Date April 13, 2011, 9:42 a.m.
Message ID <1302687759-1649-1-git-send-email-afleming@freescale.com>
Download mbox | patch
Permalink /patch/90974/
State RFC
Headers show

Comments

Andy Fleming - April 13, 2011, 9:42 a.m.
The MAKEALL script cleverly runs make with the appropriate options
to use all of the cores on the system, but your average U-Boot build
can't make much use of more than a few cores.  If you happen to have
a many-core server, your builds will leave most of the system idle.

In order to make full use of such a system, we need to build multiple
targets in parallel, and this requires directing make output into
multiple directories.  This patch adds support for a "-o" option, which
declares a build directory.  Each build will take place in
${output_prefix}/$target.  We launch up to 8 builds in parallel (with
each build still being told to use n+1 cores), and wait for all 8 to
finish before launching 8 more.  It's not ideal, but it is faster.

Once each build finishes, we run make clean on its directory, to reduce
the footprint, and remove all .depend files.

As a result, we are left with a build directory with all of the built
targets still there for use, which means anyone who wanted to use
MAKEALL as part of a test harness can now do so.

I feel certain that I've done some bone-headed things in this version,
but I tested by building all ppc, and it seemed to work (only took 16 min
on our 24-core server).

Signed-off-by: Andy Fleming <afleming@freescale.com>
---
 MAKEALL |   67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 59 insertions(+), 8 deletions(-)
Wolfgang Denk - April 30, 2011, 7:49 p.m.
Dear Andy Fleming,

In message <1302687759-1649-1-git-send-email-afleming@freescale.com> you wrote:
> The MAKEALL script cleverly runs make with the appropriate options
> to use all of the cores on the system, but your average U-Boot build
> can't make much use of more than a few cores.  If you happen to have
> a many-core server, your builds will leave most of the system idle.
...
> ${output_prefix}/$target.  We launch up to 8 builds in parallel (with
> each build still being told to use n+1 cores), and wait for all 8 to
> finish before launching 8 more.  It's not ideal, but it is faster.

How did you figure that 8 * (n+1) would be an efficient number of
tasks to use?  Does this not depend on a lot of factors, like number
of cores, relative speed of CPUs versus I/O subsystem, etc. ?

...
> +# Output
> +# It is now possible to save the output to a build directory.  This serves
> +# two purposes.  It allows you to keep the images for testing, and it
> +# allows you to launch the makes in tandem.  Pass in -o <dir>, and the build
> +# will happen in <dir>/$target/

Hm... this conflicts with / duplicates the function of BUILD_DIR,
doesn't it?

>  [ -d ${LOG_DIR} ] || mkdir ${LOG_DIR} || exit 1
>  
> +if [ "${output_prefix}" ] ; then
> +	[ -d ${output_prefix} ] || mkdir -p ${output_prefix} || exit 1
> +	[ -d "${output_prefix}/ERR" ] && rm -rf "${output_prefix}/ERR"
> +	mkdir "${output_prefix}/ERR"
> +fi

Should LOG_DIR not be adjusted, too?

> +	[ "$output_prefix" ] && export BUILD_DIR="${output_prefix}/${target}"

Ouch.  This means you are messing with user settings without warning
or any indication.  I don't like this.

If we have a conflict (and here we have one), there should be at least
errot / warning messages.

> +	if [ "$output_prefix" ] ; then
> +		${MAKE} clean
> +		find "${output_prefix}/$target" -type f -name '*.depend' | xargs rm

Why remove the .depend files and not any of the other temporary files?

> -	TOTAL_CNT=$((TOTAL_CNT + 1))

Um....

>  	${CROSS_COMPILE}size ${BUILD_DIR}/u-boot \
>  				| tee -a ${LOG_DIR}/$target.MAKELOG
>  }
> @@ -650,7 +681,20 @@ build_targets() {
>  		if [ -n "${list}" ] ; then
>  			build_targets ${list}
>  		else
> -			build_target ${t}
> +			if [ "$output_prefix" ] ; then
> +				build_target ${t} &
> +			else
> +				build_target ${t}
> +			fi
> +
> +			TOTAL_CNT=$((TOTAL_CNT + 1))
> +			CURRENT_COUNT=$((CURRENT_COUNT + 1))
> +		fi

So TOTAL_CNT and CURRENT_COUNT get only set in the "else" case?

> +		# Only run 8 builds concurrently
> +		if [ ${CURRENT_COUNT} -gt 8 ]; then

Magic number...

>  	echo "Boards compiled: ${TOTAL_CNT}"

Is this report correct for all use cases, now?

Best regards,

Wolfgang Denk
Andy Fleming - May 19, 2011, 12:11 a.m.
On Sat, Apr 30, 2011 at 2:49 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Andy Fleming,
>
> In message <1302687759-1649-1-git-send-email-afleming@freescale.com> you wrote:
>> The MAKEALL script cleverly runs make with the appropriate options
>> to use all of the cores on the system, but your average U-Boot build
>> can't make much use of more than a few cores.  If you happen to have
>> a many-core server, your builds will leave most of the system idle.
> ...
>> ${output_prefix}/$target.  We launch up to 8 builds in parallel (with
>> each build still being told to use n+1 cores), and wait for all 8 to
>> finish before launching 8 more.  It's not ideal, but it is faster.
>
> How did you figure that 8 * (n+1) would be an efficient number of
> tasks to use?  Does this not depend on a lot of factors, like number
> of cores, relative speed of CPUs versus I/O subsystem, etc. ?


Heh, yes, this is still quite hacky.  I'm curious if anyone has ideas
on this.  I ran this on a 24-core system, and if I didn't limit the
number of concurrent builds to something (I don't recall how many I
tried), then make quickly consumed so many resources that fork refused
to work until some things had finished, and I ended up hitting ctrl-c
repeatedly. I'm not much of a Makefile guru, so I'm hoping someone
might know of better ways to do such resource management. My thought
was that 8 was decently high, and shouldn't consume more than a
smaller desktop system can handle.


>
> ...
>> +# Output
>> +# It is now possible to save the output to a build directory.  This serves
>> +# two purposes.  It allows you to keep the images for testing, and it
>> +# allows you to launch the makes in tandem.  Pass in -o <dir>, and the build
>> +# will happen in <dir>/$target/
>
> Hm... this conflicts with / duplicates the function of BUILD_DIR,
> doesn't it?

Yes, the two usage models are slightly different in meaning. In order
to do concurrent builds, we have to build each board in a separate
directory. It should be possible to honor both, but we'd have to come
up with an accepted behavior. I also believe I had some issues with
using BUILD_DIR in the environment. But that may have been a temporary
issue brought on by syntax problems I was having. When I have some
free time again, I'll investigate.


>
>>  [ -d ${LOG_DIR} ] || mkdir ${LOG_DIR} || exit 1
>>
>> +if [ "${output_prefix}" ] ; then
>> +     [ -d ${output_prefix} ] || mkdir -p ${output_prefix} || exit 1
>> +     [ -d "${output_prefix}/ERR" ] && rm -rf "${output_prefix}/ERR"
>> +     mkdir "${output_prefix}/ERR"
>> +fi
>
> Should LOG_DIR not be adjusted, too?


It's not necessary, but it seems like a good idea.


>
>> +     [ "$output_prefix" ] && export BUILD_DIR="${output_prefix}/${target}"
>
> Ouch.  This means you are messing with user settings without warning
> or any indication.  I don't like this.
>
> If we have a conflict (and here we have one), there should be at least
> errot / warning messages.


Ok.


>
>> +     if [ "$output_prefix" ] ; then
>> +             ${MAKE} clean
>> +             find "${output_prefix}/$target" -type f -name '*.depend' | xargs rm
>
> Why remove the .depend files and not any of the other temporary files?

Ignorance/laziness.  :) This was a first pass at keeping the build dir
at a manageable size. When I did a "find" on the build directory after
make clean, the ".depend" files were the most prominent ones left. I
could add more in the next rev.

>
>> -     TOTAL_CNT=$((TOTAL_CNT + 1))
>
> Um....
>
>>       ${CROSS_COMPILE}size ${BUILD_DIR}/u-boot \
>>                               | tee -a ${LOG_DIR}/$target.MAKELOG
>>  }
>> @@ -650,7 +681,20 @@ build_targets() {
>>               if [ -n "${list}" ] ; then
>>                       build_targets ${list}
>>               else
>> -                     build_target ${t}
>> +                     if [ "$output_prefix" ] ; then
>> +                             build_target ${t} &
>> +                     else
>> +                             build_target ${t}
>> +                     fi
>> +
>> +                     TOTAL_CNT=$((TOTAL_CNT + 1))
>> +                     CURRENT_COUNT=$((CURRENT_COUNT + 1))
>> +             fi
>
> So TOTAL_CNT and CURRENT_COUNT get only set in the "else" case?

Right. Unless I misread the code, this is a recursive function.  I
only want to count "leaf" builds.  It's the same as before, but if the
counter were incremented in build_target, it would get concurrently
set 8 times to the same value.


>
>> +             # Only run 8 builds concurrently
>> +             if [ ${CURRENT_COUNT} -gt 8 ]; then
>
> Magic number...
>
>>       echo "Boards compiled: ${TOTAL_CNT}"
>
> Is this report correct for all use cases, now?


It should be the same as it was before.  I believe I checked it, since
I had a bug where it was clearly wrong in an earlier draft.  I did a
build of all ppc, and the number looked right.  I will double-check,
though.

Andy

Patch

diff --git a/MAKEALL b/MAKEALL
index e1b928f..071027c 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -49,9 +49,14 @@ 
 #
 #	MAKEALL -c mpc83xx -v freescale 4xx
 #
+# Output
+# It is now possible to save the output to a build directory.  This serves
+# two purposes.  It allows you to keep the images for testing, and it
+# allows you to launch the makes in tandem.  Pass in -o <dir>, and the build
+# will happen in <dir>/$target/
 #########################################################################
 
-SHORT_OPTS="a:c:v:s:"
+SHORT_OPTS="a:c:v:s:o:"
 LONG_OPTS="arch:,cpu:,vendor:,soc:"
 
 # Option processing based on util-linux-2.13/getopt-parse.bash
@@ -108,6 +113,10 @@  while true ; do
 		fi
 		SELECTED='y'
 		shift 2 ;;
+	-o)
+		# echo "Option OUTPUT: argument \`$2'"
+		output_prefix=$2
+		shift 2 ;;
 	--)
 		shift ; break ;;
 	*)
@@ -168,6 +177,13 @@  fi
 
 [ -d ${LOG_DIR} ] || mkdir ${LOG_DIR} || exit 1
 
+if [ "${output_prefix}" ] ; then
+	[ -d ${output_prefix} ] || mkdir -p ${output_prefix} || exit 1
+	[ -d "${output_prefix}/ERR" ] && rm -rf "${output_prefix}/ERR"
+	mkdir "${output_prefix}/ERR"
+fi
+
+
 LIST=""
 
 # Keep track of the number of builds and errors
@@ -610,11 +626,17 @@  LIST_sh="$(boards_by_arch sh)"
 
 LIST_sparc="$(boards_by_arch sparc)"
 
+CURRENT_COUNT=0
 #-----------------------------------------------------------------------
 
 build_target() {
 	target=$1
 
+	if [ "$output_prefix" ] ; then
+		[ -d "${output_prefix}/${target}" ] || mkdir ${output_prefix}/${target}
+	fi
+	[ "$output_prefix" ] && export BUILD_DIR="${output_prefix}/${target}"
+
 	${MAKE} distclean >/dev/null
 	${MAKE} -s ${target}_config
 
@@ -626,15 +648,24 @@  build_target() {
 		RC=1
 	fi
 
-	if [ -s ${LOG_DIR}/$target.ERR ] ; then
-		ERR_CNT=$((ERR_CNT + 1))
-		ERR_LIST="${ERR_LIST} $target"
+	if [ "$output_prefix" ] ; then
+		${MAKE} clean
+		find "${output_prefix}/$target" -type f -name '*.depend' | xargs rm
+
+		if [ -s ${LOG_DIR}/$target.ERR ] ; then
+			touch ${output_prefix}/ERR/$target
+		else
+			rm ${LOG_DIR}/$target.ERR
+		fi
 	else
-		rm ${LOG_DIR}/$target.ERR
+		if [ -s ${LOG_DIR}/$target.ERR ] ; then
+			ERR_CNT=$((ERR_CNT + 1))
+			ERR_LIST="${ERR_LIST} $target"
+		else
+			rm ${LOG_DIR}/$target.ERR
+		fi
 	fi
 
-	TOTAL_CNT=$((TOTAL_CNT + 1))
-
 	${CROSS_COMPILE}size ${BUILD_DIR}/u-boot \
 				| tee -a ${LOG_DIR}/$target.MAKELOG
 }
@@ -650,7 +681,20 @@  build_targets() {
 		if [ -n "${list}" ] ; then
 			build_targets ${list}
 		else
-			build_target ${t}
+			if [ "$output_prefix" ] ; then
+				build_target ${t} &
+			else
+				build_target ${t}
+			fi
+
+			TOTAL_CNT=$((TOTAL_CNT + 1))
+			CURRENT_COUNT=$((CURRENT_COUNT + 1))
+		fi
+
+		# Only run 8 builds concurrently
+		if [ ${CURRENT_COUNT} -gt 8 ]; then
+			CURRENT_COUNT=0;
+			wait;
 		fi
 	done
 }
@@ -658,6 +702,12 @@  build_targets() {
 #-----------------------------------------------------------------------
 
 print_stats() {
+	if [ "${output_prefix}" ] ; then
+		for file in `ls ${output_prefix}/ERR/`; do
+			ERR_LIST="${ERR_LIST} $file"
+		done
+		ERR_CNT=`ls -l ${output_prefix}/ERR/ | wc | awk '{print $1}'`
+	fi
 	echo ""
 	echo "--------------------- SUMMARY ----------------------------"
 	echo "Boards compiled: ${TOTAL_CNT}"
@@ -676,3 +726,4 @@  set -- ${SELECTED} "$@"
 # run PowerPC by default
 [ $# = 0 ] && set -- powerpc
 build_targets "$@"
+wait