Hi Andi,

On Mon, 14 Apr 2014 09:09:31 -0700, Andi Kleen wrote:
> From: Andi Kleen <a...@linux.intel.com>
>
> perf stat -rX prints the stddev for multiple measurements.
> Just looking at the stddev for judging the quality of the data
> is a bit dangerous The simplest sanity check is to just look
> at a simple plot. This patchs add a sparkline to the end
> of the measurements to make it simple to judge the data.
>
> The sparkline only uses UTF-8, so should be readable
> in all modern tools and terminals.
>
> The sparkline is between the minimum and maximum of the data,
> so it's mainly a indicator of variance. To keep the code
> simple and make the output not too wide only the first
> 8 values are printed. If more values are there it adds '..'
>
> The code is inspired by Zach Holman's spark shell script.
>
> Example output (view in non-proportial font):
>
>  Performance counter stats for 'true' (10 runs):
>
>           0.175672      task-clock (msec)         #    0.555 CPUs utilized    
>         ( +-  1.77% ) █▄▁▁▁▁▁▁..
>                  0      context-switches          #    0.000 K/sec
>                  0      cpu-migrations            #    0.000 K/sec
>                114      page-faults               #    0.647 M/sec            
>         ( +-  0.14% ) ▁█▁▁████..
>            520,798      cycles                    #    2.965 GHz              
>         ( +-  1.75% ) █▄▁▁▁▁▁▁..
>            433,525      instructions              #    0.83  insns per cycle  
>         ( +-  0.28% ) ▅▇▅▄▇█▁▆..
>             83,012      branches                  #  472.537 M/sec            
>         ( +-  0.31% ) ▅▇▆▄▇█▁▆..
>              3,157      branch-misses             #    3.80% of all branches  
>         ( +-  2.55% ) ▇█▃▅▁▃▁▂..
>
>        0.000316660 seconds time elapsed                                       
>    ( +-  1.78% ) █▅▁▁▁▁▁▁..
>
> As you can see even in the most simple run there are quite interesting
> patterns. The time sparkline suggests it would be also useful to have an 
> option
> to throw the first measurement away.
>
> Known issues:
> - Makes the perf stat output wider. Could be adjust by shrinking
> some white space. Not done so far.
> - No output for -A/--per-socket/--per-core with -rX. This code
> is missing the basic noise detection code. Once it's added there
> sparklines could be shown too.
>
> v2: Avoid printing spark lines for normal CSV case (Jiri)
> Signed-off-by: Andi Kleen <a...@linux.intel.com>
> ---


[SNIP]
> diff --git a/tools/perf/util/spark.c b/tools/perf/util/spark.c
> new file mode 100644
> index 0000000..a677d2c
> --- /dev/null
> +++ b/tools/perf/util/spark.c
> @@ -0,0 +1,45 @@
> +#include <stdio.h>
> +#include <limits.h>
> +#include "spark.h"
> +
> +#define NUM_SPARKS 8
> +#define SPARK_SHIFT 8
> +
> +/* Print spark lines on outf for numval values in val. */
> +void print_spark(FILE *outf, unsigned long *val, int numval)
> +{
> +     static const char *ticks[NUM_SPARKS] = {
> +             "▁",  "▂", "▃", "▄", "▅", "▆", "▇", "█"
> +     };
> +     int i;
> +     unsigned long min = LONG_MAX, max = 0, f;

s/LONG_MAX/ULONG_MAX/ ?

> +
> +     for (i = 0; i < numval; i++) {
> +             if (val[i] < min)
> +                     min = val[i];
> +             if (val[i] > max)
> +                     max = val[i];
> +     }
> +     f = ((max - min) << SPARK_SHIFT) / (NUM_SPARKS - 1);
> +     if (f < 1)
> +             f = 1;
> +     for (i = 0; i < numval; i++) {
> +             fputs(ticks[((val[i] - min) << SPARK_SHIFT) / f], outf);
> +     }
> +}
> +
> +#ifdef TEST
> +#include <stdlib.h>
> +
> +int main(int ac, char **av)
> +{
> +     unsigned long *val = calloc(ac - 1, sizeof(unsigned long));
> +     int i;
> +
> +     for (i = 1; i < ac; i++)
> +             val[i - 1] = strtoul(av[i], NULL, 0);
> +     print_spark(stdout, val, ac - 1);
> +     putchar('\n');
> +     return 0;
> +}
> +#endif

Hmm.. test codes usually live in the tools/perf/tests directory.


> diff --git a/tools/perf/util/spark.h b/tools/perf/util/spark.h
> new file mode 100644
> index 0000000..f2d5ac5
> --- /dev/null
> +++ b/tools/perf/util/spark.h
> @@ -0,0 +1,3 @@
> +#pragma once
> +void print_spark(FILE *outf, unsigned long *val, int numval);
> +
> diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> index 6506b3d..2b26d74 100644
> --- a/tools/perf/util/stat.c
> +++ b/tools/perf/util/stat.c
> @@ -1,10 +1,16 @@
>  #include <math.h>
> +#include <stdio.h>
>  
>  #include "stat.h"
> +#include "spark.h"
>  
>  void update_stats(struct stats *stats, u64 val)
>  {
>       double delta;
> +     int n = stats->n;
> +
> +     if (n < NUM_SPARK_VALS)
> +             stats->svals[n] = val;
>  
>       stats->n++;
>       delta = val - stats->mean;
> @@ -61,3 +67,30 @@ double rel_stddev_stats(double stddev, double avg)
>  
>       return pct;
>  }
> +
> +static int all_the_same(unsigned long *vals, int len)
> +{
> +     int i;
> +     unsigned long v0 = vals[0];
> +
> +     for (i = 1; i < len; i++)
> +             if (vals[i] != v0)
> +                     return 0;
> +     return 1;
> +}
> +
> +void print_stat_spark(FILE *f, struct stats *stat)
> +{
> +     int n = stat->n, len;
> +
> +     if (n <= 1)
> +             return;
> +     len = n;

It seems the 'n' is not needed at all - just use 'len'.


> +     if (len > NUM_SPARK_VALS)
> +             len = NUM_SPARK_VALS;
> +     if (all_the_same(stat->svals, len))
> +             return;

Why does it skip printing if all values are same?  I think you wanted to
skip the "all zero" (uncounted) case, right?

Also adding a few blank lines might improve readability a bit IMHO.

Thanks,
Namhyung


> +     print_spark(f, stat->svals, len);
> +     if (stat->n > NUM_SPARK_VALS)
> +             fputs("..", f);
> +}
> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> index ae8ccd7..1b4dc71 100644
> --- a/tools/perf/util/stat.h
> +++ b/tools/perf/util/stat.h
> @@ -1,12 +1,16 @@
>  #ifndef __PERF_STATS_H
>  #define __PERF_STATS_H
>  
> +#include <stdio.h>
>  #include "types.h"
>  
> +#define NUM_SPARK_VALS 8 /* support spark line on first N items */
> +
>  struct stats
>  {
>       double n, mean, M2;
>       u64 max, min;
> +     unsigned long svals[NUM_SPARK_VALS];
>  };
>  
>  void update_stats(struct stats *stats, u64 val);
> @@ -14,12 +18,18 @@ double avg_stats(struct stats *stats);
>  double stddev_stats(struct stats *stats);
>  double rel_stddev_stats(double stddev, double avg);
>  
> +void print_stat_spark(FILE *f, struct stats *stat);
> +
>  static inline void init_stats(struct stats *stats)
>  {
> +     int i;
> +
>       stats->n    = 0.0;
>       stats->mean = 0.0;
>       stats->M2   = 0.0;
>       stats->min  = (u64) -1;
>       stats->max  = 0;
> +     for (i = 0; i < NUM_SPARK_VALS; i++)
> +             stats->svals[i] = 0;
>  }
>  #endif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to