[RFC,1/3] test/meson: auto detect number of cores

Message ID 20190329172241.11916-2-aconole@redhat.com (mailing list archive)
State RFC, archived
Delegated to: Thomas Monjalon
Headers
Series ci: enable unit tests for non-aarch64 platforms |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK

Commit Message

Aaron Conole March 29, 2019, 5:22 p.m. UTC
  Some environments do not provide a minimum 4 cores for running tests.  This
allows those environments to still execute 'ninja test' without causing
multiple failures.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 app/test/meson.build | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
  

Comments

David Marchand April 1, 2019, 1:48 p.m. UTC | #1
On Fri, Mar 29, 2019 at 6:23 PM Aaron Conole <aconole@redhat.com> wrote:

> Some environments do not provide a minimum 4 cores for running tests.  This
> allows those environments to still execute 'ninja test' without causing
> multiple failures.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  app/test/meson.build | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/app/test/meson.build b/app/test/meson.build
> index ddb4d09ae..975b38daa 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -337,10 +337,15 @@ if get_option('tests')
>         timeout_seconds = 600
>         timeout_seconds_fast = 10
>
> +       # Retreive the number of CPU cores
> +       num_cores = run_command('lscpu',
> '-p=cpu').stdout().strip().split('\n')[-1]
> +       num_cores_arg = '-l 0-' + num_cores
>

Seeing how we can reduce the core number, we can at least cap it to 4 and
not use all the cores on bigger systems.
But, thinking again, do you know if there is a need for more than 2 cores
in the existing tests ?


+
> +       test_args = [num_cores_arg, '-n 4']
>         foreach arg : fast_parallel_test_names
>                 test(arg, dpdk_test,
>                         env : ['DPDK_TEST=' + arg],
> -                       args : ['-c f','-n 4', '--file-prefix=@0@
> '.format(arg)],
> +                       args : test_args + ['--file-prefix=@0@
> '.format(arg)],
>                         timeout : timeout_seconds_fast,
>                         suite : 'fast-tests')
>         endforeach
> @@ -348,6 +353,7 @@ if get_option('tests')
>         foreach arg : fast_non_parallel_test_names
>                 test(arg, dpdk_test,
>                         env : ['DPDK_TEST=' + arg],
> +                       args : test_args + ['--file-prefix=@0@
> '.format(arg)],
>

I don't understand this part.

(How|) was it working before ?
Is there some default args ? I could not find it.

                        timeout : timeout_seconds_fast,
>                         is_parallel : false,
>                         suite : 'fast-tests')
>
  
Aaron Conole April 1, 2019, 2:07 p.m. UTC | #2
David Marchand <david.marchand@redhat.com> writes:

Thanks for the review, David!

> On Fri, Mar 29, 2019 at 6:23 PM Aaron Conole <aconole@redhat.com> wrote:
>
>  Some environments do not provide a minimum 4 cores for running tests.  This
>  allows those environments to still execute 'ninja test' without causing
>  multiple failures.
>
>  Signed-off-by: Aaron Conole <aconole@redhat.com>
>  ---
>   app/test/meson.build | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
>  diff --git a/app/test/meson.build b/app/test/meson.build
>  index ddb4d09ae..975b38daa 100644
>  --- a/app/test/meson.build
>  +++ b/app/test/meson.build
>  @@ -337,10 +337,15 @@ if get_option('tests')
>          timeout_seconds = 600
>          timeout_seconds_fast = 10
>
>  +       # Retreive the number of CPU cores
>  +       num_cores = run_command('lscpu', '-p=cpu').stdout().strip().split('\n')[-1]
>  +       num_cores_arg = '-l 0-' + num_cores
>
> Seeing how we can reduce the core number, we can at least cap it to 4 and not use all the cores on bigger
> systems.
> But, thinking again, do you know if there is a need for more than 2 cores in the existing tests ?

Probably not.  At least, the systems in the travis environment only
provide 2 cores and most of the tests pass.  OTOH, some still fail and I
need to investigate them a bit more.  Some of the test case failures are
ex. eal flags failures and maybe they do pass some '-c f' as part of
the test case, which would fail on systems without 4 cores.

>  +
>  +       test_args = [num_cores_arg, '-n 4']

Whoops!  I think, this '-n 4' option can probably be removed as well.  I
think the memory channel config is optional, and I believe there's no
need to try and tune it.

>          foreach arg : fast_parallel_test_names
>                  test(arg, dpdk_test,
>                          env : ['DPDK_TEST=' + arg],
>  -                       args : ['-c f','-n 4', '--file-prefix=@0@'.format(arg)],
>  +                       args : test_args + ['--file-prefix=@0@'.format(arg)],
>                          timeout : timeout_seconds_fast,
>                          suite : 'fast-tests')
>          endforeach
>  @@ -348,6 +353,7 @@ if get_option('tests')
>          foreach arg : fast_non_parallel_test_names
>                  test(arg, dpdk_test,
>                          env : ['DPDK_TEST=' + arg],
>  +                       args : test_args + ['--file-prefix=@0@'.format(arg)],
>
> I don't understand this part.
>
> (How|) was it working before ?

It did work before.

> Is there some default args ? I could not find it.

I think it does try to do some auto-detection.  I did extend this
because it seemed to make sense, and I have some future work to allow
passing '--no-huge' (in case we have a CI system that doesn't allow
allocating hugepages).  Maybe it doesn't make sense to add that right
now, though.  I'll try without it.

>                          timeout : timeout_seconds_fast,
>                          is_parallel : false,
>                          suite : 'fast-tests')
  
Pattan, Reshma April 1, 2019, 4:23 p.m. UTC | #3
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Aaron Conole
> Sent: Friday, March 29, 2019 5:23 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [RFC 1/3] test/meson: auto detect number of cores
> 
> Some environments do not provide a minimum 4 cores for running tests.  This
> allows those environments to still execute 'ninja test' without causing multiple
> failures.
> 

FYI, there is a patch http://patches.dpdk.org/patch/50850/ doing something similar.

--file-prefix is not supported on FreeBSD, so how about you also include the related fix from above patch link into your patch series?


Thanks,
Reshma
  
Aaron Conole April 1, 2019, 5:48 p.m. UTC | #4
"Pattan, Reshma" <reshma.pattan@intel.com> writes:

>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Aaron Conole
>> Sent: Friday, March 29, 2019 5:23 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [RFC 1/3] test/meson: auto detect number of cores
>> 
>> Some environments do not provide a minimum 4 cores for running tests.  This
>> allows those environments to still execute 'ninja test' without causing multiple
>> failures.
>> 
>
> FYI, there is a patch http://patches.dpdk.org/patch/50850/ doing something similar.
>
> --file-prefix is not supported on FreeBSD, so how about you also
> include the related fix from above patch link into your patch series?

Is that other patch going to be accepted?  Then I can drop this patch
from my series.  Otherwise, I'll fold in such a change with my next
version.

> Thanks,
> Reshma
  
Thomas Monjalon April 1, 2019, 7:39 p.m. UTC | #5
01/04/2019 19:48, Aaron Conole:
> "Pattan, Reshma" <reshma.pattan@intel.com> writes:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Aaron Conole
> >> 
> >> Some environments do not provide a minimum 4 cores for running tests.  This
> >> allows those environments to still execute 'ninja test' without causing multiple
> >> failures.
> >> 
> >
> > FYI, there is a patch http://patches.dpdk.org/patch/50850/ doing something similar.
> >
> > --file-prefix is not supported on FreeBSD, so how about you also
> > include the related fix from above patch link into your patch series?
> 
> Is that other patch going to be accepted?  Then I can drop this patch
> from my series.  Otherwise, I'll fold in such a change with my next
> version.

Sorry, I'm late on applying patches for unit tests.
As I am not sure which one will get the best review first,
you should keep it with a note that it conflicts with the other one.
  
Aaron Conole April 1, 2019, 7:58 p.m. UTC | #6
Thomas Monjalon <thomas@monjalon.net> writes:

> 01/04/2019 19:48, Aaron Conole:
>> "Pattan, Reshma" <reshma.pattan@intel.com> writes:
>> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Aaron Conole
>> >> 
>> >> Some environments do not provide a minimum 4 cores for running tests.  This
>> >> allows those environments to still execute 'ninja test' without causing multiple
>> >> failures.
>> >> 
>> >
>> > FYI, there is a patch http://patches.dpdk.org/patch/50850/ doing something similar.
>> >
>> > --file-prefix is not supported on FreeBSD, so how about you also
>> > include the related fix from above patch link into your patch series?
>> 
>> Is that other patch going to be accepted?  Then I can drop this patch
>> from my series.  Otherwise, I'll fold in such a change with my next
>> version.
>
> Sorry, I'm late on applying patches for unit tests.
> As I am not sure which one will get the best review first,
> you should keep it with a note that it conflicts with the other one.

Okay - thanks, Thomas!
  

Patch

diff --git a/app/test/meson.build b/app/test/meson.build
index ddb4d09ae..975b38daa 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -337,10 +337,15 @@  if get_option('tests')
 	timeout_seconds = 600
 	timeout_seconds_fast = 10
 
+	# Retreive the number of CPU cores
+	num_cores = run_command('lscpu', '-p=cpu').stdout().strip().split('\n')[-1]
+	num_cores_arg = '-l 0-' + num_cores
+
+	test_args = [num_cores_arg, '-n 4']
 	foreach arg : fast_parallel_test_names
 		test(arg, dpdk_test,
 			env : ['DPDK_TEST=' + arg],
-			args : ['-c f','-n 4', '--file-prefix=@0@'.format(arg)],
+			args : test_args + ['--file-prefix=@0@'.format(arg)],
 			timeout : timeout_seconds_fast,
 			suite : 'fast-tests')
 	endforeach
@@ -348,6 +353,7 @@  if get_option('tests')
 	foreach arg : fast_non_parallel_test_names
 		test(arg, dpdk_test,
 			env : ['DPDK_TEST=' + arg],
+			args : test_args + ['--file-prefix=@0@'.format(arg)],
 			timeout : timeout_seconds_fast,
 			is_parallel : false,
 			suite : 'fast-tests')
@@ -356,6 +362,7 @@  if get_option('tests')
 	foreach arg : perf_test_names
 		test(arg, dpdk_test,
 		env : ['DPDK_TEST=' + arg],
+		args : test_args + ['--file-prefix=@0@'.format(arg)],
 		timeout : timeout_seconds,
 		is_parallel : false,
 		suite : 'perf-tests')
@@ -364,6 +371,7 @@  if get_option('tests')
 	foreach arg : driver_test_names
 		test(arg, dpdk_test,
 			env : ['DPDK_TEST=' + arg],
+			args : test_args + ['--file-prefix=@0@'.format(arg)],
 			timeout : timeout_seconds,
 			is_parallel : false,
 			suite : 'driver-tests')
@@ -372,6 +380,7 @@  if get_option('tests')
 	foreach arg : dump_test_names
 		test(arg, dpdk_test,
 			env : ['DPDK_TEST=' + arg],
+			args : test_args + ['--file-prefix=@0@'.format(arg)],
 			timeout : timeout_seconds,
 			is_parallel : false,
 			suite : 'debug-tests')