stdlib: Tune down thread arc4random tests

Message ID 20230601155934.296647-1-adhemerval.zanella@linaro.org
State New
Headers
Series stdlib: Tune down thread arc4random tests |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm pending Patch applied

Commit Message

Adhemerval Zanella Netto June 1, 2023, 3:59 p.m. UTC
  There is no per-thread state on current arc4random implementation,
so limit the maximum number of threads.  The tests now run on 1s
instead of 8s on a aarch64 system with 160 cores.

Checked on aarch64-linux-gnu.
---
 stdlib/tst-arc4random-thread.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
  

Comments

Carlos O'Donell June 12, 2023, 9:34 p.m. UTC | #1
On 6/1/23 11:59, Adhemerval Zanella via Libc-alpha wrote:
> There is no per-thread state on current arc4random implementation,
> so limit the maximum number of threads.  The tests now run on 1s
> instead of 8s on a aarch64 system with 160 cores.

I agree this is a good idea...

> Checked on aarch64-linux-gnu.
> ---
>  stdlib/tst-arc4random-thread.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/stdlib/tst-arc4random-thread.c b/stdlib/tst-arc4random-thread.c
> index 010828920e..4f8cc27b03 100644
> --- a/stdlib/tst-arc4random-thread.c
> +++ b/stdlib/tst-arc4random-thread.c
> @@ -25,6 +25,7 @@
>  #include <support/namespace.h>
>  #include <support/support.h>
>  #include <support/xthread.h>
> +#include <sys/param.h>
>  
>  /* Number of arc4random_buf calls per thread.  */
>  enum { count_per_thread = 2048 };
> @@ -35,6 +36,9 @@ enum { inner_threads = 4 };
>  /* Number of threads launching other threads.  */
>  static int outer_threads = 1;
>  
> +/* Maximum number of other threads.  */
> +enum { max_outer_threads = 4 };
> +
>  /* Number of launching rounds performed by the outer threads.  */
>  enum { outer_rounds = 10 };
>  
> @@ -337,7 +341,8 @@ do_test (void)
>      {
>        unsigned int ncpus = CPU_COUNT (&cpuset);
>        /* Limit the number to not overload the system.  */
> -      outer_threads = (ncpus / 2) / inner_threads ?: 1;
> +      outer_threads = MIN ((ncpus / 2) / inner_threads ?: 1,
> +			   max_outer_threads);


... but may you please clean this up a little more?

Can we remove the conditional?

Use only two values in a MIN condition?

>      }
>  
>    printf ("info: outer_threads=%d inner_threads=%d\n", outer_threads,
  
Adhemerval Zanella Netto June 13, 2023, 1:28 p.m. UTC | #2
On 12/06/23 18:34, Carlos O'Donell wrote:
> On 6/1/23 11:59, Adhemerval Zanella via Libc-alpha wrote:
>> There is no per-thread state on current arc4random implementation,
>> so limit the maximum number of threads.  The tests now run on 1s
>> instead of 8s on a aarch64 system with 160 cores.
> 
> I agree this is a good idea...
> 
>> Checked on aarch64-linux-gnu.
>> ---
>>  stdlib/tst-arc4random-thread.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/stdlib/tst-arc4random-thread.c b/stdlib/tst-arc4random-thread.c
>> index 010828920e..4f8cc27b03 100644
>> --- a/stdlib/tst-arc4random-thread.c
>> +++ b/stdlib/tst-arc4random-thread.c
>> @@ -25,6 +25,7 @@
>>  #include <support/namespace.h>
>>  #include <support/support.h>
>>  #include <support/xthread.h>
>> +#include <sys/param.h>
>>  
>>  /* Number of arc4random_buf calls per thread.  */
>>  enum { count_per_thread = 2048 };
>> @@ -35,6 +36,9 @@ enum { inner_threads = 4 };
>>  /* Number of threads launching other threads.  */
>>  static int outer_threads = 1;
>>  
>> +/* Maximum number of other threads.  */
>> +enum { max_outer_threads = 4 };
>> +
>>  /* Number of launching rounds performed by the outer threads.  */
>>  enum { outer_rounds = 10 };
>>  
>> @@ -337,7 +341,8 @@ do_test (void)
>>      {
>>        unsigned int ncpus = CPU_COUNT (&cpuset);
>>        /* Limit the number to not overload the system.  */
>> -      outer_threads = (ncpus / 2) / inner_threads ?: 1;
>> +      outer_threads = MIN ((ncpus / 2) / inner_threads ?: 1,
>> +			   max_outer_threads);
> 
> 
> ... but may you please clean this up a little more?
> 
> Can we remove the conditional?
> 
> Use only two values in a MIN condition?

The '(ncpus / 2) / inner_threads ?: 1' is to avoid having a 0 from
the division, and the CPU_COUNT is to avoid having more threads than
CPUs.  Not sure how more we can simplify it, unless to setup a
static maximum number of threads.

> 
>>      }
>>  
>>    printf ("info: outer_threads=%d inner_threads=%d\n", outer_threads,
>
  
Adhemerval Zanella Netto Aug. 2, 2023, 4:31 p.m. UTC | #3
Ping.

On 01/06/23 12:59, Adhemerval Zanella wrote:
> There is no per-thread state on current arc4random implementation,
> so limit the maximum number of threads.  The tests now run on 1s
> instead of 8s on a aarch64 system with 160 cores.
> 
> Checked on aarch64-linux-gnu.
> ---
>  stdlib/tst-arc4random-thread.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/stdlib/tst-arc4random-thread.c b/stdlib/tst-arc4random-thread.c
> index 010828920e..4f8cc27b03 100644
> --- a/stdlib/tst-arc4random-thread.c
> +++ b/stdlib/tst-arc4random-thread.c
> @@ -25,6 +25,7 @@
>  #include <support/namespace.h>
>  #include <support/support.h>
>  #include <support/xthread.h>
> +#include <sys/param.h>
>  
>  /* Number of arc4random_buf calls per thread.  */
>  enum { count_per_thread = 2048 };
> @@ -35,6 +36,9 @@ enum { inner_threads = 4 };
>  /* Number of threads launching other threads.  */
>  static int outer_threads = 1;
>  
> +/* Maximum number of other threads.  */
> +enum { max_outer_threads = 4 };
> +
>  /* Number of launching rounds performed by the outer threads.  */
>  enum { outer_rounds = 10 };
>  
> @@ -337,7 +341,8 @@ do_test (void)
>      {
>        unsigned int ncpus = CPU_COUNT (&cpuset);
>        /* Limit the number to not overload the system.  */
> -      outer_threads = (ncpus / 2) / inner_threads ?: 1;
> +      outer_threads = MIN ((ncpus / 2) / inner_threads ?: 1,
> +			   max_outer_threads);
>      }
>  
>    printf ("info: outer_threads=%d inner_threads=%d\n", outer_threads,
  
Noah Goldstein Aug. 2, 2023, 4:44 p.m. UTC | #4
On Thu, Jun 1, 2023 at 11:00 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> There is no per-thread state on current arc4random implementation,
> so limit the maximum number of threads.  The tests now run on 1s
> instead of 8s on a aarch64 system with 160 cores.
>
> Checked on aarch64-linux-gnu.
> ---
>  stdlib/tst-arc4random-thread.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/stdlib/tst-arc4random-thread.c b/stdlib/tst-arc4random-thread.c
> index 010828920e..4f8cc27b03 100644
> --- a/stdlib/tst-arc4random-thread.c
> +++ b/stdlib/tst-arc4random-thread.c
> @@ -25,6 +25,7 @@
>  #include <support/namespace.h>
>  #include <support/support.h>
>  #include <support/xthread.h>
> +#include <sys/param.h>
>
>  /* Number of arc4random_buf calls per thread.  */
>  enum { count_per_thread = 2048 };
> @@ -35,6 +36,9 @@ enum { inner_threads = 4 };
>  /* Number of threads launching other threads.  */
>  static int outer_threads = 1;
>
> +/* Maximum number of other threads.  */
> +enum { max_outer_threads = 4 };
> +
>  /* Number of launching rounds performed by the outer threads.  */
>  enum { outer_rounds = 10 };
>
> @@ -337,7 +341,8 @@ do_test (void)
>      {
>        unsigned int ncpus = CPU_COUNT (&cpuset);
>        /* Limit the number to not overload the system.  */
> -      outer_threads = (ncpus / 2) / inner_threads ?: 1;
> +      outer_threads = MIN ((ncpus / 2) / inner_threads ?: 1,
> +                          max_outer_threads);
Since this is properly no random state how about just MAX(ncpus, 2)?
>      }
>
>    printf ("info: outer_threads=%d inner_threads=%d\n", outer_threads,
> --
> 2.34.1
>
  
Adhemerval Zanella Netto Aug. 2, 2023, 4:46 p.m. UTC | #5
On 02/08/23 13:44, Noah Goldstein wrote:
> On Thu, Jun 1, 2023 at 11:00 AM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> There is no per-thread state on current arc4random implementation,
>> so limit the maximum number of threads.  The tests now run on 1s
>> instead of 8s on a aarch64 system with 160 cores.
>>
>> Checked on aarch64-linux-gnu.
>> ---
>>  stdlib/tst-arc4random-thread.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/stdlib/tst-arc4random-thread.c b/stdlib/tst-arc4random-thread.c
>> index 010828920e..4f8cc27b03 100644
>> --- a/stdlib/tst-arc4random-thread.c
>> +++ b/stdlib/tst-arc4random-thread.c
>> @@ -25,6 +25,7 @@
>>  #include <support/namespace.h>
>>  #include <support/support.h>
>>  #include <support/xthread.h>
>> +#include <sys/param.h>
>>
>>  /* Number of arc4random_buf calls per thread.  */
>>  enum { count_per_thread = 2048 };
>> @@ -35,6 +36,9 @@ enum { inner_threads = 4 };
>>  /* Number of threads launching other threads.  */
>>  static int outer_threads = 1;
>>
>> +/* Maximum number of other threads.  */
>> +enum { max_outer_threads = 4 };
>> +
>>  /* Number of launching rounds performed by the outer threads.  */
>>  enum { outer_rounds = 10 };
>>
>> @@ -337,7 +341,8 @@ do_test (void)
>>      {
>>        unsigned int ncpus = CPU_COUNT (&cpuset);
>>        /* Limit the number to not overload the system.  */
>> -      outer_threads = (ncpus / 2) / inner_threads ?: 1;
>> +      outer_threads = MIN ((ncpus / 2) / inner_threads ?: 1,
>> +                          max_outer_threads);
> Since this is properly no random state how about just MAX(ncpus, 2)?

Works for me.
  

Patch

diff --git a/stdlib/tst-arc4random-thread.c b/stdlib/tst-arc4random-thread.c
index 010828920e..4f8cc27b03 100644
--- a/stdlib/tst-arc4random-thread.c
+++ b/stdlib/tst-arc4random-thread.c
@@ -25,6 +25,7 @@ 
 #include <support/namespace.h>
 #include <support/support.h>
 #include <support/xthread.h>
+#include <sys/param.h>
 
 /* Number of arc4random_buf calls per thread.  */
 enum { count_per_thread = 2048 };
@@ -35,6 +36,9 @@  enum { inner_threads = 4 };
 /* Number of threads launching other threads.  */
 static int outer_threads = 1;
 
+/* Maximum number of other threads.  */
+enum { max_outer_threads = 4 };
+
 /* Number of launching rounds performed by the outer threads.  */
 enum { outer_rounds = 10 };
 
@@ -337,7 +341,8 @@  do_test (void)
     {
       unsigned int ncpus = CPU_COUNT (&cpuset);
       /* Limit the number to not overload the system.  */
-      outer_threads = (ncpus / 2) / inner_threads ?: 1;
+      outer_threads = MIN ((ncpus / 2) / inner_threads ?: 1,
+			   max_outer_threads);
     }
 
   printf ("info: outer_threads=%d inner_threads=%d\n", outer_threads,