This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] benchtests/Makefile: Run the string benchmarks four times by default.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Will Newton <will dot newton at linaro dot org>
- Cc: libc-alpha at sourceware dot org, patches at linaro dot org
- Date: Thu, 05 Sep 2013 19:11:22 -0400
- Subject: Re: [PATCH] benchtests/Makefile: Run the string benchmarks four times by default.
- Authentication-results: sourceware.org; auth=none
- References: <52274838 dot 7010902 at linaro dot org>
On 09/04/2013 10:48 AM, Will Newton wrote:
>
> The string benchmarks can be affected by physical page placement, so
> running them multiple times is required to account for this. Also
> backup the results of the previous run like is done for the other
> benchmarks.
Siddhesh already commented on what I was going to say which was
that you have no guarantee that what you want is going to actually
happen. The OS might give you back exactly the same physical pages
on the subsequent run (though unlikely).
I agree on many of Ondrej's points. Driving engineering rigour into
these benchmarks is important.
> ChangeLog:
>
> 2013-09-03 Will Newton <will.newton@linaro.org>
>
> * benchtests/Makefile (BENCH_RUNS): New variable.
> (bench-set): Run tests BENCH_RUNS times and backup old run
> results.
> ---
> benchtests/Makefile | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/benchtests/Makefile b/benchtests/Makefile
> index 6037e5c..d72c98f 100644
> --- a/benchtests/Makefile
> +++ b/benchtests/Makefile
> @@ -118,6 +118,11 @@ ifndef BENCH_DURATION
> BENCH_DURATION := 10
> endif
>
> +# The default number of runs of a benchmark: 4.
This needs a lengthy comment on the purpose of BENCH_RUNS.
Why do we have it? What do we use it for? Why is 4 a good default?
> +ifndef BENCH_RUNS
> +BENCH_RUNS := 4
> +endif
> +
> CPPFLAGS-nonlib += -DDURATION=$(BENCH_DURATION)
>
> # Use clock_gettime to measure performance of functions. The default is to use
> @@ -146,8 +151,15 @@ bench: bench-set bench-func
>
> bench-set: $(binaries-benchset)
> for run in $^; do \
> - echo "Running $${run}"; \
> - $(run-bench) > $${run}.out; \
> + for old in $${run}.*.out; do \
> + if [ -f $$old ]; then \
> + mv $$old $${old}.old; \
> + fi; \
> + done; \
> + for count in $$(seq 1 $(BENCH_RUNS)); do \
> + echo "Running $${run} ($${count})"; \
> + $(run-bench) > $${run}.$${count}.out; \
> + done; \
> done
>
> bench-func: $(binaries-bench)
>
Given that extra runs have no adverse effects I'm OK with this patch in principle.
Cheers,
Carlos.