[U-Boot] [PATCH] Allow for parallel builds and saved output

[Simon Glass]

Hi Mike,

On Sun, Nov 20, 2011 at 1:23 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Thursday 03 November 2011 03:28:29 Andy Fleming wrote:
>> +if [ ! "${BUILD_NBUILDS}" ] ; then
>> +     BUILD_NBUILDS="1"
>> +fi
>
> : ${BUILD_NBUILDS:=1}
>
>> +     if [ ! "${BUILD_DIR}" ] ; then
>> +             BUILD_DIR="./build"
>> +     fi
>
> : ${BUILD_DIR:=./build}
>
>> +     mkdir -p ${BUILD_DIR}/ERR
>
> mkdir -p "${BUILD_DIR}/ERR"
>
>>  if [ ! "${BUILD_DIR}" ] ; then
>>       BUILD_DIR="."
>>  fi
>
> : ${BUILD_DIR:=.}
>
>> +             mkdir -p ${output_dir}
>
> mkdir -p "${output_dir}"
>
>> +             ${MAKE} clean
>> +             find "${output_dir}" -type f -name '*.depend' | xargs rm
>
> why not use distclean and avoid the `find` ?  otherwise, this find should be:
>        find "${output_dir}" -type f -name '*.depend' -exec rm -f {} +
>
>> +                     ERR_CNT=$((ERR_CNT + 1))
>
>        : $(( ERR_CNT += 1 ))
>
>> -                     build_target ${t}
>> +                     if [ "$BUILD_NBUILDS" -gt 1 ] ; then
>> +                             build_target ${t} &
>> +                     else
>> +                             build_target ${t}
>> +                     fi
>
> you could keep this one line as:
>        build_target ${t} &
>        [ ${BUILD_NBUILDS} -eq 1 ] && wait
>
>> +                     TOTAL_CNT=$((TOTAL_CNT + 1))
>
> : $(( TOTAL_CNT += 1 ))
>
>> +                     CURRENT_COUNT=$((CURRENT_COUNT + 1))
>
> : $(( CURRENT_COUNT += 1 ))
>
>> +             # Limit number of parallel builds
>> +             if [ ${CURRENT_COUNT} -gt ${BUILD_NBUILDS} ] ; then
>> +                     CURRENT_COUNT=0;
>> +                     wait;
>>               fi
>
> you don't need those semicolons.  also, this is not as good as it should be:
> if you're running 10 jobs in parallel, you fork 10, and then you wait for all
> of them to finish before forking another set of 10.
>
> what you could do is something like:
>        JOB_IDX=0
>        JOB_IDX_FIRST=0
>        BUILD_NBUILDS=1
>
>        ... foreach target ... ; do
>                build_target &
>                pids[$(( JOB_IDX++ ))]=$!
>                if [ $(( JOB_IDX - JOB_IDX_FIRST )) -ge ${BUILD_NBUILDS} ] ; then
>                        wait ${pids[$(( JOB_IDX_FIRST++ ))]}
>                fi
>        done
>        wait
>
> this isn't perfect as it assumes the first job launched will always finish first,
> but it's a lot closer than the current code.

Since all the jobs are launched at the same time this is not really a
valid assumption is it? IMO on this point it's good enough as it is
for now...

Regards,
Simon

>
>> +             for file in `ls ${OUTPUT_PREFIX}/ERR/`; do
>> +                     ERR_LIST="${ERR_LIST} $file"
>> +             done
>
> ERR_LIST=$(ls ${OUTPUT_PREFIX}/ERR/)
>
>> +             ERR_CNT=`ls -l ${OUTPUT_PREFIX}/ERR/ | wc | awk '{print $1}'`
>
> you should use -1 instead of -l
> -mike
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>