Re: [Qemu-devel] [PATCH v2] tests: Run qtest cases in parallel

["Marc-André Lureau" <marcandre.lureau@gmail.com>]

Hi

On Thu, Sep 29, 2016 at 12:13 PM Fam Zheng <famz@redhat.com> wrote:

> Previously all qtest cases in a category, such as check-qtest-y, are
> executed in a single long gtester command. This patch separates each
> test into its own make target to allow better parallism, unless gcov is
> configured.
>
> Slightly reorganize the gcov enabled case too, but the execution
> sequence is kept.
>
> This saves >50% of the time (see below), which means a lot for patchew
> automatic testing given how 'make check' takes part in testings.
>
> On my machine:
>
> before:
>
>     $ make docker-test-quick@fedora J=8 &>/dev/null && \
>         time make docker-test-quick@fedora J=8 >/dev/null;
>
>     real    1m47.090s
>     user    0m1.599s
>     sys     0m0.258s
>
> after:
>
>     real    0m54.067s
>     user    0m1.447s
>     sys     0m0.253s
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> ---
>
> v2: Fix gcov case by not changing it. [Daniel]
> ---
>  tests/Makefile.include | 48
> ++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 36 insertions(+), 12 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 8162f6f..575030a 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -729,27 +729,51 @@ GCOV_OPTIONS = -n $(if $(V),-f,)
>  # gtester tests, possibly with verbose output
>
>  .PHONY: $(patsubst %, check-qtest-%, $(QTEST_TARGETS))
> -$(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%:
> $(check-qtest-y)
> -       $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda
> */*/*/*.gcda,)
> -       $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \
> +
> +run-tests = $(call quiet-command,\
> +               $(if $(QTEST_TARGET), \
> +
>  QTEST_QEMU_BINARY=$(QTEST_TARGET)-softmmu/qemu-system-$(QTEST_TARGET)) \
>                 QTEST_QEMU_IMG=qemu-img$(EXESUF) \
>                 MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 +
> 1))} \
> -               gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y)
> $(check-qtest-generic-y),"GTESTER $@")
> -       $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y)
> $(gcov-files-generic-y); do \
> +               gtester $1 $(GTESTER_OPTIONS) -m=$(SPEED),"GTESTER $2")
> +
> +ifneq ($(CONFIG_GCOV),)
> +
> +check-qtest-%: QTEST_TARGET=$*
> +$(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%:
> $(check-qtest-y)
> +       @rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda
> +       $(call run-tests, $(check-qtest-$*-y), $@)
> +       @for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
>           echo Gcov report for $$f:;\
>           $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
> -       done,)
> +       done
>
>  .PHONY: $(patsubst %, check-%, $(check-unit-y))
>  $(patsubst %, check-%, $(check-unit-y)): check-%: %
> -       $(if $(CONFIG_GCOV),@rm -f *.gcda */*.gcda */*/*.gcda
> */*/*/*.gcda,)
> -       $(call quiet-command, \
> -               MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 +
> 1))} \
> -               gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
> -       $(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y)
> $(gcov-files-generic-y); do \
> +       @rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda
> +       $(call run-tests, $*, $*)
> +       @for f in $(gcov-files-$(subst tests/,,$*)-y)
> $(gcov-files-generic-y); do \
>           echo Gcov report for $$f:;\
>           $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
> -       done,)
> +       done
> +
> +else
> +
> +run-test-%: tests/%
> +       $(call run-tests, $<, $<)
> +
> +$(foreach target, $(QTEST_TARGETS), \
> +       $(eval check-qtest-$(target): QTEST_TARGET := $(target)) \
> +       $(eval check-qtest-$(target): $(patsubst tests/%, run-test-%, \
> +                                             $(check-qtest-y) \
> +                                             $(check-qtest-$(target)-y) \
> +                                             $(check-qtest-generic-y))) \
> +)
> +$(patsubst %, check-%, $(check-unit-y)): check-tests/%: run-test-%
> +
> +endif
> +
> +.PHONY: $(patsubst %, check-%, $(check-unit-y))
>
>
it's quite difficult to read, but it looks correct to me. Something I wish
we had that we can perhaps add as well here (add dep on -softmmu build):

diff --git a/tests/Makefile.include b/tests/Makefile.include
index f5491c7..8e5dc72 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -743,6 +743,7 @@ run-tests = $(call quiet-command,\
 ifneq ($(CONFIG_GCOV),)

 check-qtest-%: QTEST_TARGET=$*
+check-qtest-%: subdir-$*-softmmu
 $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%:
$(check-qtest-y)
        @rm -f *.gcda */*.gcda */*/*.gcda */*/*/*.gcda
        $(call run-tests, $(check-qtest-$*-y), $@)
@@ -767,6 +768,7 @@ run-test-%: tests/%

 $(foreach target, $(QTEST_TARGETS), \
        $(eval check-qtest-$(target): QTEST_TARGET := $(target)) \
+       $(eval check-qtest-$(target): subdir-$(target)-softmmu) \
        $(eval check-qtest-$(target): $(patsubst tests/%, run-test-%, \
                                              $(check-qtest-y) \
                                              $(check-qtest-$(target)-y) \


I can post a seperate patch later though,

Similarly, qga test:

-tests/test-qga: tests/test-qga.o $(qtest-obj-y)
+tests/test-qga: tests/test-qga.o $(qtest-obj-y) qemu-ga$(EXESUF)

In any case
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



>  # gtester tests with XML output
>
> --
> 2.7.4
>
>
> --
Marc-André Lureau