This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] benchtests/Makefile: Run the string benchmarks four times by default.


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]