Re: [PATCH 1/6] perf, tools, stat: Abstract stat metrics printing

2015-12-21 Thread Jiri Olsa
On Tue, Dec 22, 2015 at 02:16:30AM +0100, Andi Kleen wrote:
> > > - fprintf(out, "   ");
> > > + print_metric(ctxp, NULL, NULL, "insn per cycle", 0);
> > >   }
> > >   total = 
> > > avg_stats(_stalled_cycles_front_stats[ctx][cpu]);
> > >   total = max(total, 
> > > avg_stats(_stalled_cycles_back_stats[ctx][cpu]));
> > >  
> > > + out->new_line(ctxp);
> > >   if (total && avg) {
> > >   ratio = total / avg;
> > > - fprintf(out, "\n");
> > 
> > you haven't address my first comment in here 
> > http://marc.info/?l=linux-kernel=144662610723134=2
> 
> The new_line is always needed because stalled cycles is always printed.
> 
> The reason it is always printed is that metric-only needs to see all the
> metrics for its column headers. That's why there are else cases
> everywhere.

please notice extra line below - between instructions and cycles lines

[jolsa@krava perf]$ ./perf stat -e instructions,cycles  kill
kill: not enough arguments

 Performance counter stats for 'kill':

   769,784  instructions  #0.78  insn per cycle 



   990,795  cycles  



jirka
--
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/


Re: [PATCH 1/6] perf, tools, stat: Abstract stat metrics printing

2015-12-21 Thread Andi Kleen
> > -   fprintf(out, "   ");
> > +   print_metric(ctxp, NULL, NULL, "insn per cycle", 0);
> > }
> > total = 
> > avg_stats(_stalled_cycles_front_stats[ctx][cpu]);
> > total = max(total, 
> > avg_stats(_stalled_cycles_back_stats[ctx][cpu]));
> >  
> > +   out->new_line(ctxp);
> > if (total && avg) {
> > ratio = total / avg;
> > -   fprintf(out, "\n");
> 
> you haven't address my first comment in here 
> http://marc.info/?l=linux-kernel=144662610723134=2

The new_line is always needed because stalled cycles is always printed.

The reason it is always printed is that metric-only needs to see all the
metrics for its column headers. That's why there are else cases
everywhere.


-Andi
-- 
a...@linux.intel.com -- Speaking for myself only.
--
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/


Re: [PATCH 1/6] perf, tools, stat: Abstract stat metrics printing

2015-12-21 Thread Jiri Olsa
On Mon, Dec 14, 2015 at 06:04:14PM -0800, Andi Kleen wrote:

SNIP

> -double avg, int cpu, enum aggr_mode aggr)
> +void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
> +double avg, int cpu,
> +struct perf_stat_output_ctx *out)
>  {
> + void *ctxp = out->ctx;
> + print_metric_t print_metric = out->print_metric;
>   double total, ratio = 0.0, total2;
>   int ctx = evsel_context(evsel);
>  
> @@ -307,119 +302,145 @@ void perf_stat__print_shadow_stats(FILE *out, struct 
> perf_evsel *evsel,
>   total = avg_stats(_cycles_stats[ctx][cpu]);
>   if (total) {
>   ratio = avg / total;
> - fprintf(out, " #   %5.2f  insns per cycle", 
> ratio);
> + print_metric(ctxp, NULL, "%7.2f ",
> + "insn per cycle", ratio);
>   } else {
> - fprintf(out, "   ");
> + print_metric(ctxp, NULL, NULL, "insn per cycle", 0);
>   }
>   total = 
> avg_stats(_stalled_cycles_front_stats[ctx][cpu]);
>   total = max(total, 
> avg_stats(_stalled_cycles_back_stats[ctx][cpu]));
>  
> + out->new_line(ctxp);
>   if (total && avg) {
>   ratio = total / avg;
> - fprintf(out, "\n");

you haven't address my first comment in here 
http://marc.info/?l=linux-kernel=144662610723134=2

also please rebase to latest acme/perf/core
FYI there's been some stat changes, but your branch cleanly rebased for me

thanks,
jirka
--
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/


Re: [PATCH 1/6] perf, tools, stat: Abstract stat metrics printing

2015-12-21 Thread Andi Kleen
> > -   fprintf(out, "   ");
> > +   print_metric(ctxp, NULL, NULL, "insn per cycle", 0);
> > }
> > total = 
> > avg_stats(_stalled_cycles_front_stats[ctx][cpu]);
> > total = max(total, 
> > avg_stats(_stalled_cycles_back_stats[ctx][cpu]));
> >  
> > +   out->new_line(ctxp);
> > if (total && avg) {
> > ratio = total / avg;
> > -   fprintf(out, "\n");
> 
> you haven't address my first comment in here 
> http://marc.info/?l=linux-kernel=144662610723134=2

The new_line is always needed because stalled cycles is always printed.

The reason it is always printed is that metric-only needs to see all the
metrics for its column headers. That's why there are else cases
everywhere.


-Andi
-- 
a...@linux.intel.com -- Speaking for myself only.
--
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/


Re: [PATCH 1/6] perf, tools, stat: Abstract stat metrics printing

2015-12-21 Thread Jiri Olsa
On Tue, Dec 22, 2015 at 02:16:30AM +0100, Andi Kleen wrote:
> > > - fprintf(out, "   ");
> > > + print_metric(ctxp, NULL, NULL, "insn per cycle", 0);
> > >   }
> > >   total = 
> > > avg_stats(_stalled_cycles_front_stats[ctx][cpu]);
> > >   total = max(total, 
> > > avg_stats(_stalled_cycles_back_stats[ctx][cpu]));
> > >  
> > > + out->new_line(ctxp);
> > >   if (total && avg) {
> > >   ratio = total / avg;
> > > - fprintf(out, "\n");
> > 
> > you haven't address my first comment in here 
> > http://marc.info/?l=linux-kernel=144662610723134=2
> 
> The new_line is always needed because stalled cycles is always printed.
> 
> The reason it is always printed is that metric-only needs to see all the
> metrics for its column headers. That's why there are else cases
> everywhere.

please notice extra line below - between instructions and cycles lines

[jolsa@krava perf]$ ./perf stat -e instructions,cycles  kill
kill: not enough arguments

 Performance counter stats for 'kill':

   769,784  instructions  #0.78  insn per cycle 



   990,795  cycles  



jirka
--
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/


Re: [PATCH 1/6] perf, tools, stat: Abstract stat metrics printing

2015-12-21 Thread Jiri Olsa
On Mon, Dec 14, 2015 at 06:04:14PM -0800, Andi Kleen wrote:

SNIP

> -double avg, int cpu, enum aggr_mode aggr)
> +void perf_stat__print_shadow_stats(struct perf_evsel *evsel,
> +double avg, int cpu,
> +struct perf_stat_output_ctx *out)
>  {
> + void *ctxp = out->ctx;
> + print_metric_t print_metric = out->print_metric;
>   double total, ratio = 0.0, total2;
>   int ctx = evsel_context(evsel);
>  
> @@ -307,119 +302,145 @@ void perf_stat__print_shadow_stats(FILE *out, struct 
> perf_evsel *evsel,
>   total = avg_stats(_cycles_stats[ctx][cpu]);
>   if (total) {
>   ratio = avg / total;
> - fprintf(out, " #   %5.2f  insns per cycle", 
> ratio);
> + print_metric(ctxp, NULL, "%7.2f ",
> + "insn per cycle", ratio);
>   } else {
> - fprintf(out, "   ");
> + print_metric(ctxp, NULL, NULL, "insn per cycle", 0);
>   }
>   total = 
> avg_stats(_stalled_cycles_front_stats[ctx][cpu]);
>   total = max(total, 
> avg_stats(_stalled_cycles_back_stats[ctx][cpu]));
>  
> + out->new_line(ctxp);
>   if (total && avg) {
>   ratio = total / avg;
> - fprintf(out, "\n");

you haven't address my first comment in here 
http://marc.info/?l=linux-kernel=144662610723134=2

also please rebase to latest acme/perf/core
FYI there's been some stat changes, but your branch cleanly rebased for me

thanks,
jirka
--
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/