On Fri, Jan 06, 2017 at 11:33:01AM +0800, Peter Xu wrote: > run_task.sh is getting slow. This patch is trying to make it faster by > running the tests concurrently. > > We provide a new parameter "-j" for the run_tests.sh, which can be used > to specify how many run queues we want for the tests. Default queue > length is 1, which is the old behavior. > > Quick test on my laptop (4 cores, 2 threads each) shows 3x speed boost: > > |-----------------+-----------| > | command | time used | > |-----------------+-----------| > | run_test.sh | 75s | > | run_test.sh -j8 | 27s | > |-----------------+-----------| > > Suggested-by: Radim Krčmář <rkrc...@redhat.com>
Radim suggested how to implement this, but not the idea of implementing it. The suggested-by tag is for ideas, not the code, and the idea (which is a good one) was yours, not Radim's. So the above tag should be removed. It's hard to credit Radim for his help here. Adding a signed-off-by to indicate co-authorship is probably the best you can do. Of course he's the maintainer and will add that when he merges anyway though... > Signed-off-by: Peter Xu <pet...@redhat.com> > --- > run_tests.sh | 12 ++++++++++-- > scripts/functions.bash | 17 ++++++++++++++++- > scripts/global.bash | 11 +++++++++++ > 3 files changed, 37 insertions(+), 3 deletions(-) > > diff --git a/run_tests.sh b/run_tests.sh > index e1bb3a6..a4fc895 100755 > --- a/run_tests.sh > +++ b/run_tests.sh > @@ -14,10 +14,11 @@ function usage() > { > cat <<EOF > > -Usage: $0 [-g group] [-h] [-v] > +Usage: $0 [-g group] [-h] [-v] [-j N] s/N/num_run_queues/ > > -g: Only execute tests in the given group > -h: Output this help text > + -j: Execute tests in parallel > -v: Enables verbose mode > > Set the environment variable QEMU=/path/to/qemu-system-ARCH to > @@ -29,7 +30,7 @@ EOF > RUNTIME_arch_run="./$TEST_DIR/run" > source scripts/runtime.bash > > -while getopts "g:hv" opt; do > +while getopts "g:hj:v" opt; do > case $opt in > g) > only_group=$OPTARG > @@ -38,6 +39,13 @@ while getopts "g:hv" opt; do > usage > exit > ;; > + j) > + ut_run_queues=$OPTARG > + if ! is_number "$ut_run_queues"; then > + echo "Invalid -j option: $ut_run_queues" > + exit 1 > + fi Instead of adding is_number() and just checking for numeric input, I'd check the input is > 0. A string input resolves as zero when treated as a number, so it'll fail too. > + ;; > v) > verbose="yes" > ;; > diff --git a/scripts/functions.bash b/scripts/functions.bash > index d1d2e1c..c6281f4 100644 > --- a/scripts/functions.bash > +++ b/scripts/functions.bash > @@ -1,9 +1,18 @@ > +source scripts/global.bash > + > function run_task() > { > local testname="$2" > > + while [[ "$(jobs | wc -l)" == $ut_run_queues ]]; do Bash arithmetic expressions should use (( ... )) instead of [[ ... ]] > + # wait for any background test to finish > + wait -n > + done > + > RUNTIME_log_file="${ut_log_dir}/${testname}.log" > - "$@" > + > + # start the testcase in the background > + "$@" & > } > > function for_each_unittest() > @@ -20,6 +29,8 @@ function for_each_unittest() > local accel > local timeout > > + trap "wait; exit 130" SIGINT > + > exec {fd}<"$unittests" > > while read -u $fd line; do > @@ -53,5 +64,9 @@ function for_each_unittest() > fi > done > run_task "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" > "$check" "$accel" "$timeout" > + > + # wait all task finish > + wait > + > exec {fd}<&- > } > diff --git a/scripts/global.bash b/scripts/global.bash > index 77b0b29..52095bd 100644 > --- a/scripts/global.bash > +++ b/scripts/global.bash > @@ -1,2 +1,13 @@ > : ${ut_log_dir:=logs} > : ${ut_log_summary:=${ut_log_dir}/SUMMARY} > +: ${ut_run_queues:=1} > + > +function ut_in_parallel() > +{ > + [[ $ut_run_queues != 1 ]] > +} I don't see any users of ut_in_parallel. Anyway I'd drop it and open code the condition, with ((...)), when needed. > + > +function is_number() > +{ > + [[ "$1" =~ ^[0-9]+$ ]] > +} > -- > 2.7.4 > > Thanks, drew