Re: [PATCH] Benchtests: Improve large memcpy/memset benchmarks

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Thu, Mar 23, 2023 at 6:01 PM Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>
> Hi Noah,
>
>
> >> -  align &= 63;
> >
> > Can you replace with `align &= getpagesize () - 1`?
>
> Yes that would make more sense indeed.
>
> >> -  if ((align + len) * sizeof (CHAR) > page_size)
> >> -    return;
> >
> > Is there a reason to remove the out of bounds check?
>
> Well it can't ever go out of bounds. The allocated buffer size is
> 2 * page_size (ie. 4 times real page size or 2 times MIN_PAGE_SIZE
> due to the incorrect logic in init_sizes).
>

I disagree, we have a bounds check in nearly all other benchmarks
and I think its 1) a convenience so you don't need to add the equivalent
code in many other places and 2) a portability thing as page size varies
by system but people often write benchmark bounds with constant size
bounds.

> And if it could go out of bounds it should be an assert so we don't
> silently benchmark a bounds check!

Then I think the approach would be to output the json parameters and
report invalid in some way for the times.

In other places benchmarks rely on this bounds check and it seems more
error prone than anything else to just leave it unchecked. I would much
rather the benchmark not run or fail in some way than report a potentially
incorrect time as true.

>
> >>    align2 &= 4095;
> >
> > Might as well fix these up, can you replace 4095 with `getpagesize () - 1`?
>
> Sure.
>
> >> +  size_t i, iters = (MIN_PAGE_SIZE * 64) / n;
> >
> > this takes [.5, 10] seconds?
>
> Yes, it's only 1 GB written per test. It takes ~8.5 seconds on an old slow
> Cortex-A72.
>
> Cheers,
> Wilco
>
>