This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2] Preheat CPU in benchtests
- From: Andrew Pinski <pinskia at gmail dot com>
- To: OndÅej BÃlka <neleai at seznam dot cz>
- Cc: "Carlos O'Donell" <carlos at redhat dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Fri, 3 May 2013 10:45:36 -0700
- Subject: Re: [PATCH v2] Preheat CPU in benchtests
- References: <20130423061028 dot GA6257 at domone dot kolej dot mff dot cuni dot cz> <20130501192347 dot GA19748 at domone dot kolej dot mff dot cuni dot cz> <5183140C dot 3030805 at redhat dot com> <20130503114526 dot GA19176 at domone dot kolej dot mff dot cuni dot cz>
On Fri, May 3, 2013 at 4:45 AM, OndÅej BÃlka <neleai@seznam.cz> wrote:
> On Thu, May 02, 2013 at 09:34:04PM -0400, Carlos O'Donell wrote:
>> On 05/01/2013 03:23 PM, OndÅej BÃlka wrote:
>> >
>> > Ping,
>> >
>>
>> Needs a comment explaining why we don't disable requency
>> scaling e.g. needs root (Andi Kleen's comment).
>>
>> Needs a comment explaining future work around CPU_CLK_UNHALTED.
> This was independent idea, It belongs to hp_timing and so.
>>
>> Needs a comment explaining why we run for a fixed number
>> of cycles instead of a fixed amount of time (Petr Baudis' comment).
>>
> Done.
>> > + /* This loop should cause CPU switch to maximal freqency. This makes
>> > + subsequent measurement more accurate. We need side effect to avoid loop
>> > + being deleted by compiler. */
>> > + for(k = 0; k < 1000000; k++)
>> > + dontoptimize += 23.0 * dontoptimize + 2.1;
>> > +
>> > +
>>
>> Make it a function.
>>
>> Can we avoid using the fpu here and avoid possible
>> double overflow? Does a volatile unsigned int work just
>> as well? If we need to heat the CPU and FPU
> Done.
>
> * benchtests/bench-skeleton.c (main): Preheat CPU.
>
> ---
> benchtests/bench-skeleton.c | 16 ++++++++++++++++
> 1 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/benchtests/bench-skeleton.c b/benchtests/bench-skeleton.c
> index 7359184..a0e7bac 100644
> --- a/benchtests/bench-skeleton.c
> +++ b/benchtests/bench-skeleton.c
> @@ -22,6 +22,20 @@
> #include <time.h>
> #include <inttypes.h>
>
> +volatile unsigned int = 0;
I think this should be:
volatile unsigned int dontoptimize = 0;
Unless I am missing something obvious. Other than that it looks good
to me though I cannot approve it.
Thanks,
Andrew Pinski
> +void startup ()
> +{
> + /* This loop should cause CPU switch to maximal freqency.
> + This makes subsequent measurement more accurate. We need side effect to
> + avoid loop being deleted by compiler.
> + This should be enougth to cause switch and it is simpler than run loop
> + for constant time. This is used when user has not root to set
> + constant freqency. */
> + int k;
> + for (k = 0; k < 10000000; k++)
> + dontoptimize += 23 * dontoptimize + 2;
> +}
> +
> #define TIMESPEC_AFTER(a, b) \
> (((a).tv_sec == (b).tv_sec) ? \
> ((a).tv_nsec > (b).tv_nsec) : \
> @@ -32,6 +46,8 @@ main (int argc, char **argv)
> unsigned long i, k;
> struct timespec start, end, runtime;
>
> + startup();
> +
> memset (&runtime, 0, sizeof (runtime));
> memset (&start, 0, sizeof (start));
> memset (&end, 0, sizeof (end));
> --
> 1.7.4.4
>
On Fri, May 3, 2013 at 4:45 AM, OndÅej BÃlka <neleai@seznam.cz> wrote:
> On Thu, May 02, 2013 at 09:34:04PM -0400, Carlos O'Donell wrote:
>> On 05/01/2013 03:23 PM, OndÅej BÃlka wrote:
>> >
>> > Ping,
>> >
>>
>> Needs a comment explaining why we don't disable requency
>> scaling e.g. needs root (Andi Kleen's comment).
>>
>> Needs a comment explaining future work around CPU_CLK_UNHALTED.
> This was independent idea, It belongs to hp_timing and so.
>>
>> Needs a comment explaining why we run for a fixed number
>> of cycles instead of a fixed amount of time (Petr Baudis' comment).
>>
> Done.
>> > + /* This loop should cause CPU switch to maximal freqency. This makes
>> > + subsequent measurement more accurate. We need side effect to avoid loop
>> > + being deleted by compiler. */
>> > + for(k = 0; k < 1000000; k++)
>> > + dontoptimize += 23.0 * dontoptimize + 2.1;
>> > +
>> > +
>>
>> Make it a function.
>>
>> Can we avoid using the fpu here and avoid possible
>> double overflow? Does a volatile unsigned int work just
>> as well? If we need to heat the CPU and FPU
> Done.
>
> * benchtests/bench-skeleton.c (main): Preheat CPU.
>
> ---
> benchtests/bench-skeleton.c | 16 ++++++++++++++++
> 1 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/benchtests/bench-skeleton.c b/benchtests/bench-skeleton.c
> index 7359184..a0e7bac 100644
> --- a/benchtests/bench-skeleton.c
> +++ b/benchtests/bench-skeleton.c
> @@ -22,6 +22,20 @@
> #include <time.h>
> #include <inttypes.h>
>
> +volatile unsigned int = 0;
> +void startup ()
> +{
> + /* This loop should cause CPU switch to maximal freqency.
> + This makes subsequent measurement more accurate. We need side effect to
> + avoid loop being deleted by compiler.
> + This should be enougth to cause switch and it is simpler than run loop
> + for constant time. This is used when user has not root to set
> + constant freqency. */
> + int k;
> + for (k = 0; k < 10000000; k++)
> + dontoptimize += 23 * dontoptimize + 2;
> +}
> +
> #define TIMESPEC_AFTER(a, b) \
> (((a).tv_sec == (b).tv_sec) ? \
> ((a).tv_nsec > (b).tv_nsec) : \
> @@ -32,6 +46,8 @@ main (int argc, char **argv)
> unsigned long i, k;
> struct timespec start, end, runtime;
>
> + startup();
> +
> memset (&runtime, 0, sizeof (runtime));
> memset (&start, 0, sizeof (start));
> memset (&end, 0, sizeof (end));
> --
> 1.7.4.4
>