2021-03-19 07:07:45

by Jin Yao

[permalink] [raw]
Subject: [PATCH v3 2/2] perf test: Add CVS summary test

The patch "perf stat: Align CSV output for summary mode" aligned
CVS output and added "summary" to the first column of summary
lines.

Now we check if the "summary" string is added to the CVS output.

If we set '--no-cvs-summary' option, the "summary" string would
not be added, also check with this case.

Signed-off-by: Jin Yao <[email protected]>
---
v3:
- New in v3.

tools/perf/tests/shell/stat+cvs_summary.sh | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100755 tools/perf/tests/shell/stat+cvs_summary.sh

diff --git a/tools/perf/tests/shell/stat+cvs_summary.sh b/tools/perf/tests/shell/stat+cvs_summary.sh
new file mode 100755
index 000000000000..dd14f2ce7f6b
--- /dev/null
+++ b/tools/perf/tests/shell/stat+cvs_summary.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+# perf stat cvs summary test
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+#
+# 1.001364330 9224197 cycles 8012885033 100.00
+# summary 9224197 cycles 8012885033 100.00
+#
+perf stat -e cycles -x' ' -I1000 --interval-count 1 --summary 2>&1 | \
+grep -e summary | \
+while read summary num event run pct
+do
+ if [ $summary != "summary" ]; then
+ exit 1
+ fi
+done
+
+#
+# 1.001360298 9148534 cycles 8012853854 100.00
+#9148534 cycles 8012853854 100.00
+#
+perf stat -e cycles -x' ' -I1000 --interval-count 1 --summary --no-cvs-summary 2>&1 | \
+grep -e summary | \
+while read num event run pct
+do
+ exit 1
+done
+
+exit 0
--
2.17.1


2021-03-24 13:07:13

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] perf test: Add CVS summary test

Em Fri, Mar 19, 2021 at 03:01:56PM +0800, Jin Yao escreveu:
> The patch "perf stat: Align CSV output for summary mode" aligned
> CVS output and added "summary" to the first column of summary
> lines.
>
> Now we check if the "summary" string is added to the CVS output.
>
> If we set '--no-cvs-summary' option, the "summary" string would
> not be added, also check with this case.

You mixed up cvs with csv in various places, I'm fixing it up...

- Arnaldo

> Signed-off-by: Jin Yao <[email protected]>
> ---
> v3:
> - New in v3.
>
> tools/perf/tests/shell/stat+cvs_summary.sh | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100755 tools/perf/tests/shell/stat+cvs_summary.sh
>
> diff --git a/tools/perf/tests/shell/stat+cvs_summary.sh b/tools/perf/tests/shell/stat+cvs_summary.sh
> new file mode 100755
> index 000000000000..dd14f2ce7f6b
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat+cvs_summary.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +# perf stat cvs summary test
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +#
> +# 1.001364330 9224197 cycles 8012885033 100.00
> +# summary 9224197 cycles 8012885033 100.00
> +#
> +perf stat -e cycles -x' ' -I1000 --interval-count 1 --summary 2>&1 | \
> +grep -e summary | \
> +while read summary num event run pct
> +do
> + if [ $summary != "summary" ]; then
> + exit 1
> + fi
> +done
> +
> +#
> +# 1.001360298 9148534 cycles 8012853854 100.00
> +#9148534 cycles 8012853854 100.00
> +#
> +perf stat -e cycles -x' ' -I1000 --interval-count 1 --summary --no-cvs-summary 2>&1 | \
> +grep -e summary | \
> +while read num event run pct
> +do
> + exit 1
> +done
> +
> +exit 0
> --
> 2.17.1
>

--

- Arnaldo

2021-03-25 03:04:59

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] perf test: Add CVS summary test

Em Wed, Mar 24, 2021 at 10:05:18AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Mar 19, 2021 at 03:01:56PM +0800, Jin Yao escreveu:
> > The patch "perf stat: Align CSV output for summary mode" aligned
> > CVS output and added "summary" to the first column of summary
> > lines.
> >
> > Now we check if the "summary" string is added to the CVS output.
> >
> > If we set '--no-cvs-summary' option, the "summary" string would
> > not be added, also check with this case.
>
> You mixed up cvs with csv in various places, I'm fixing it up...

This, for the first patch, now fixing the second.


diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index e81a45cadd4a0bdb..6ec5960b08c3de21 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -482,14 +482,14 @@ convenient for post processing.
--summary::
Print summary for interval mode (-I).

---no-cvs-summary::
+--no-csv-summary::
Don't print 'summary' at the first column for CVS summary output.
This option must be used with -x and --summary.

This option can be enabled in perf config by setting the variable
-'stat.no-cvs-summary'.
+'stat.no-csv-summary'.

-$ perf config stat.no-cvs-summary=true
+$ perf config stat.no-csv-summary=true

EXAMPLES
--------
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 6daa090129a65c78..2a2c15cac80a3bee 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1093,9 +1093,9 @@ void perf_stat__set_big_num(int set)
stat_config.big_num = (set != 0);
}

-void perf_stat__set_no_cvs_summary(int set)
+void perf_stat__set_no_csv_summary(int set)
{
- stat_config.no_cvs_summary = (set != 0);
+ stat_config.no_csv_summary = (set != 0);
}

static int stat__set_big_num(const struct option *opt __maybe_unused,
@@ -1254,8 +1254,8 @@ static struct option stat_options[] = {
"threads of same physical core"),
OPT_BOOLEAN(0, "summary", &stat_config.summary,
"print summary for interval mode"),
- OPT_BOOLEAN(0, "no-cvs-summary", &stat_config.no_cvs_summary,
- "don't print 'summary' for CVS summary output"),
+ OPT_BOOLEAN(0, "no-csv-summary", &stat_config.no_csv_summary,
+ "don't print 'summary' for CSV summary output"),
OPT_BOOLEAN(0, "quiet", &stat_config.quiet,
"don't print output (useful with record)"),
#ifdef HAVE_LIBPFM
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index df78f11f6fb50a0b..6bcb5ef221f8c1be 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -457,8 +457,8 @@ static int perf_stat_config(const char *var, const char *value)
if (!strcmp(var, "stat.big-num"))
perf_stat__set_big_num(perf_config_bool(var, value));

- if (!strcmp(var, "stat.no-cvs-summary"))
- perf_stat__set_no_cvs_summary(perf_config_bool(var, value));
+ if (!strcmp(var, "stat.no-csv-summary"))
+ perf_stat__set_no_csv_summary(perf_config_bool(var, value));

/* Add other config variables here. */
return 0;

2021-03-25 03:05:06

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] perf test: Add CVS summary test

Em Wed, Mar 24, 2021 at 10:12:43AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Mar 24, 2021 at 10:05:18AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Fri, Mar 19, 2021 at 03:01:56PM +0800, Jin Yao escreveu:
> > > The patch "perf stat: Align CSV output for summary mode" aligned
> > > CVS output and added "summary" to the first column of summary
> > > lines.
> > >
> > > Now we check if the "summary" string is added to the CVS output.
> > >
> > > If we set '--no-cvs-summary' option, the "summary" string would
> > > not be added, also check with this case.
> >
> > You mixed up cvs with csv in various places, I'm fixing it up...
>
> This, for the first patch, now fixing the second.

nah, there were some missing fixes:


diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index e81a45cadd4a0bdb..6ec5960b08c3de21 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -482,14 +482,14 @@ convenient for post processing.
--summary::
Print summary for interval mode (-I).

---no-cvs-summary::
+--no-csv-summary::
Don't print 'summary' at the first column for CVS summary output.
This option must be used with -x and --summary.

This option can be enabled in perf config by setting the variable
-'stat.no-cvs-summary'.
+'stat.no-csv-summary'.

-$ perf config stat.no-cvs-summary=true
+$ perf config stat.no-csv-summary=true

EXAMPLES
--------
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 6daa090129a65c78..2a2c15cac80a3bee 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1093,9 +1093,9 @@ void perf_stat__set_big_num(int set)
stat_config.big_num = (set != 0);
}

-void perf_stat__set_no_cvs_summary(int set)
+void perf_stat__set_no_csv_summary(int set)
{
- stat_config.no_cvs_summary = (set != 0);
+ stat_config.no_csv_summary = (set != 0);
}

static int stat__set_big_num(const struct option *opt __maybe_unused,
@@ -1254,8 +1254,8 @@ static struct option stat_options[] = {
"threads of same physical core"),
OPT_BOOLEAN(0, "summary", &stat_config.summary,
"print summary for interval mode"),
- OPT_BOOLEAN(0, "no-cvs-summary", &stat_config.no_cvs_summary,
- "don't print 'summary' for CVS summary output"),
+ OPT_BOOLEAN(0, "no-csv-summary", &stat_config.no_csv_summary,
+ "don't print 'summary' for CSV summary output"),
OPT_BOOLEAN(0, "quiet", &stat_config.quiet,
"don't print output (useful with record)"),
#ifdef HAVE_LIBPFM
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index df78f11f6fb50a0b..6bcb5ef221f8c1be 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -457,8 +457,8 @@ static int perf_stat_config(const char *var, const char *value)
if (!strcmp(var, "stat.big-num"))
perf_stat__set_big_num(perf_config_bool(var, value));

- if (!strcmp(var, "stat.no-cvs-summary"))
- perf_stat__set_no_cvs_summary(perf_config_bool(var, value));
+ if (!strcmp(var, "stat.no-csv-summary"))
+ perf_stat__set_no_csv_summary(perf_config_bool(var, value));

/* Add other config variables here. */
return 0;
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 2e7fec0bd8f3f3bb..d3137bc1706548d4 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -440,7 +440,7 @@ static void printout(struct perf_stat_config *config, struct aggr_cpu_id id, int
os.nfields++;
}

- if (!config->no_cvs_summary && config->csv_output &&
+ if (!config->no_csv_summary && config->csv_output &&
config->summary && !config->interval) {
fprintf(config->output, "%16s%s", "summary", config->csv_sep);
}
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index def0cdc841330210..48e6a06233faef8e 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -128,7 +128,7 @@ struct perf_stat_config {
bool all_user;
bool percore_show_thread;
bool summary;
- bool no_cvs_summary;
+ bool no_csv_summary;
bool metric_no_group;
bool metric_no_merge;
bool stop_read_counter;
@@ -161,7 +161,7 @@ struct perf_stat_config {
};

void perf_stat__set_big_num(int set);
-void perf_stat__set_no_cvs_summary(int set);
+void perf_stat__set_no_csv_summary(int set);

void update_stats(struct stats *stats, u64 val);
double avg_stats(struct stats *stats);