This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Consolidate *chr benchtests
- From: Will Newton <will dot newton at linaro dot org>
- To: Ondřej Bílka <neleai at seznam dot cz>
- Cc: libc-alpha <libc-alpha at sourceware dot org>
- Date: Fri, 6 Sep 2013 11:32:06 +0100
- Subject: Re: [PATCH] Consolidate *chr benchtests
- Authentication-results: sourceware.org; auth=none
- References: <20130906083221 dot GB21564 at domone dot kolej dot mff dot cuni dot cz>
On 6 September 2013 09:32, OndÅej BÃlka <neleai@seznam.cz> wrote:
> Hi,
>
> this continue consolidation with *chr benchmarks.
>
> It uses a common bench-chr.h header that i derived from bench-copy.h
> it could be factored more in future.
>
> An additional patch to select representative input combinations would
> be needed.
The description does not mention that things such as test output
change and that the randomisation changes have been added. Again I
feel like it would be much easier to review the changes if the
functional and non-functional changes in the patch were separated.
> 9 files changed, 179 insertions(+), 785 deletions(-)
That is a lot of lines to go through to figure out whether a change is
functional or not.
> OK to commit?
>
>
> * benchtests/bench-chr.h: New implementation.
> * benchtests/bench-memchr.c: Include benchtests/bench-chr.h.
> * benchtests/bench-memrchr.c: Likewise.
> * benchtests/bench-rawmemchr.c: Likewise.
> * benchtests/bench-strchr.c: Likewise.
> * benchtests/bench-strchrnul.c: Likewise.
> * benchtests/bench-strlen.c: Likewise.
> * benchtests/bench-strnlen.c: Likewise.
> * benchtests/bench-strrchr.c: Likewise.
> ---
> benchtests/bench-chr.h | 105 ++++++++++++++++++++++++
> benchtests/bench-memchr.c | 113 ++------------------------
> benchtests/bench-memrchr.c | 28 +++++++
> benchtests/bench-rawmemchr.c | 110 ++-----------------------
> benchtests/bench-strchr.c | 184 ++----------------------------------------
> benchtests/bench-strchrnul.c | 14 +++-
> benchtests/bench-strlen.c | 126 ++---------------------------
> benchtests/bench-strnlen.c | 116 ++------------------------
> benchtests/bench-strrchr.c | 168 ++------------------------------------
> 9 files changed, 179 insertions(+), 785 deletions(-)
> create mode 100644 benchtests/bench-chr.h
> create mode 100644 benchtests/bench-memrchr.c
>
> diff --git a/benchtests/bench-chr.h b/benchtests/bench-chr.h
> new file mode 100644
> index 0000000..7ec6e12
> --- /dev/null
> +++ b/benchtests/bench-chr.h
> @@ -0,0 +1,105 @@
> +/* Measure memcpy, strcpy, stpcpy, ... functions.
> + Copyright (C) 2013 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#define MEMCPY_RESULT(dst, len) dst
> +#define MIN_PAGE_SIZE (1<<22)
MIN_PAGE_SIZE is not a very descriptive name to me. It does not seem
to be related to machine page size as far as I can tell.
> +#define TEST_MAIN
> +#include "bench-string.h"
> +
> +PROTO
> +IMPLS
> +
> +static void
> +do_one_test (impl_t *impl, size_t aligned, size_t len)
> +{
> + size_t i, iters = INNER_LOOP_ITERS;
> + timing_t start, stop, cur;
> + static unsigned int r_seed = 42;
> +
> + char *srcs[INNER_LOOP_ITERS];
> + for (i = 0; i < iters; ++i)
> + {
> + srcs[i] = ((char *) buf1) + rand_r (&r_seed)%(MIN_PAGE_SIZE/2);
> + if (aligned) /* Align pointers. */
> + {
> + srcs[i] = srcs[i] - ((uintptr_t)srcs[i])%64;
64 is a magic number, it should be a #define and commented.
> + }
> + }
> +
> + TIMING_NOW (start);
> + for (i = 0; i < iters; ++i)
> + {
> + srcs[i][len] = 0;
> + CALL2 (impl, srcs[i], len);
> + srcs[i][len] = 1;
> + }
> + TIMING_NOW (stop);
> +
> + TIMING_DIFF (cur, start, stop);
> +
> + TIMING_PRINT_MEAN ((double) cur, (double) iters);
> +}
> +
> +static void
> +do_test (size_t aligned, size_t len)
> +{
> + printf ("Length %4zd, aligned %c:", len, aligned ? 'y' : 'n' );
It is not clear to me what "aligned" "y/n" means, as alignment could
mean anything from 4-byte aligned to page aligned.
The precise value of the alignment is not quite as useful in the
single buffer funciton benchmarks (e.g. strchr, strlen) as it is in
e.g. memcpy but I still think we should try and keep as much
information available as possible.
--
Will Newton
Toolchain Working Group, Linaro