diff mbox series

test-memcpy.c: Double TIMEOUT to (8 * 60)

Message ID 20211107160829.1004634-1-hjl.tools@gmail.com
State Committed
Commit a6a9c1a36b48f304c8a2138c147d88a448161366
Headers
Series test-memcpy.c: Double TIMEOUT to (8 * 60) |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

H.J. Lu Nov. 7, 2021, 4:08 p.m. UTC 
  commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
Author: Noah Goldstein <goldstein.w.n@gmail.com>
Date:   Mon Nov 1 00:49:48 2021 -0500

    string: Make tests birdirectional test-memcpy.c

    This commit updates the memcpy tests to test both dst > src and dst <
    src. This is because there is logic in the code based on the

    Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
    Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

significantly increased the number of tests.  On Intel Core i7-1165G7,
test-memcpy takes 120 seconds to run when machine is idle.  Double
TIMEOUT to (8 * 60) for test-memcpy to avoid timeout when machine is
under heavy load.
---
 string/test-memcpy.c | 1 +
 string/test-string.h | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

Comments

Noah Goldstein Nov. 7, 2021, 5:40 p.m. UTC | #1 
On Sun, Nov 7, 2021 at 10:08 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
> Author: Noah Goldstein <goldstein.w.n@gmail.com>
> Date:   Mon Nov 1 00:49:48 2021 -0500
>
>     string: Make tests birdirectional test-memcpy.c
>
>     This commit updates the memcpy tests to test both dst > src and dst <
>     src. This is because there is logic in the code based on the
>
>     Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
>     Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
>
> significantly increased the number of tests.  On Intel Core i7-1165G7,
> test-memcpy takes 120 seconds to run when machine is idle.  Double
> TIMEOUT to (8 * 60) for test-memcpy to avoid timeout when machine is
> under heavy load.
> ---
>  string/test-memcpy.c | 1 +
>  string/test-string.h | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> index 3b0f3127b7..101d51c487 100644
> --- a/string/test-memcpy.c
> +++ b/string/test-memcpy.c
> @@ -22,6 +22,7 @@
>  # define MIN_PAGE_SIZE 131072
>  # define TEST_MAIN
>  # define TEST_NAME "memcpy"
> +# define TIMEOUT (8 * 60)
>  # include "test-string.h"
>
>  char *simple_memcpy (char *, const char *, size_t);
> diff --git a/string/test-string.h b/string/test-string.h
> index 8ee00a04b1..9a6b76daa4 100644
> --- a/string/test-string.h
> +++ b/string/test-string.h
> @@ -68,7 +68,9 @@ extern impl_t __start_impls[], __stop_impls[];
>
>
>  # define TEST_FUNCTION test_main
> -# define TIMEOUT (4 * 60)
> +# ifndef TIMEOUT
> +#  define TIMEOUT (4 * 60)
> +# endif
>  # define OPT_ITERATIONS 10000
>  # define OPT_RANDOM 10001
>  # define OPT_SEED 10002
> --
> 2.33.1
>

LGTM
Adhemerval Zanella Nov. 8, 2021, 11:39 a.m. UTC | #2 
On 07/11/2021 13:08, H.J. Lu via Libc-alpha wrote:
> commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
> Author: Noah Goldstein <goldstein.w.n@gmail.com>
> Date:   Mon Nov 1 00:49:48 2021 -0500
> 
>     string: Make tests birdirectional test-memcpy.c
> 
>     This commit updates the memcpy tests to test both dst > src and dst <
>     src. This is because there is logic in the code based on the
> 
>     Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
>     Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> 
> significantly increased the number of tests.  On Intel Core i7-1165G7,
> test-memcpy takes 120 seconds to run when machine is idle.  Double
> TIMEOUT to (8 * 60) for test-memcpy to avoid timeout when machine is
> under heavy load.

Shouldn't we split the test instead? If it takes 120s on a high-end chip,
it might take way more in other chips.  It also optimizes the testsuite,
since it would allow to better use parallel builds.

> ---
>  string/test-memcpy.c | 1 +
>  string/test-string.h | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> index 3b0f3127b7..101d51c487 100644
> --- a/string/test-memcpy.c
> +++ b/string/test-memcpy.c
> @@ -22,6 +22,7 @@
>  # define MIN_PAGE_SIZE 131072
>  # define TEST_MAIN
>  # define TEST_NAME "memcpy"
> +# define TIMEOUT (8 * 60)
>  # include "test-string.h"
>  
>  char *simple_memcpy (char *, const char *, size_t);
> diff --git a/string/test-string.h b/string/test-string.h
> index 8ee00a04b1..9a6b76daa4 100644
> --- a/string/test-string.h
> +++ b/string/test-string.h
> @@ -68,7 +68,9 @@ extern impl_t __start_impls[], __stop_impls[];
>  
>  
>  # define TEST_FUNCTION test_main
> -# define TIMEOUT (4 * 60)
> +# ifndef TIMEOUT
> +#  define TIMEOUT (4 * 60)
> +# endif
>  # define OPT_ITERATIONS 10000
>  # define OPT_RANDOM 10001
>  # define OPT_SEED 10002
>
H.J. Lu Nov. 8, 2021, 1:13 p.m. UTC | #3 
On Mon, Nov 8, 2021 at 3:39 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 07/11/2021 13:08, H.J. Lu via Libc-alpha wrote:
> > commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
> > Author: Noah Goldstein <goldstein.w.n@gmail.com>
> > Date:   Mon Nov 1 00:49:48 2021 -0500
> >
> >     string: Make tests birdirectional test-memcpy.c
> >
> >     This commit updates the memcpy tests to test both dst > src and dst <
> >     src. This is because there is logic in the code based on the
> >
> >     Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
> >     Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> >
> > significantly increased the number of tests.  On Intel Core i7-1165G7,
> > test-memcpy takes 120 seconds to run when machine is idle.  Double
> > TIMEOUT to (8 * 60) for test-memcpy to avoid timeout when machine is
> > under heavy load.
>
> Shouldn't we split the test instead? If it takes 120s on a high-end chip,
> it might take way more in other chips.  It also optimizes the testsuite,
> since it would allow to better use parallel builds.

This sounds like a good idea.  Noah, can you do that?

Thanks.

> > ---
> >  string/test-memcpy.c | 1 +
> >  string/test-string.h | 4 +++-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> > index 3b0f3127b7..101d51c487 100644
> > --- a/string/test-memcpy.c
> > +++ b/string/test-memcpy.c
> > @@ -22,6 +22,7 @@
> >  # define MIN_PAGE_SIZE 131072
> >  # define TEST_MAIN
> >  # define TEST_NAME "memcpy"
> > +# define TIMEOUT (8 * 60)
> >  # include "test-string.h"
> >
> >  char *simple_memcpy (char *, const char *, size_t);
> > diff --git a/string/test-string.h b/string/test-string.h
> > index 8ee00a04b1..9a6b76daa4 100644
> > --- a/string/test-string.h
> > +++ b/string/test-string.h
> > @@ -68,7 +68,9 @@ extern impl_t __start_impls[], __stop_impls[];
> >
> >
> >  # define TEST_FUNCTION test_main
> > -# define TIMEOUT (4 * 60)
> > +# ifndef TIMEOUT
> > +#  define TIMEOUT (4 * 60)
> > +# endif
> >  # define OPT_ITERATIONS 10000
> >  # define OPT_RANDOM 10001
> >  # define OPT_SEED 10002
> >
Noah Goldstein Nov. 8, 2021, 2:02 p.m. UTC | #4 
On Mon, Nov 8, 2021 at 7:14 AM H.J. Lu via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On Mon, Nov 8, 2021 at 3:39 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
> >
> >
> >
> > On 07/11/2021 13:08, H.J. Lu via Libc-alpha wrote:
> > > commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
> > > Author: Noah Goldstein <goldstein.w.n@gmail.com>
> > > Date:   Mon Nov 1 00:49:48 2021 -0500
> > >
> > >     string: Make tests birdirectional test-memcpy.c
> > >
> > >     This commit updates the memcpy tests to test both dst > src and dst <
> > >     src. This is because there is logic in the code based on the
> > >
> > >     Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
> > >     Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> > >
> > > significantly increased the number of tests.  On Intel Core i7-1165G7,
> > > test-memcpy takes 120 seconds to run when machine is idle.  Double
> > > TIMEOUT to (8 * 60) for test-memcpy to avoid timeout when machine is
> > > under heavy load.
> >
> > Shouldn't we split the test instead? If it takes 120s on a high-end chip,
> > it might take way more in other chips.  It also optimizes the testsuite,
> > since it would allow to better use parallel builds.
>
> This sounds like a good idea.  Noah, can you do that?

Yeah.

>
> Thanks.
>
> > > ---
> > >  string/test-memcpy.c | 1 +
> > >  string/test-string.h | 4 +++-
> > >  2 files changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> > > index 3b0f3127b7..101d51c487 100644
> > > --- a/string/test-memcpy.c
> > > +++ b/string/test-memcpy.c
> > > @@ -22,6 +22,7 @@
> > >  # define MIN_PAGE_SIZE 131072
> > >  # define TEST_MAIN
> > >  # define TEST_NAME "memcpy"
> > > +# define TIMEOUT (8 * 60)
> > >  # include "test-string.h"
> > >
> > >  char *simple_memcpy (char *, const char *, size_t);
> > > diff --git a/string/test-string.h b/string/test-string.h
> > > index 8ee00a04b1..9a6b76daa4 100644
> > > --- a/string/test-string.h
> > > +++ b/string/test-string.h
> > > @@ -68,7 +68,9 @@ extern impl_t __start_impls[], __stop_impls[];
> > >
> > >
> > >  # define TEST_FUNCTION test_main
> > > -# define TIMEOUT (4 * 60)
> > > +# ifndef TIMEOUT
> > > +#  define TIMEOUT (4 * 60)
> > > +# endif
> > >  # define OPT_ITERATIONS 10000
> > >  # define OPT_RANDOM 10001
> > >  # define OPT_SEED 10002
> > >
>
>
>
> --
> H.J.
Noah Goldstein Nov. 8, 2021, 7:43 p.m. UTC | #5 
On Mon, Nov 8, 2021 at 8:02 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Mon, Nov 8, 2021 at 7:14 AM H.J. Lu via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > On Mon, Nov 8, 2021 at 3:39 AM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> > >
> > >
> > >
> > > On 07/11/2021 13:08, H.J. Lu via Libc-alpha wrote:
> > > > commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
> > > > Author: Noah Goldstein <goldstein.w.n@gmail.com>
> > > > Date:   Mon Nov 1 00:49:48 2021 -0500
> > > >
> > > >     string: Make tests birdirectional test-memcpy.c
> > > >
> > > >     This commit updates the memcpy tests to test both dst > src and dst <
> > > >     src. This is because there is logic in the code based on the
> > > >
> > > >     Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
> > > >     Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> > > >
> > > > significantly increased the number of tests.  On Intel Core i7-1165G7,
> > > > test-memcpy takes 120 seconds to run when machine is idle.  Double
> > > > TIMEOUT to (8 * 60) for test-memcpy to avoid timeout when machine is
> > > > under heavy load.
> > >
> > > Shouldn't we split the test instead? If it takes 120s on a high-end chip,
> > > it might take way more in other chips.  It also optimizes the testsuite,
> > > since it would allow to better use parallel builds.
> >
> > This sounds like a good idea.  Noah, can you do that?

The reality is 95% of the extra test time is from the additional large
memcpy tests in `#ifdef DO_EXTRA_TESTS` region. Not sure
there is really any value in making a seperate test for them as
I think the runtime would still require the larger timeout.

Do we still want it? Alternatively I could cut out some of the tests.

>
> Yeah.
>
> >
> > Thanks.
> >
> > > > ---
> > > >  string/test-memcpy.c | 1 +
> > > >  string/test-string.h | 4 +++-
> > > >  2 files changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/string/test-memcpy.c b/string/test-memcpy.c
> > > > index 3b0f3127b7..101d51c487 100644
> > > > --- a/string/test-memcpy.c
> > > > +++ b/string/test-memcpy.c
> > > > @@ -22,6 +22,7 @@
> > > >  # define MIN_PAGE_SIZE 131072
> > > >  # define TEST_MAIN
> > > >  # define TEST_NAME "memcpy"
> > > > +# define TIMEOUT (8 * 60)
> > > >  # include "test-string.h"
> > > >
> > > >  char *simple_memcpy (char *, const char *, size_t);
> > > > diff --git a/string/test-string.h b/string/test-string.h
> > > > index 8ee00a04b1..9a6b76daa4 100644
> > > > --- a/string/test-string.h
> > > > +++ b/string/test-string.h
> > > > @@ -68,7 +68,9 @@ extern impl_t __start_impls[], __stop_impls[];
> > > >
> > > >
> > > >  # define TEST_FUNCTION test_main
> > > > -# define TIMEOUT (4 * 60)
> > > > +# ifndef TIMEOUT
> > > > +#  define TIMEOUT (4 * 60)
> > > > +# endif
> > > >  # define OPT_ITERATIONS 10000
> > > >  # define OPT_RANDOM 10001
> > > >  # define OPT_SEED 10002
> > > >
> >
> >
> >
> > --
> > H.J.
Adhemerval Zanella Nov. 8, 2021, 7:46 p.m. UTC | #6 
On 08/11/2021 16:43, Noah Goldstein wrote:
> On Mon, Nov 8, 2021 at 8:02 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>>
>> On Mon, Nov 8, 2021 at 7:14 AM H.J. Lu via Libc-alpha
>> <libc-alpha@sourceware.org> wrote:
>>>
>>> On Mon, Nov 8, 2021 at 3:39 AM Adhemerval Zanella
>>> <adhemerval.zanella@linaro.org> wrote:
>>>>
>>>>
>>>>
>>>> On 07/11/2021 13:08, H.J. Lu via Libc-alpha wrote:
>>>>> commit d585ba47fcda99fdf228e3e45a01b11a15efbc5a
>>>>> Author: Noah Goldstein <goldstein.w.n@gmail.com>
>>>>> Date:   Mon Nov 1 00:49:48 2021 -0500
>>>>>
>>>>>     string: Make tests birdirectional test-memcpy.c
>>>>>
>>>>>     This commit updates the memcpy tests to test both dst > src and dst <
>>>>>     src. This is because there is logic in the code based on the
>>>>>
>>>>>     Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
>>>>>     Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
>>>>>
>>>>> significantly increased the number of tests.  On Intel Core i7-1165G7,
>>>>> test-memcpy takes 120 seconds to run when machine is idle.  Double
>>>>> TIMEOUT to (8 * 60) for test-memcpy to avoid timeout when machine is
>>>>> under heavy load.
>>>>
>>>> Shouldn't we split the test instead? If it takes 120s on a high-end chip,
>>>> it might take way more in other chips.  It also optimizes the testsuite,
>>>> since it would allow to better use parallel builds.
>>>
>>> This sounds like a good idea.  Noah, can you do that?
> 
> The reality is 95% of the extra test time is from the additional large
> memcpy tests in `#ifdef DO_EXTRA_TESTS` region. Not sure
> there is really any value in making a seperate test for them as
> I think the runtime would still require the larger timeout.
> 
> Do we still want it? Alternatively I could cut out some of the tests.

Maybe spawn multiple processes to cover different sizes within the loop
(something analogous to #omp for)?
diff mbox series

Patch

diff --git a/string/test-memcpy.c b/string/test-memcpy.c
index 3b0f3127b7..101d51c487 100644
--- a/string/test-memcpy.c
+++ b/string/test-memcpy.c
@@ -22,6 +22,7 @@ 
 # define MIN_PAGE_SIZE 131072
 # define TEST_MAIN
 # define TEST_NAME "memcpy"
+# define TIMEOUT (8 * 60)
 # include "test-string.h"
 
 char *simple_memcpy (char *, const char *, size_t);
diff --git a/string/test-string.h b/string/test-string.h
index 8ee00a04b1..9a6b76daa4 100644
--- a/string/test-string.h
+++ b/string/test-string.h
@@ -68,7 +68,9 @@  extern impl_t __start_impls[], __stop_impls[];
 
 
 # define TEST_FUNCTION test_main
-# define TIMEOUT (4 * 60)
+# ifndef TIMEOUT
+#  define TIMEOUT (4 * 60)
+# endif
 # define OPT_ITERATIONS 10000
 # define OPT_RANDOM 10001
 # define OPT_SEED 10002