| Message ID | 1302687759-1649-1-git-send-email-afleming@freescale.com |
|---|---|
| State | RFC |
| Headers | show |
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
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
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
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(-)