Re: [PATCH 2/2] dont commify big numbers by default, let -B do it

[Jim Cromie <jim.cromie@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3334 bytes --]

On Sat, May 21, 2011 at 4:33 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Jim Cromie <jim.cromie@gmail.com> wrote:
>
>> Currently, big numbers get commas by default, which complicates parsing
>> by automation. [...]
>
> Well, automation can turn it off via --no-big-num, right?

ah.  my man-page is a bit behind
(but is in usage, and in Doc/perf-stat.txt)

> Also, i think for automation we'd also like to have a 'simple output' mode,
> would you like to add that?

OK, heres 1st cut at it, adding option --simple, for review / feedback

Its based upon the csv-output code mostly, with some vestiges of verbose..

I altered csv-output to have event-name 1st
this seems what folks would expect normally,
I surmised that existing was a short-cut..
Then added "noise"

part of reason for sending early is this column swap,
it might annoy existing users.

[jimc@groucho perf]$ ./perf stat -r3 -x' ' true
task-clock-msecs1.009533  0.142%
context-switches 0 -nan%
CPU-migrations 0 100.000%
page-faults 85 0.000%
cycles 789434 0.107%
instructions 488768 0.546%
branches 102796 0.505%
branch-misses 8472 1.688%
cache-references 0 -nan%  (scaled from 0.00%)
cache-misses 0 -nan%  (scaled from 0.00%)

[jimc@groucho perf]$ ./perf stat -r3 -x' ------- ' true
task-clock-msecs1.036450 -------  ------- 3.519%
context-switches ------- 0 ------- -nan%
CPU-migrations ------- 0 ------- -nan%
page-faults ------- 85 ------- 0.000%
cycles ------- 812151 ------- 3.463%
instructions ------- 499263 ------- 0.368%
branches ------- 104240 ------- 0.224%
branch-misses ------- 8125 ------- 4.627%
cache-references ------- 0 ------- -nan%  (scaled from 0.00%)
cache-misses ------- 0 ------- -nan%  (scaled from 0.00%)

I also need to do something better with -no-aggr format,
doesnt do right with extra column

[jimc@groucho perf]$ sudo ./perf stat -r3 -x' ------- ' -A -a true
CPU0 ------- task-clock-msecs2.099862 -------
CPU1 ------- task-clock-msecs2.084522 -------
CPU2 ------- task-clock-msecs2.072948 -------
CPU3 ------- task-clock-msecs2.071611 -------
CPU0 ------- context-switches ------- 2
CPU1 ------- context-switches ------- 0
CPU2 ------- context-switches ------- 3
CPU3 ------- context-switches ------- 6
CPU0 ------- CPU-migrations ------- 2
...




> Something a bit like what you can see in 'perf stat -v true':
> Without the human output later on, and with elapsed time added as well.

for my part, Id like the moral equivalent of time(s)? output too,
though I suspect thats a separate patch..

[jimc@groucho perf]$ time ./perf stat -x'  ' -- sh -c 'sleep 3'
task-clock-msecs6.830999
context-switches  2
CPU-migrations  1
page-faults  477
cycles  5404163  (scaled from 72.07%)
instructions  3302699
branches  804750
branch-misses  51870
cache-references  1584532  (scaled from 45.93%)
cache-misses  36616  (scaled from 31.36%)

real	0m3.019s
user	0m0.005s
sys	0m0.012s

are these timings already taken by perf-stat ?
is it a simple matter of addition and printing ?
If not, whats involved ?

Also, task-clock-msec doesnt quite match up with times' user number
Whats going on here ?


> Thanks,
>
>        Ingo
>

thank you
~jimc

PS: attaching now (in gmail web iface),
will try to thread up to this, RSN, with git send-email

[-- Attachment #2: 0001-add-simple-output-mode-to-perf-stat-based-upon-csv-o.patch --]
[-- Type: text/x-patch, Size: 5447 bytes --]

From 6cd1667ae522737d577d3f67314c6952bc3a5e4f Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Tue, 24 May 2011 15:56:39 -0600
Subject: [PATCH] add --simple output mode to perf-stat, based upon csv-output

produces output like this (WIP):

perf]$ ./perf stat -x'  ' -r3 -- sh -c 'sleep 3'
task-clock-msecs6.896594    1.105%
context-switches  3  0.000%
CPU-migrations  0  -nan%
page-faults  477  0.000%
cycles  5540483  1.002%  (scaled from 69.92%)
instructions  3881794  7.779%  (scaled from 83.70%)
branches  932435  3.343%  (scaled from 82.21%)
branch-misses  43830  11.999%
cache-references  1582931  3.718%  (scaled from 52.72%)
cache-misses  37199  10.154%  (scaled from 37.65%)

Its based upon csv-output, and prints event-name in 1st column, in
contrast to previous format.  This may be problematic for some
existing users, but is more "normal" and therefore better.

Whether '%' symbols or scalings should be printed is TBD.
Also, -A -a output needs work.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 tools/perf/builtin-stat.c |   44 ++++++++++++++++++++++++++++++++------------
 1 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 3a703e4..1711535 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -91,6 +91,7 @@ static int			big_num_opt			=  -1;
 static const char		*cpu_list;
 static const char		*csv_sep			= NULL;
 static bool			csv_output			= false;
+static bool			simple				= false;
 
 static int logfd = 2;	/* stderr by default, override with -l */
 static FILE *logfp;	/* = fdopen(logfd,"w") */
@@ -212,7 +213,7 @@ static int read_counter_aggr(struct perf_evsel *counter)
 	for (i = 0; i < 3; i++)
 		update_stats(&ps->res_stats[i], count[i]);
 
-	if (verbose) {
+	if (verbose || simple) {
 		fprintf(logfp, "%s: %" PRIu64 " %" PRIu64 " %" PRIu64 "\n",
 			event_name(counter), count[0], count[1], count[2]);
 	}
@@ -383,22 +384,31 @@ static void print_noise(struct perf_evsel *evsel, double avg)
 		return;
 
 	ps = evsel->priv;
-	fprintf(logfp, "   ( +- %7.3f%% )",
-			100 * stddev_stats(&ps->res_stats[0]) / avg);
+	if (!csv_output)
+		fprintf(logfp, "   ( +- %7.3f%% )",
+				100 * stddev_stats(&ps->res_stats[0]) / avg);
+	else
+	  fprintf(logfp, "%s%.3f%%",
+		  csv_sep, 100 * stddev_stats(&ps->res_stats[0]) / avg);
+
+
 }
 
 static void nsec_printout(int cpu, struct perf_evsel *evsel, double avg)
 {
 	double msecs = avg / 1e6;
 	char cpustr[16] = { '\0', };
-	const char *fmt = csv_output ? "%s%.6f%s%s" : "%s%18.6f%s%-24s";
+	const char *fmt = csv_output ? "%s%s%.6f%s" : "%s%18.6f%s%-24s";
 
 	if (no_aggr)
 		sprintf(cpustr, "CPU%*d%s",
 			csv_output ? 0 : -4,
 			evsel_list->cpus->map[cpu], csv_sep);
 
-	fprintf(logfp, fmt, cpustr, msecs, csv_sep, event_name(evsel));
+	if (!csv_output)
+	  fprintf(logfp, fmt, cpustr, msecs, csv_sep, event_name(evsel));
+	else
+	  fprintf(logfp, fmt, cpustr, event_name(evsel), csv_sep, msecs);
 
 	if (evsel->cgrp)
 		fprintf(logfp, "%s%s", csv_sep, evsel->cgrp->name);
@@ -418,7 +428,7 @@ static void abs_printout(int cpu, struct perf_evsel *evsel, double avg)
 	const char *fmt;
 
 	if (csv_output)
-		fmt = "%s%.0f%s%s";
+		fmt = "%s%s%s%.0f";
 	else if (big_num)
 		fmt = "%s%'18.0f%s%-24s";
 	else
@@ -431,7 +441,10 @@ static void abs_printout(int cpu, struct perf_evsel *evsel, double avg)
 	else
 		cpu = 0;
 
-	fprintf(logfp, fmt, cpustr, avg, csv_sep, event_name(evsel));
+	if (csv_output)
+	  fprintf(logfp, fmt, cpustr, event_name(evsel), csv_sep, avg);
+	else
+	  fprintf(logfp, fmt, cpustr, avg, csv_sep, event_name(evsel));
 
 	if (evsel->cgrp)
 		fprintf(logfp, "%s%s", csv_sep, evsel->cgrp->name);
@@ -475,7 +488,7 @@ static void print_counter_aggr(struct perf_evsel *counter)
 	double avg = avg_stats(&ps->res_stats[0]);
 	int scaled = counter->counts->scaled;
 
-	if (scaled == -1) {
+	if (scaled == -1 && !csv_output) {
 		fprintf(logfp, "%*s%s%*s",
 			csv_output ? 0 : 18,
 			"<not counted>",
@@ -495,7 +508,7 @@ static void print_counter_aggr(struct perf_evsel *counter)
 	else
 		abs_printout(-1, counter, avg);
 
-	if (csv_output) {
+	if (0 && csv_output) {
 		fputc('\n', logfp);
 		return;
 	}
@@ -536,9 +549,12 @@ static void print_counter(struct perf_evsel *counter)
 				csv_output ? 0 : -24,
 				event_name(counter));
 
-			if (counter->cgrp)
+			if (counter->cgrp) {
+			  if (csv_output)
+				fprintf(logfp, "%s%s", counter->cgrp->name, csv_sep);
+			  else
 				fprintf(logfp, "%s%s", csv_sep, counter->cgrp->name);
-
+			}
 			fputc('\n', logfp);
 			continue;
 		}
@@ -567,6 +583,8 @@ static void print_stat(int argc, const char **argv)
 
 	fflush(stdout);
 
+	if (simple) return;
+
 	if (!csv_output) {
 		fprintf(logfp, "\n");
 		fprintf(logfp, " Performance counter stats for ");
@@ -670,12 +688,14 @@ static const struct option options[] = {
 	OPT_BOOLEAN('A', "no-aggr", &no_aggr,
 		    "disable CPU count aggregation"),
 	OPT_STRING('x', "field-separator", &csv_sep, "separator",
-		   "print counts with custom separator"),
+		   "print counts with custom separator (csv-output)"),
 	OPT_CALLBACK('G', "cgroup", &evsel_list, "name",
 		     "monitor event in cgroup name only",
 		     parse_cgroups),
 	OPT_INTEGER('l', "logfd", &logfd,
 		    "log output to fd, instead of stderr"),
+	OPT_BOOLEAN('s', "simple", &simple,
+		    "simple output for scripts/automation"),
 	OPT_END()
 };
 
-- 
1.7.4.4