RE: [PATCH v2 2/5] benchtests: Add memset zero fill benchtest

[naohirot--- via Libc-alpha <libc-alpha@sourceware.org>]

Hi Noah,

Thank you for the review.

> > +#define TEST_MAIN
> > +#define TEST_NAME "memset"
> > +#define START_SIZE (16 * 1024)
> > +#define MIN_PAGE_SIZE (getpagesize () + 64 * 1024 * 1024)
> > +#define TIMEOUT (20 * 60)
> > +#include "bench-string.h"
> > +
> > +#include "json-lib.h"
> > +
> > +void *generic_memset (void *, int, size_t);
> > +typedef void *(*proto_t) (void *, int, size_t);
> > +
> > +IMPL (MEMSET, 1)
> > +IMPL (generic_memset, 0)
> > +
> > +static void
> Do we want __attribute__((noinline, noclone))? 

Yes, I'll add it.

> > +do_one_test (json_ctx_t *json_ctx, impl_t *impl, CHAR *s,
> > +            int c1 __attribute ((unused)), int c2 __attribute ((unused)),
> > +            size_t n)
> > +{
> > +  size_t i, iters = 16;
> 
> I think 16 is probably too few iterations for reliable benchmarking. 
> Maybe `INNER_LOOP_ITERS` which is 8192

I tried it. If it is changed to 8192, it hit the TIMEOUT (20 * 60) on a64fx.
Please check the code below.

> 
> > +  timing_t start, stop, cur;
> > +
> > +  TIMING_NOW (start);
> > +  for (i = 0; i < iters; i += 2)
> > +    {
> > +      CALL (impl, s, c1, n);
> I am a bit worried that the overhead from the first call with `c1` will distort the results.
> Is it possible to implement it with a nested loop where you fill `s` with `c1` for 
> `n * inner_loop_iterations` in the outer loop and in the inner loop fill `c2` on `s + n * i`? 
> In that case maybe 16 for inner loop iterations and 512 for outer loop iterations. 

It seems that we have to set smaller number if this implementation is not wrong.
Because it will take 99.4 minutes estimating from the case that "iters = 32"
took 23.3 seconds.
(8192/32*23.3/60=99.4)


#define START_SIZE (16 * 1024)
...
static void
__attribute__((noinline, noclone))
do_one_test (json_ctx_t *json_ctx, impl_t *impl, CHAR *s,
             int c1 __attribute ((unused)), int c2 __attribute ((unused)),
             size_t n)
{
  size_t i, j, iters = INNER_LOOP_ITERS; // 32;
  timing_t start, stop, cur, latency = 0;

  for (i = 0; i < 512; i++) // for (i = 0; i < 2; i++)
    {
      CALL (impl, s, c1, n * 16);
      TIMING_NOW (start);
      for (j = 0; j < 16; j++)
        CALL (impl, s + n * j, c2, n);
      TIMING_NOW (stop);
      TIMING_DIFF (cur, start, stop);
      TIMING_ACCUM (latency, cur);
    }

  json_element_double (json_ctx, (double) latency / (double) iters);
}

> > +      CALL (impl, s, c2, n);
> > +    }
> > +  TIMING_NOW (stop);
> > +
> > +  TIMING_DIFF (cur, start, stop);
> > +
> > +  json_element_double (json_ctx, (double) cur / (double) iters);
> > +}
> > +
> > +static void
> > +do_test (json_ctx_t *json_ctx, size_t align, int c1, int c2, size_t len)
> > +{
> > +  align &= 63;
> Can you make this `align &= getpagesize () - 1;`?  

I'll change it.

Thanks.
Naohiro