This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v1.1] Randomize memcpy benchmark addresses.
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Will Newton <will dot newton at linaro dot org>
- Cc: libc-alpha <libc-alpha at sourceware dot org>
- Date: Thu, 5 Sep 2013 13:32:41 +0200
- Subject: Re: [PATCH v1.1] Randomize memcpy benchmark addresses.
- Authentication-results: sourceware.org; auth=none
- References: <20130904163151 dot GB10358 at domone dot kolej dot mff dot cuni dot cz> <20130904165025 dot GA15899 at domone dot kolej dot mff dot cuni dot cz> <CANu=DmjT9Do_1rbVJUGmR=WqGZmwX+KWNvp1QDOQrij3L3q3Xg at mail dot gmail dot com>
On Thu, Sep 05, 2013 at 08:41:02AM +0100, Will Newton wrote:
> On 4 September 2013 17:50, OndÅej BÃlka <neleai@seznam.cz> wrote:
> > static void
> > -do_one_test (impl_t *impl, char *dst, const char *src,
> > +do_one_test (impl_t *impl, size_t align1, size_t align2,
> > size_t len)
> > {
> > size_t i, iters = INNER_LOOP_ITERS;
> > timing_t start, stop, cur;
> > -
> > - if (CALL (impl, dst, src, len) != MEMCPY_RESULT (dst, len))
> > + static unsigned int r_seed = 42;
>
> Isn't randomizing with a static seed value replacing one predictable
> sequence with another? And it replaces a sequence that is simple to
> reason about with one that is not.
>
No, predictability depends on adversary. Unless branch predictor is
sophisticated enough that in can perfectly predict branches when they follow pattern
generated by rand_r function then randomization is enough.
Also new sequence is much harder than previous one from adversaries from
ACME corp. Now he could write following and show improvement in
benchmark.
char *evil_memcpy (char *x,char *y, size_t n)
{
if (((uintptr_t)x)%4096 < 32 || ((uintptr_t)y)%4096 < 32)
return optimized_memcpy (x, y, n); // Needs to improve current state.
while (n--)
x[n]=y[n];
return x;
}
> This introduces build warnings:
>
> bench-memcpy.c: In function âdo_one_testâ:
> bench-memcpy.c:69:15: warning: pointer targets in assignment differ in
> signedness [-Wpointer-sign]
> bench-memcpy.c:72:15: warning: pointer targets in assignment differ in
> signedness [-Wpointer-sign]
>
yes, I will add extra cast.
> > -
> > - printf ("Length %4zd, alignment %2zd/%2zd:", len, align1, align2);
> > + printf ("Length %4zd, aligned %c/%c:", len, align1 ? 'y' : 'n', align2 ? 'y' : 'n');
>
> This means we no longer print what the buffer alignment is which makes
> results analysis impossible.
>
Could you elaborate.
> > test_init ();
> > + buf1 = malloc (2 * SIZE + 4096);
> > + buf2 = malloc (2 * SIZE + 4096);
> > + buf1 = buf1 + 4096 - ((uintptr_t) buf1) % 4096;
> > + buf2 = buf2 + 4096 - ((uintptr_t) buf1) % 4096;
> > +
>
> What is 4096? We should avoid magic constants.
>
> buf1 and buf2 have already been allocated in bench-string.h. We should
> make any solution work with that header and work for all string
> benchmarks, not just one at a time.
>
Well buf2 was allocated as
buf2 = mmap (0, 2 * page_size ...
which is too small for this benchmark, so it needs to get changed.