[PATCH] meson: adjust timeouts for some slower tests

Paolo Bonzini posted 1 patch 3 years, 2 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210209174541.150011-1-pbonzini@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Thomas Huth <thuth@redhat.com>
tests/meson.build       |  6 ++++++
tests/qtest/meson.build | 15 +++++++++++++++
2 files changed, 21 insertions(+)
[PATCH] meson: adjust timeouts for some slower tests
Posted by Paolo Bonzini 3 years, 2 months ago
Adjust the timeouts for the longest running tests.  These are the
times that I measured and the corresponding timeouts.  For generic
qtests, the target that reported the longest runtime is included.

unit tests:
    test-crypto-tlscredsx509        13.15s   60s
    test-crypto-tlssession          14.12s   60s

qtests:
    ide-test                        13.65s   60s
    npcm7xx_watchdog_timer-test     15.02s   60s
    qos-test                        21.26s   60s   (i386)
    ahci-test                       22.18s   60s
    pxe-test                        26.51s   90s
    boot-serial-test                28.02s   90s   (sparc)
    prom-env-test                   28.86s   90s
    bios-tables-test                50.17s   90s   (aarch64)
    test-hmp                        57.15s   90s   (aarch64)
    npcm7xx_pwm-test                71.27s   120s
    migration-test                  97.09s   150s  (aarch64)
    qom-test                        139.20s  180s  (aarch64)

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/meson.build       |  6 ++++++
 tests/qtest/meson.build | 15 +++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/tests/meson.build b/tests/meson.build
index 0c939f89f7..fde21b6c8f 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -237,6 +237,11 @@ test_env = environment()
 test_env.set('G_TEST_SRCDIR', meson.current_source_dir())
 test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
 
+slow_tests = {
+  'test-crypto-tlscredsx509': 2,
+  'test-crypto-tlssession': 2
+}
+
 foreach test_name, extra: tests
   src = [test_name + '.c']
   deps = [qemuutil]
@@ -254,6 +259,7 @@ foreach test_name, extra: tests
        env: test_env,
        args: ['--tap', '-k'],
        protocol: 'tap',
+       timeout: 30 * slow_tests.get(test_name, 1),
        suite: ['unit'])
 endforeach
 
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 7e082be6fb..b1f8790b75 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -4,6 +4,20 @@ if not config_host.has_key('CONFIG_POSIX')
   subdir_done()
 endif
 
+slow_qtests = {
+  'ahci-test' : 2,
+  'bios-tables-test' : 3,
+  'boot-serial-test' : 3,
+  'ide-test' : 2,
+  'migration-test' : 5,
+  'npcm7xx_pwm-test': 4,
+  'npcm7xx_watchdog_timer-test': 2,
+  'prom-env-test' : 3,
+  'pxe-test' : 3,
+  'qos-test : 2,
+  'qom-test' : 6,
+  'test-hmp' : 3,
+]
 qtests_generic = [
   'cdrom-test',
   'device-introspect-test',
@@ -272,6 +286,7 @@ foreach dir : target_dirs
          env: qtest_env,
          args: ['--tap', '-k'],
          protocol: 'tap',
+         timeout: 30 * slow_qtests.get(test_name, 1),
          suite: ['qtest', 'qtest-' + target_base])
   endforeach
 endforeach
-- 
2.29.2


Re: [PATCH] meson: adjust timeouts for some slower tests
Posted by Richard Henderson 3 years, 2 months ago
On 2/9/21 9:45 AM, Paolo Bonzini wrote:
> Adjust the timeouts for the longest running tests.  These are the
> times that I measured and the corresponding timeouts.  For generic
> qtests, the target that reported the longest runtime is included.
> 
> unit tests:
>     test-crypto-tlscredsx509        13.15s   60s
>     test-crypto-tlssession          14.12s   60s
> 
> qtests:
>     ide-test                        13.65s   60s
>     npcm7xx_watchdog_timer-test     15.02s   60s
>     qos-test                        21.26s   60s   (i386)
>     ahci-test                       22.18s   60s
>     pxe-test                        26.51s   90s
>     boot-serial-test                28.02s   90s   (sparc)
>     prom-env-test                   28.86s   90s
>     bios-tables-test                50.17s   90s   (aarch64)
>     test-hmp                        57.15s   90s   (aarch64)
>     npcm7xx_pwm-test                71.27s   120s
>     migration-test                  97.09s   150s  (aarch64)
>     qom-test                        139.20s  180s  (aarch64)
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/meson.build       |  6 ++++++
>  tests/qtest/meson.build | 15 +++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/tests/meson.build b/tests/meson.build
> index 0c939f89f7..fde21b6c8f 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -237,6 +237,11 @@ test_env = environment()
>  test_env.set('G_TEST_SRCDIR', meson.current_source_dir())
>  test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
>  
> +slow_tests = {
> +  'test-crypto-tlscredsx509': 2,
> +  'test-crypto-tlssession': 2
> +}
> +
>  foreach test_name, extra: tests
>    src = [test_name + '.c']
>    deps = [qemuutil]
> @@ -254,6 +259,7 @@ foreach test_name, extra: tests
>         env: test_env,
>         args: ['--tap', '-k'],
>         protocol: 'tap',
> +       timeout: 30 * slow_tests.get(test_name, 1),
>         suite: ['unit'])
>  endforeach
>  
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 7e082be6fb..b1f8790b75 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -4,6 +4,20 @@ if not config_host.has_key('CONFIG_POSIX')
>    subdir_done()
>  endif
>  
> +slow_qtests = {
> +  'ahci-test' : 2,
> +  'bios-tables-test' : 3,
> +  'boot-serial-test' : 3,
> +  'ide-test' : 2,
> +  'migration-test' : 5,
> +  'npcm7xx_pwm-test': 4,
> +  'npcm7xx_watchdog_timer-test': 2,
> +  'prom-env-test' : 3,
> +  'pxe-test' : 3,
> +  'qos-test : 2,
> +  'qom-test' : 6,
> +  'test-hmp' : 3,
> +]

Mismatched brace?

>  qtests_generic = [
>    'cdrom-test',
>    'device-introspect-test',
> @@ -272,6 +286,7 @@ foreach dir : target_dirs
>           env: qtest_env,
>           args: ['--tap', '-k'],
>           protocol: 'tap',
> +         timeout: 30 * slow_qtests.get(test_name, 1),

Why scale here, rather than putting the real time in the dict?  Is there some
other scaling going on that I can't see?


r~

>           suite: ['qtest', 'qtest-' + target_base])
>    endforeach
>  endforeach
> 


Re: [PATCH] meson: adjust timeouts for some slower tests
Posted by Paolo Bonzini 3 years, 2 months ago
Il mar 9 feb 2021, 18:54 Richard Henderson <richard.henderson@linaro.org>
ha scritto:

> >           env: qtest_env,
> >           args: ['--tap', '-k'],
> >           protocol: 'tap',
> > +         timeout: 30 * slow_qtests.get(test_name, 1),
>
> Why scale here, rather than putting the real time in the dict?


Sure. In retrospect that isn't really useful.

Paolo

  Is there some
> other scaling going on that I can't see?
>
>
> r~
>
> >           suite: ['qtest', 'qtest-' + target_base])
> >    endforeach
> >  endforeach
> >
>
>
Re: [PATCH] meson: adjust timeouts for some slower tests
Posted by Daniel P. Berrangé 3 years, 2 months ago
On Tue, Feb 09, 2021 at 06:45:41PM +0100, Paolo Bonzini wrote:
> Adjust the timeouts for the longest running tests.  These are the
> times that I measured and the corresponding timeouts.  For generic
> qtests, the target that reported the longest runtime is included.
> 
> unit tests:
>     test-crypto-tlscredsx509        13.15s   60s
>     test-crypto-tlssession          14.12s   60s

The default meson timeout is 30 seconds which is enough for these
tests. Of course larger timeouts give more headroom.

If running on particularly slow environemnt (eg under TCG)
then it would be expecte dto instead use "meson --timeout-multiplier=10"
to make everything have more headroom.

So I wonder what threshold we need the extra headroom above
30 seconds in the default config ?


> qtests:
>     ide-test                        13.65s   60s
>     npcm7xx_watchdog_timer-test     15.02s   60s
>     qos-test                        21.26s   60s   (i386)
>     ahci-test                       22.18s   60s
>     pxe-test                        26.51s   90s
>     boot-serial-test                28.02s   90s   (sparc)
>     prom-env-test                   28.86s   90s
>     bios-tables-test                50.17s   90s   (aarch64)
>     test-hmp                        57.15s   90s   (aarch64)
>     npcm7xx_pwm-test                71.27s   120s
>     migration-test                  97.09s   150s  (aarch64)
>     qom-test                        139.20s  180s  (aarch64)
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/meson.build       |  6 ++++++
>  tests/qtest/meson.build | 15 +++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/tests/meson.build b/tests/meson.build
> index 0c939f89f7..fde21b6c8f 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -237,6 +237,11 @@ test_env = environment()
>  test_env.set('G_TEST_SRCDIR', meson.current_source_dir())
>  test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
>  
> +slow_tests = {
> +  'test-crypto-tlscredsx509': 2,
> +  'test-crypto-tlssession': 2
> +}
> +
>  foreach test_name, extra: tests
>    src = [test_name + '.c']
>    deps = [qemuutil]
> @@ -254,6 +259,7 @@ foreach test_name, extra: tests
>         env: test_env,
>         args: ['--tap', '-k'],
>         protocol: 'tap',
> +       timeout: 30 * slow_tests.get(test_name, 1),
>         suite: ['unit'])
>  endforeach
>  
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 7e082be6fb..b1f8790b75 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -4,6 +4,20 @@ if not config_host.has_key('CONFIG_POSIX')
>    subdir_done()
>  endif
>  
> +slow_qtests = {
> +  'ahci-test' : 2,
> +  'bios-tables-test' : 3,
> +  'boot-serial-test' : 3,
> +  'ide-test' : 2,
> +  'migration-test' : 5,
> +  'npcm7xx_pwm-test': 4,
> +  'npcm7xx_watchdog_timer-test': 2,
> +  'prom-env-test' : 3,
> +  'pxe-test' : 3,
> +  'qos-test : 2,
> +  'qom-test' : 6,
> +  'test-hmp' : 3,
> +]
>  qtests_generic = [
>    'cdrom-test',
>    'device-introspect-test',
> @@ -272,6 +286,7 @@ foreach dir : target_dirs
>           env: qtest_env,
>           args: ['--tap', '-k'],
>           protocol: 'tap',
> +         timeout: 30 * slow_qtests.get(test_name, 1),
>           suite: ['qtest', 'qtest-' + target_base])
>    endforeach
>  endforeach
> -- 
> 2.29.2
> 
> 

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 :|


Re: [PATCH] meson: adjust timeouts for some slower tests
Posted by Paolo Bonzini 3 years, 2 months ago
On 09/02/21 18:58, Daniel P. Berrangé wrote:
> On Tue, Feb 09, 2021 at 06:45:41PM +0100, Paolo Bonzini wrote:
>> Adjust the timeouts for the longest running tests.  These are the
>> times that I measured and the corresponding timeouts.  For generic
>> qtests, the target that reported the longest runtime is included.
>>
>> unit tests:
>>      test-crypto-tlscredsx509        13.15s   60s
>>      test-crypto-tlssession          14.12s   60s
> 
> The default meson timeout is 30 seconds which is enough for these
> tests. Of course larger timeouts give more headroom.

This was a relatively fast run, I've had them take as little as 7s and 
as much as 25s on the same machine.  I suspect it's because the machine 
has a very slow NFS home directory (yes those things still exist :)). 
In general a 2x-ish headroom makes sense in case someone is doing a 
build at the same time as a test run.

By the way, with Meson 0.57 there's the possibility of specifying 
"infinite timeout", and this could be used for the benchmarks.  Giving 
slower tests a higher priority is also a good idea, and even though this 
is not guaranteed in theory, Make ends up taking into account the 
priority as well.  With these tweaks "meson test" and "make check" 
(minus check-block of course) both clock at 2:20s, which is exactly the 
time it takes to run the longest-running test.

I will also give "meson test" a shot on the GitLab runners before 
posting v2, and see if it needs a timeout multiplier.

Paolo


Re: [PATCH] meson: adjust timeouts for some slower tests
Posted by Daniel P. Berrangé 3 years, 2 months ago
On Thu, Feb 11, 2021 at 12:30:35PM +0100, Paolo Bonzini wrote:
> On 09/02/21 18:58, Daniel P. Berrangé wrote:
> > On Tue, Feb 09, 2021 at 06:45:41PM +0100, Paolo Bonzini wrote:
> > > Adjust the timeouts for the longest running tests.  These are the
> > > times that I measured and the corresponding timeouts.  For generic
> > > qtests, the target that reported the longest runtime is included.
> > > 
> > > unit tests:
> > >      test-crypto-tlscredsx509        13.15s   60s
> > >      test-crypto-tlssession          14.12s   60s
> > 
> > The default meson timeout is 30 seconds which is enough for these
> > tests. Of course larger timeouts give more headroom.
> 
> This was a relatively fast run, I've had them take as little as 7s and as
> much as 25s on the same machine.  I suspect it's because the machine has a
> very slow NFS home directory (yes those things still exist :)). In general a
> 2x-ish headroom makes sense in case someone is doing a build at the same
> time as a test run.
> 
> By the way, with Meson 0.57 there's the possibility of specifying "infinite
> timeout", and this could be used for the benchmarks.  Giving slower tests a
> higher priority is also a good idea, and even though this is not guaranteed
> in theory, Make ends up taking into account the priority as well.  With
> these tweaks "meson test" and "make check" (minus check-block of course)
> both clock at 2:20s, which is exactly the time it takes to run the
> longest-running test.
> 
> I will also give "meson test" a shot on the GitLab runners before posting
> v2, and see if it needs a timeout multiplier.

I'd say we should set the default timeouts so that they are suitable for
our automated GitLab CI, and what we expect typical developer platforms.

Timeout multiplier should only be needed if users are running on outdated
hardware, or under slow emulation.


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 :|