tests/avocado: using several workers while testing

Commit Message

Pavel Dovgalyuk Nov. 16, 2022, 1:48 p.m. UTC

From: bakulinm <[e-mail]>

make check-avocado takes a lot of time, and avocado since version 91 has
multithreaded mode for running several tests simultaneously.
This patch allows to run "make check-avocado -j" to use all cores or,
for example, "make check-avocado -j4" to select number of workers to use.
By default ("make check-avocado") only one worker is used.

Changes:
1) Version of avocado in requirements.txt upgraded from 88.1 to <93
 (LTS version is used, as mentioned here
  https://avocado-framework.readthedocs.io/en/latest/releases/lts/92_0.html )
2) Makefile 4.1 (used in e.g. Ubuntu 18.04) doesn't provide number of jobs
 in $MAKEFLAGS, so python script from here
 https://stackoverflow.com/a/67247743/5936122 is used. Python script parses
 Makeflags, communicates with jobserver to get the number of available jobs
 and outputs it as a number, that is used as nrunner-max-parallel-tasks.
 If -j is used, it outputs total number of cores. If -j is not used at all,
 it returns 1. Limitations: script can probably report lower number of jobs
 than provided, if some slots are already taken by other makefiles, so make
 check-avocado should be run separately from other make tasks.

Signed-off-by: Maksim Bakulin <[e-mail]>
Signed-off-by: Pavel Dovgalyuk <[e-mail]>
---
 tests/Makefile.include |   20 ++++++++++++++++++--
 tests/jobs.py          |   42 ++++++++++++++++++++++++++++++++++++++++++
 tests/requirements.txt |    2 +-
 3 files changed, 61 insertions(+), 3 deletions(-)
 create mode 100644 tests/jobs.py

Comments

Daniel P. Berrangé Nov. 16, 2022, 2:06 p.m. UTC | #1

On Wed, Nov 16, 2022 at 04:48:10PM +0300, Pavel Dovgalyuk wrote:
> From: bakulinm <[e-mail]>
> 
> make check-avocado takes a lot of time, and avocado since version 91 has
> multithreaded mode for running several tests simultaneously.
> This patch allows to run "make check-avocado -j" to use all cores or,
> for example, "make check-avocado -j4" to select number of workers to use.
> By default ("make check-avocado") only one worker is used.
> 
> Changes:
> 1) Version of avocado in requirements.txt upgraded from 88.1 to <93
>  (LTS version is used, as mentioned here
>   https://avocado-framework.readthedocs.io/en/latest/releases/lts/92_0.html )
> 2) Makefile 4.1 (used in e.g. Ubuntu 18.04) doesn't provide number of jobs
>  in $MAKEFLAGS, so python script from here
>  https://stackoverflow.com/a/67247743/5936122 is used.


> diff --git a/tests/jobs.py b/tests/jobs.py
> new file mode 100644
> index 0000000000..a339192d97
> --- /dev/null
> +++ b/tests/jobs.py
> @@ -0,0 +1,42 @@

No license information or attribution put on this code that
you've said was directly copied from stackoverflow. AFAICT,
all content on stackoverflow is placed under the creative
commons license. This is not something we would generally
want to be applied to code in QEMU as that's generally
considered as a content license.

Unless the copied code is trivial (this case is not), then
stackoverflow should really only be used a learning resource,
and then code written from scratch without copying, so it
can be placed under the project's usual license.

> +import argparse, os
> +import sys
> +
> +def safe_int(s):
> +    try:
> +        return int(s)
> +    except:
> +        return -1
> +
> +class JobserverArgs:
> +    known_names = ["jobserver-fds","jobserver-auth"]
> +    def __init__(self):
> +        self.fds = "-1,-1"
> +
> +    @staticmethod
> +    def from_argv():
> +        ja = JobserverArgs()
> +        parser = argparse.ArgumentParser()
> +        for name in JobserverArgs.known_names:
> +            parser.add_argument('--'+name, dest="fds")
> +        parser.parse_known_args(namespace=ja)
> +        return ja
> +
> +    def get_fds(self):
> +        return tuple([safe_int(fd) for fd in (self.fds+",").split(",")][:2])
> +
> +fd_in, fd_out = JobserverArgs.from_argv().get_fds()
> +
> +if fd_in == -1 or fd_out == -1:
> +# if no jobserver is used, but -j is present, use total number of cpu cores
> +    if '-j' in sys.argv:
> +        print(os.cpu_count())
> +# use single thread
> +    else:
> +        print(1)
> +else:
> +    os.set_blocking(fd_in, False)
> +
> +    tokens = os.read(fd_in, 1024)
> +    os.write(fd_out, tokens)
> +
> +    print(len(tokens)+1)
> \ No newline at end of file
> diff --git a/tests/requirements.txt b/tests/requirements.txt
> index 0ba561b6bd..3b8c4d4706 100644
> --- a/tests/requirements.txt
> +++ b/tests/requirements.txt
> @@ -2,5 +2,5 @@
>  # in the tests/venv Python virtual environment. For more info,
>  # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
>  # Note that qemu.git/python/ is always implicitly installed.
> -avocado-framework==88.1
> +avocado-framework<93
>  pycdlib==1.11.0
> 
> 

With regards,
Daniel

[e-mail] Nov. 16, 2022, 2:37 p.m. UTC | #2

Valid point, thank you.

I can see three options what to do:
1) Ignore older version of make and assume that 4.2 or newer is used (4.1 is in Ubuntu 18.04 that is no longer supported as a build platform as I was told; 20.04 has 4.2). In this case make provides number of jobs in $MAKEFLAGS and this makes getting it trivial. In case of an older make only two options will be available: (default) single-threaded, and using all cores.
2) Use some environment variable to provide number of jobs instead of -j. For example NRUNNER_MAX_JOBS=4 make check-avocado. I don't like this option because it is not transparent to the user who doesn't know about this env_var. Maybe a message should be printed when no environment variable is provided (e.g. make check-avocado -> 1 worker is used for check-avocado testing. You can speed up the testing by providing desired number of workers with environment variable like this: NRUNNER_MAX_JOBS=16 make check-avocado)
3) A harder approach: rewrite python code from scratch, as you have suggested.

Which one should I choose?

November 16, 2022 6:06 PM, "Daniel P. Berrangé" <[e-mail]> wrote:

> On Wed, Nov 16, 2022 at 04:48:10PM +0300, Pavel Dovgalyuk wrote:
> 
>> From: bakulinm <[e-mail]>
>> 
>> make check-avocado takes a lot of time, and avocado since version 91 has
>> multithreaded mode for running several tests simultaneously.
>> This patch allows to run "make check-avocado -j" to use all cores or,
>> for example, "make check-avocado -j4" to select number of workers to use.
>> By default ("make check-avocado") only one worker is used.
>> 
>> Changes:
>> 1) Version of avocado in requirements.txt upgraded from 88.1 to <93
>> (LTS version is used, as mentioned here
>> https://avocado-framework.readthedocs.io/en/latest/releases/lts/92_0.html )
>> 2) Makefile 4.1 (used in e.g. Ubuntu 18.04) doesn't provide number of jobs
>> in $MAKEFLAGS, so python script from here
>> https://stackoverflow.com/a/67247743/5936122 is used.
>> 
>> diff --git a/tests/jobs.py b/tests/jobs.py
>> new file mode 100644
>> index 0000000000..a339192d97
>> --- /dev/null
>> +++ b/tests/jobs.py
>> @@ -0,0 +1,42 @@
> 
> No license information or attribution put on this code that
> you've said was directly copied from stackoverflow. AFAICT,
> all content on stackoverflow is placed under the creative
> commons license. This is not something we would generally
> want to be applied to code in QEMU as that's generally
> considered as a content license.
> 
> Unless the copied code is trivial (this case is not), then
> stackoverflow should really only be used a learning resource,
> and then code written from scratch without copying, so it
> can be placed under the project's usual license.
> 
>> +import argparse, os
>> +import sys
>> +
>> +def safe_int(s):
>> + try:
>> + return int(s)
>> + except:
>> + return -1
>> +
>> +class JobserverArgs:
>> + known_names = ["jobserver-fds","jobserver-auth"]
>> + def __init__(self):
>> + self.fds = "-1,-1"
>> +
>> + @staticmethod
>> + def from_argv():
>> + ja = JobserverArgs()
>> + parser = argparse.ArgumentParser()
>> + for name in JobserverArgs.known_names:
>> + parser.add_argument('--'+name, dest="fds")
>> + parser.parse_known_args(namespace=ja)
>> + return ja
>> +
>> + def get_fds(self):
>> + return tuple([safe_int(fd) for fd in (self.fds+",").split(",")][:2])
>> +
>> +fd_in, fd_out = JobserverArgs.from_argv().get_fds()
>> +
>> +if fd_in == -1 or fd_out == -1:
>> +# if no jobserver is used, but -j is present, use total number of cpu cores
>> + if '-j' in sys.argv:
>> + print(os.cpu_count())
>> +# use single thread
>> + else:
>> + print(1)
>> +else:
>> + os.set_blocking(fd_in, False)
>> +
>> + tokens = os.read(fd_in, 1024)
>> + os.write(fd_out, tokens)
>> +
>> + print(len(tokens)+1)
>> \ No newline at end of file
>> diff --git a/tests/requirements.txt b/tests/requirements.txt
>> index 0ba561b6bd..3b8c4d4706 100644
>> --- a/tests/requirements.txt
>> +++ b/tests/requirements.txt
>> @@ -2,5 +2,5 @@
>> # in the tests/venv Python virtual environment. For more info,
>> # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
>> # Note that qemu.git/python/ is always implicitly installed.
>> -avocado-framework==88.1
>> +avocado-framework<93
>> pycdlib==1.11.0
> 
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Daniel P. Berrangé Nov. 16, 2022, 2:42 p.m. UTC | #3

On Wed, Nov 16, 2022 at 02:37:43PM +0000, [e-mail] wrote:
> Valid point, thank you.
> 
> I can see three options what to do:
> e1) Ignore older version of make and assume that 4.2 or newer is used
> (4.1 is in Ubuntu 18.04 that is no longer supported as a build platform
> as I was told; 20.04 has 4.2). In this case make provides number of
> jobs in $MAKEFLAGS and this makes getting it trivial. In case of an
> older make only two options will be available: (default) single-threaded,
> and using all cores.

> Which one should I choose?

Ignore older make. Ubuntu 18.04 is not a platform we target anymore,
so we shouldn't be writing compat code for handling it, and in any
case degrading to single threaded or all-cores is fine fallback IMHO.

With regards,
Daniel

[e-mail] Nov. 22, 2022, 11:14 a.m. UTC | #4

Version of patch that uses sh script to parse MAKEFLAGS and set --nrunner-max-parallel-tasks accordingly. As already mentioned, works with Make 4.2 or newer, otherwise only single thread or all cores can be used.

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 9422ddaece..ee059dc135 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -93,6 +93,9 @@ TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
 TESTS_VENV_REQ=$(SRC_PATH)/tests/requirements.txt
 TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
 TESTS_PYTHON=$(TESTS_VENV_DIR)/bin/python3
+# 1 task is used by default
+NRUNNER_MAX_TASKS=--nrunner-max-parallel-tasks 1
+
 ifndef AVOCADO_TESTS
 	AVOCADO_TESTS=tests/avocado
 endif
@@ -111,6 +114,21 @@ quiet-venv-pip = $(quiet-@)$(call quiet-command-run, \
     $(TESTS_PYTHON) -m pip -q --disable-pip-version-check $1, \
     "VENVPIP","$1")
 
+get_avocado_max_tasks:
+# make 4.2 and later provide number of jobs in MAKEFLAGS
+# in earlier versions only -j can be used
+# Use N jobs if -jN isprovided. Use $(nproc) jobs if only -j is provided,
+# Use 1 job if no -j is found in MAKEFLAGS string.
+	$(eval MAKE_JOBS=$(shell (if (echo $(MAKEFLAGS) | grep -Eq ^.*-j\([0-9]+\).*$$); then \
+		(echo $(MAKEFLAGS) | sed -r 's/.*-j([0-9]+).*/\1/'); \
+	elif (echo $(MAKEFLAGS) | grep -Eq ^.*-j.*$$); then \
+		nproc; \
+	else \
+		echo 1; \
+	fi)))
+
+	$(eval NRUNNER_MAX_TASKS=--nrunner-max-parallel-tasks $$(MAKE_JOBS))
+
 $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
 	$(call quiet-command, $(PYTHON) -m venv $@, VENV, $@)
 	$(call quiet-venv-pip,install -e "$(SRC_PATH)/python/")
@@ -138,14 +156,14 @@ get-vm-image-fedora-31-%: check-venv
 # download all vm images, according to defined targets
 get-vm-images: check-venv $(patsubst %,get-vm-image-fedora-31-%, $(FEDORA_31_DOWNLOAD))
 
-check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images
+check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images get_avocado_max_tasks
 	$(call quiet-command, \
             $(TESTS_PYTHON) -m avocado \
             --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
             $(if $(AVOCADO_TAGS),, --filter-by-tags-include-empty \
 			--filter-by-tags-include-empty-key) \
             $(AVOCADO_CMDLINE_TAGS) \
-            $(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS), \
+            $(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS) $(NRUNNER_MAX_TASKS), \
             "AVOCADO", "tests/avocado")
 
 check-acceptance-deprecated-warning:
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 0ba561b6bd..3b8c4d4706 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,5 +2,5 @@
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
 # Note that qemu.git/python/ is always implicitly installed.
-avocado-framework==88.1
+avocado-framework<93
 pycdlib==1.11.0


November 16, 2022 6:42 PM, "Daniel P. Berrangé" <[e-mail]> wrote:

> On Wed, Nov 16, 2022 at 02:37:43PM +0000, [e-mail] wrote:
> 
>> Valid point, thank you.
>> 
>> I can see three options what to do:
>> e1) Ignore older version of make and assume that 4.2 or newer is used
>> (4.1 is in Ubuntu 18.04 that is no longer supported as a build platform
>> as I was told; 20.04 has 4.2). In this case make provides number of
>> jobs in $MAKEFLAGS and this makes getting it trivial. In case of an
>> older make only two options will be available: (default) single-threaded,
>> and using all cores.
>> 
>> Which one should I choose?
> 
> Ignore older make. Ubuntu 18.04 is not a platform we target anymore,
> so we shouldn't be writing compat code for handling it, and in any
> case degrading to single threaded or all-cores is fine fallback IMHO.
> 
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Patch

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 9422ddaece..6da5b6b45e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -93,6 +93,9 @@  TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
 TESTS_VENV_REQ=$(SRC_PATH)/tests/requirements.txt
 TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
 TESTS_PYTHON=$(TESTS_VENV_DIR)/bin/python3
+# 1 parallel task is used by default
+NRUNNER_MAX_TASKS=--nrunner-max-parallel-tasks
+
 ifndef AVOCADO_TESTS
 	AVOCADO_TESTS=tests/avocado
 endif
@@ -111,6 +114,19 @@  quiet-venv-pip = $(quiet-@)$(call quiet-command-run, \
     $(TESTS_PYTHON) -m pip -q --disable-pip-version-check $1, \
     "VENVPIP","$1")
 
+
+get_avocado_max_tasks:
+# Make doesn't have easy ways to get number of jobs, using python for that
+# Python script communicates with jobserver to detect number of jobs
+	+$(eval MAKE_JOBS:=$$(shell python3 $(SRC_PATH)/tests/jobs.py $(MAKEFLAGS)))
+	@echo Using $(MAKE_JOBS) jobs for avocado testing
+# If 2 jobs or more are used set nrunner-max-parallel-tasks accordingly
+	@if [ ${MAKE_JOBS} -gt 1 ] ; then \
+		$(eval NRUNNER_MAX_TASKS=--nrunner-max-parallel-tasks $$(MAKE_JOBS)) true; \
+	fi
+
+$(TESTS_VENV_REQ): get_avocado_max_tasks
+
 $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
 	$(call quiet-command, $(PYTHON) -m venv $@, VENV, $@)
 	$(call quiet-venv-pip,install -e "$(SRC_PATH)/python/")
@@ -145,8 +161,8 @@  check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images
             $(if $(AVOCADO_TAGS),, --filter-by-tags-include-empty \
 			--filter-by-tags-include-empty-key) \
             $(AVOCADO_CMDLINE_TAGS) \
-            $(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS), \
-            "AVOCADO", "tests/avocado")
+            $(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS) $(NRUNNER_MAX_TASKS), \
+            "AVOCADO", "tests/avocado" \)
 
 check-acceptance-deprecated-warning:
 	@echo
diff --git a/tests/jobs.py b/tests/jobs.py
new file mode 100644
index 0000000000..a339192d97
--- /dev/null
+++ b/tests/jobs.py
@@ -0,0 +1,42 @@ 
+import argparse, os
+import sys
+
+def safe_int(s):
+    try:
+        return int(s)
+    except:
+        return -1
+
+class JobserverArgs:
+    known_names = ["jobserver-fds","jobserver-auth"]
+    def __init__(self):
+        self.fds = "-1,-1"
+
+    @staticmethod
+    def from_argv():
+        ja = JobserverArgs()
+        parser = argparse.ArgumentParser()
+        for name in JobserverArgs.known_names:
+            parser.add_argument('--'+name, dest="fds")
+        parser.parse_known_args(namespace=ja)
+        return ja
+
+    def get_fds(self):
+        return tuple([safe_int(fd) for fd in (self.fds+",").split(",")][:2])
+
+fd_in, fd_out = JobserverArgs.from_argv().get_fds()
+
+if fd_in == -1 or fd_out == -1:
+# if no jobserver is used, but -j is present, use total number of cpu cores
+    if '-j' in sys.argv:
+        print(os.cpu_count())
+# use single thread
+    else:
+        print(1)
+else:
+    os.set_blocking(fd_in, False)
+
+    tokens = os.read(fd_in, 1024)
+    os.write(fd_out, tokens)
+
+    print(len(tokens)+1)
\ No newline at end of file
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 0ba561b6bd..3b8c4d4706 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -2,5 +2,5 @@ 
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
 # Note that qemu.git/python/ is always implicitly installed.
-avocado-framework==88.1
+avocado-framework<93
 pycdlib==1.11.0