* [RESEND PATCH v2] perf stat: improve readability of shadow stats @ 2021-03-15 14:30 Changbin Du 2021-03-16 13:53 ` Jiri Olsa 2021-03-19 1:01 ` Namhyung Kim 0 siblings, 2 replies; 6+ messages in thread From: Changbin Du @ 2021-03-15 14:30 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Peter Zijlstra Cc: Ingo Molnar, Jiri Olsa, Namhyung Kim, linux-kernel, Changbin Du This adds function convert_unit_double() and selects appropriate unit for shadow stats between K/M/G. $ sudo ./perf stat -a -- sleep 1 Before: Unit 'M' is selected even the number is very small. Performance counter stats for 'system wide': 4,003.06 msec cpu-clock # 3.998 CPUs utilized 16,179 context-switches # 0.004 M/sec 161 cpu-migrations # 0.040 K/sec 4,699 page-faults # 0.001 M/sec 6,135,801,925 cycles # 1.533 GHz (83.21%) 5,783,308,491 stalled-cycles-frontend # 94.26% frontend cycles idle (83.21%) 4,543,694,050 stalled-cycles-backend # 74.05% backend cycles idle (66.49%) 4,720,130,587 instructions # 0.77 insn per cycle # 1.23 stalled cycles per insn (83.28%) 753,848,078 branches # 188.318 M/sec (83.61%) 37,457,747 branch-misses # 4.97% of all branches (83.48%) 1.001283725 seconds time elapsed After: $ sudo ./perf stat -a -- sleep 2 Performance counter stats for 'system wide': 8,005.52 msec cpu-clock # 3.999 CPUs utilized 10,715 context-switches # 1.338 K/sec 785 cpu-migrations # 98.057 /sec 102 page-faults # 12.741 /sec 1,948,202,279 cycles # 0.243 GHz 2,816,470,932 stalled-cycles-frontend # 144.57% frontend cycles idle 2,661,172,207 stalled-cycles-backend # 136.60% backend cycles idle 464,172,105 instructions # 0.24 insn per cycle # 6.07 stalled cycles per insn 91,567,662 branches # 11.438 M/sec 7,756,054 branch-misses # 8.47% of all branches 2.002040043 seconds time elapsed Signed-off-by: Changbin Du <changbin.du@gmail.com> v2: o do not change 'sec' to 'cpu-sec'. o use convert_unit_double to implement convert_unit. --- tools/perf/util/stat-shadow.c | 16 +++++++--------- tools/perf/util/units.c | 21 ++++++++++++++------- tools/perf/util/units.h | 1 + 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c index 6ccf21a72f06..3f800e71126f 100644 --- a/tools/perf/util/stat-shadow.c +++ b/tools/perf/util/stat-shadow.c @@ -9,6 +9,7 @@ #include "expr.h" #include "metricgroup.h" #include "cgroup.h" +#include "units.h" #include <linux/zalloc.h> /* @@ -1270,18 +1271,15 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config, generic_metric(config, evsel->metric_expr, evsel->metric_events, NULL, evsel->name, evsel->metric_name, NULL, 1, cpu, out, st); } else if (runtime_stat_n(st, STAT_NSECS, cpu, &rsd) != 0) { - char unit = 'M'; - char unit_buf[10]; + char unit = ' '; + char unit_buf[10] = "/sec"; total = runtime_stat_avg(st, STAT_NSECS, cpu, &rsd); - if (total) - ratio = 1000.0 * avg / total; - if (ratio < 0.001) { - ratio *= 1000; - unit = 'K'; - } - snprintf(unit_buf, sizeof(unit_buf), "%c/sec", unit); + ratio = convert_unit_double(1000000000.0 * avg / total, &unit); + + if (unit != ' ') + snprintf(unit_buf, sizeof(unit_buf), "%c/sec", unit); print_metric(config, ctxp, NULL, "%8.3f", unit_buf, ratio); } else if (perf_stat_evsel__is(evsel, SMI_NUM)) { print_smi_cost(config, cpu, out, st, &rsd); diff --git a/tools/perf/util/units.c b/tools/perf/util/units.c index a46762aec4c9..32c39cfe209b 100644 --- a/tools/perf/util/units.c +++ b/tools/perf/util/units.c @@ -33,28 +33,35 @@ unsigned long parse_tag_value(const char *str, struct parse_tag *tags) return (unsigned long) -1; } -unsigned long convert_unit(unsigned long value, char *unit) +double convert_unit_double(double value, char *unit) { *unit = ' '; - if (value > 1000) { - value /= 1000; + if (value > 1000.0) { + value /= 1000.0; *unit = 'K'; } - if (value > 1000) { - value /= 1000; + if (value > 1000.0) { + value /= 1000.0; *unit = 'M'; } - if (value > 1000) { - value /= 1000; + if (value > 1000.0) { + value /= 1000.0; *unit = 'G'; } return value; } +unsigned long convert_unit(unsigned long value, char *unit) +{ + double v = convert_unit_double((double)value, unit); + + return (unsigned long)v; +} + int unit_number__scnprintf(char *buf, size_t size, u64 n) { char unit[4] = "BKMG"; diff --git a/tools/perf/util/units.h b/tools/perf/util/units.h index 99263b6a23f7..ea43e74e3240 100644 --- a/tools/perf/util/units.h +++ b/tools/perf/util/units.h @@ -12,6 +12,7 @@ struct parse_tag { unsigned long parse_tag_value(const char *str, struct parse_tag *tags); +double convert_unit_double(double value, char *unit); unsigned long convert_unit(unsigned long value, char *unit); int unit_number__scnprintf(char *buf, size_t size, u64 n); -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH v2] perf stat: improve readability of shadow stats 2021-03-15 14:30 [RESEND PATCH v2] perf stat: improve readability of shadow stats Changbin Du @ 2021-03-16 13:53 ` Jiri Olsa 2021-03-18 15:12 ` Changbin Du 2021-03-19 1:01 ` Namhyung Kim 1 sibling, 1 reply; 6+ messages in thread From: Jiri Olsa @ 2021-03-16 13:53 UTC (permalink / raw) To: Changbin Du Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar, Namhyung Kim, linux-kernel On Mon, Mar 15, 2021 at 10:30:47PM +0800, Changbin Du wrote: SNIP > diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c > index 6ccf21a72f06..3f800e71126f 100644 > --- a/tools/perf/util/stat-shadow.c > +++ b/tools/perf/util/stat-shadow.c > @@ -9,6 +9,7 @@ > #include "expr.h" > #include "metricgroup.h" > #include "cgroup.h" > +#include "units.h" > #include <linux/zalloc.h> > > /* > @@ -1270,18 +1271,15 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config, > generic_metric(config, evsel->metric_expr, evsel->metric_events, NULL, > evsel->name, evsel->metric_name, NULL, 1, cpu, out, st); > } else if (runtime_stat_n(st, STAT_NSECS, cpu, &rsd) != 0) { > - char unit = 'M'; > - char unit_buf[10]; > + char unit = ' '; > + char unit_buf[10] = "/sec"; > > total = runtime_stat_avg(st, STAT_NSECS, cpu, &rsd); > - > if (total) > - ratio = 1000.0 * avg / total; > - if (ratio < 0.001) { > - ratio *= 1000; > - unit = 'K'; > - } > - snprintf(unit_buf, sizeof(unit_buf), "%c/sec", unit); > + ratio = convert_unit_double(1000000000.0 * avg / total, &unit); > + > + if (unit != ' ') > + snprintf(unit_buf, sizeof(unit_buf), "%c/sec", unit); > print_metric(config, ctxp, NULL, "%8.3f", unit_buf, ratio); hum, does this still change the metric unit in the csv output? 'perf -a -x,' jirka > } else if (perf_stat_evsel__is(evsel, SMI_NUM)) { > print_smi_cost(config, cpu, out, st, &rsd); > diff --git a/tools/perf/util/units.c b/tools/perf/util/units.c > index a46762aec4c9..32c39cfe209b 100644 > --- a/tools/perf/util/units.c > +++ b/tools/perf/util/units.c SNIP ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH v2] perf stat: improve readability of shadow stats 2021-03-16 13:53 ` Jiri Olsa @ 2021-03-18 15:12 ` Changbin Du 2021-03-19 10:00 ` Jiri Olsa 0 siblings, 1 reply; 6+ messages in thread From: Changbin Du @ 2021-03-18 15:12 UTC (permalink / raw) To: Jiri Olsa Cc: Changbin Du, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar, Namhyung Kim, linux-kernel On Tue, Mar 16, 2021 at 02:53:41PM +0100, Jiri Olsa wrote: > On Mon, Mar 15, 2021 at 10:30:47PM +0800, Changbin Du wrote: > > SNIP > > > diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c > > index 6ccf21a72f06..3f800e71126f 100644 > > --- a/tools/perf/util/stat-shadow.c > > +++ b/tools/perf/util/stat-shadow.c > > @@ -9,6 +9,7 @@ > > #include "expr.h" > > #include "metricgroup.h" > > #include "cgroup.h" > > +#include "units.h" > > #include <linux/zalloc.h> > > > > /* > > @@ -1270,18 +1271,15 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config, > > generic_metric(config, evsel->metric_expr, evsel->metric_events, NULL, > > evsel->name, evsel->metric_name, NULL, 1, cpu, out, st); > > } else if (runtime_stat_n(st, STAT_NSECS, cpu, &rsd) != 0) { > > - char unit = 'M'; > > - char unit_buf[10]; > > + char unit = ' '; > > + char unit_buf[10] = "/sec"; > > > > total = runtime_stat_avg(st, STAT_NSECS, cpu, &rsd); > > - > > if (total) > > - ratio = 1000.0 * avg / total; > > - if (ratio < 0.001) { > > - ratio *= 1000; > > - unit = 'K'; > > - } > > - snprintf(unit_buf, sizeof(unit_buf), "%c/sec", unit); > > + ratio = convert_unit_double(1000000000.0 * avg / total, &unit); > > + > > + if (unit != ' ') > > + snprintf(unit_buf, sizeof(unit_buf), "%c/sec", unit); > > print_metric(config, ctxp, NULL, "%8.3f", unit_buf, ratio); > > hum, does this still change the metric unit in the csv output? 'perf -a -x,' > The unit is changed in csv format, too. See below. before: $ sudo ./perf stat -a -x, -- sleep 1 8037.85,msec,cpu-clock,8037851596,100.00,7.999,CPUs utilized 714,,context-switches,8037838466,100.00,0.089,K/sec 11,,cpu-migrations,8037832590,100.00,0.001,K/sec 71,,page-faults,8037824974,100.00,0.009,K/sec 84033551,,cycles,8037750471,100.00,0.010,GHz 22563553,,instructions,8037733879,100.00,0.27,insn per cycle 4685736,,branches,8037708301,100.00,0.583,M/sec 356327,,branch-misses,8037667950,100.00,7.60,of all branches after: $ sudo ./perf stat -a -x, -- sleep 1 8026.19,msec,cpu-clock,8026194365,100.00,7.983,CPUs utilized 621,,context-switches,8026178186,100.00,77.372,/sec 16,,cpu-migrations,8026172135,100.00,1.993,/sec 73,,page-faults,8026142626,100.00,9.095,/sec 92645028,,cycles,8026066285,100.00,0.012,GHz 56268285,,instructions,8026048894,100.00,0.61,insn per cycle 10979859,,branches,8026022127,100.00,1.368,M/sec 441719,,branch-misses,8025981169,100.00,4.02,of all branches But is this a real problem? > jirka > > > } else if (perf_stat_evsel__is(evsel, SMI_NUM)) { > > print_smi_cost(config, cpu, out, st, &rsd); > > diff --git a/tools/perf/util/units.c b/tools/perf/util/units.c > > index a46762aec4c9..32c39cfe209b 100644 > > --- a/tools/perf/util/units.c > > +++ b/tools/perf/util/units.c > > SNIP > -- Cheers, Changbin Du ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH v2] perf stat: improve readability of shadow stats 2021-03-18 15:12 ` Changbin Du @ 2021-03-19 10:00 ` Jiri Olsa 2021-03-22 3:02 ` Andi Kleen 0 siblings, 1 reply; 6+ messages in thread From: Jiri Olsa @ 2021-03-19 10:00 UTC (permalink / raw) To: Changbin Du Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar, Namhyung Kim, linux-kernel, Andi Kleen On Thu, Mar 18, 2021 at 11:12:40PM +0800, Changbin Du wrote: > On Tue, Mar 16, 2021 at 02:53:41PM +0100, Jiri Olsa wrote: > > On Mon, Mar 15, 2021 at 10:30:47PM +0800, Changbin Du wrote: > > > > SNIP > > > > > diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c > > > index 6ccf21a72f06..3f800e71126f 100644 > > > --- a/tools/perf/util/stat-shadow.c > > > +++ b/tools/perf/util/stat-shadow.c > > > @@ -9,6 +9,7 @@ > > > #include "expr.h" > > > #include "metricgroup.h" > > > #include "cgroup.h" > > > +#include "units.h" > > > #include <linux/zalloc.h> > > > > > > /* > > > @@ -1270,18 +1271,15 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config, > > > generic_metric(config, evsel->metric_expr, evsel->metric_events, NULL, > > > evsel->name, evsel->metric_name, NULL, 1, cpu, out, st); > > > } else if (runtime_stat_n(st, STAT_NSECS, cpu, &rsd) != 0) { > > > - char unit = 'M'; > > > - char unit_buf[10]; > > > + char unit = ' '; > > > + char unit_buf[10] = "/sec"; > > > > > > total = runtime_stat_avg(st, STAT_NSECS, cpu, &rsd); > > > - > > > if (total) > > > - ratio = 1000.0 * avg / total; > > > - if (ratio < 0.001) { > > > - ratio *= 1000; > > > - unit = 'K'; > > > - } > > > - snprintf(unit_buf, sizeof(unit_buf), "%c/sec", unit); > > > + ratio = convert_unit_double(1000000000.0 * avg / total, &unit); > > > + > > > + if (unit != ' ') > > > + snprintf(unit_buf, sizeof(unit_buf), "%c/sec", unit); > > > print_metric(config, ctxp, NULL, "%8.3f", unit_buf, ratio); > > > > hum, does this still change the metric unit in the csv output? 'perf -a -x,' > > > The unit is changed in csv format, too. See below. > > before: > $ sudo ./perf stat -a -x, -- sleep 1 > 8037.85,msec,cpu-clock,8037851596,100.00,7.999,CPUs utilized > 714,,context-switches,8037838466,100.00,0.089,K/sec > 11,,cpu-migrations,8037832590,100.00,0.001,K/sec > 71,,page-faults,8037824974,100.00,0.009,K/sec > 84033551,,cycles,8037750471,100.00,0.010,GHz > 22563553,,instructions,8037733879,100.00,0.27,insn per cycle > 4685736,,branches,8037708301,100.00,0.583,M/sec > 356327,,branch-misses,8037667950,100.00,7.60,of all branches > > after: > $ sudo ./perf stat -a -x, -- sleep 1 > 8026.19,msec,cpu-clock,8026194365,100.00,7.983,CPUs utilized > 621,,context-switches,8026178186,100.00,77.372,/sec > 16,,cpu-migrations,8026172135,100.00,1.993,/sec > 73,,page-faults,8026142626,100.00,9.095,/sec > 92645028,,cycles,8026066285,100.00,0.012,GHz > 56268285,,instructions,8026048894,100.00,0.61,insn per cycle > 10979859,,branches,8026022127,100.00,1.368,M/sec > 441719,,branch-misses,8025981169,100.00,4.02,of all branches > > But is this a real problem? perhaps not, Andi, any idea about this? thanks, jirka ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH v2] perf stat: improve readability of shadow stats 2021-03-19 10:00 ` Jiri Olsa @ 2021-03-22 3:02 ` Andi Kleen 0 siblings, 0 replies; 6+ messages in thread From: Andi Kleen @ 2021-03-22 3:02 UTC (permalink / raw) To: Jiri Olsa Cc: Changbin Du, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar, Namhyung Kim, linux-kernel > > But is this a real problem? > > perhaps not, Andi, any idea about this? It's not a problem for my tools which don't use the unit, but I could imagine one for other parsers. I would recommend to not change it for CSV, which is expected to be parsed by tools. -Andi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH v2] perf stat: improve readability of shadow stats 2021-03-15 14:30 [RESEND PATCH v2] perf stat: improve readability of shadow stats Changbin Du 2021-03-16 13:53 ` Jiri Olsa @ 2021-03-19 1:01 ` Namhyung Kim 1 sibling, 0 replies; 6+ messages in thread From: Namhyung Kim @ 2021-03-19 1:01 UTC (permalink / raw) To: Changbin Du Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar, Jiri Olsa, linux-kernel Hello, On Mon, Mar 15, 2021 at 11:31 PM Changbin Du <changbin.du@gmail.com> wrote: > > This adds function convert_unit_double() and selects appropriate > unit for shadow stats between K/M/G. > > $ sudo ./perf stat -a -- sleep 1 > > Before: Unit 'M' is selected even the number is very small. > Performance counter stats for 'system wide': > > 4,003.06 msec cpu-clock # 3.998 CPUs utilized > 16,179 context-switches # 0.004 M/sec > 161 cpu-migrations # 0.040 K/sec > 4,699 page-faults # 0.001 M/sec > 6,135,801,925 cycles # 1.533 GHz (83.21%) > 5,783,308,491 stalled-cycles-frontend # 94.26% frontend cycles idle (83.21%) > 4,543,694,050 stalled-cycles-backend # 74.05% backend cycles idle (66.49%) > 4,720,130,587 instructions # 0.77 insn per cycle > # 1.23 stalled cycles per insn (83.28%) > 753,848,078 branches # 188.318 M/sec (83.61%) > 37,457,747 branch-misses # 4.97% of all branches (83.48%) > > 1.001283725 seconds time elapsed > > After: > $ sudo ./perf stat -a -- sleep 2 > > Performance counter stats for 'system wide': > > 8,005.52 msec cpu-clock # 3.999 CPUs utilized > 10,715 context-switches # 1.338 K/sec > 785 cpu-migrations # 98.057 /sec > 102 page-faults # 12.741 /sec > 1,948,202,279 cycles # 0.243 GHz > 2,816,470,932 stalled-cycles-frontend # 144.57% frontend cycles idle > 2,661,172,207 stalled-cycles-backend # 136.60% backend cycles idle > 464,172,105 instructions # 0.24 insn per cycle > # 6.07 stalled cycles per insn > 91,567,662 branches # 11.438 M/sec > 7,756,054 branch-misses # 8.47% of all branches > > 2.002040043 seconds time elapsed > > Signed-off-by: Changbin Du <changbin.du@gmail.com> Acked-by: Namhyung Kim <namhyung@kernel.org> Thanks, Namhyung > > v2: > o do not change 'sec' to 'cpu-sec'. > o use convert_unit_double to implement convert_unit. > --- > tools/perf/util/stat-shadow.c | 16 +++++++--------- > tools/perf/util/units.c | 21 ++++++++++++++------- > tools/perf/util/units.h | 1 + > 3 files changed, 22 insertions(+), 16 deletions(-) > > diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c > index 6ccf21a72f06..3f800e71126f 100644 > --- a/tools/perf/util/stat-shadow.c > +++ b/tools/perf/util/stat-shadow.c > @@ -9,6 +9,7 @@ > #include "expr.h" > #include "metricgroup.h" > #include "cgroup.h" > +#include "units.h" > #include <linux/zalloc.h> > > /* > @@ -1270,18 +1271,15 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config, > generic_metric(config, evsel->metric_expr, evsel->metric_events, NULL, > evsel->name, evsel->metric_name, NULL, 1, cpu, out, st); > } else if (runtime_stat_n(st, STAT_NSECS, cpu, &rsd) != 0) { > - char unit = 'M'; > - char unit_buf[10]; > + char unit = ' '; > + char unit_buf[10] = "/sec"; > > total = runtime_stat_avg(st, STAT_NSECS, cpu, &rsd); > - > if (total) > - ratio = 1000.0 * avg / total; > - if (ratio < 0.001) { > - ratio *= 1000; > - unit = 'K'; > - } > - snprintf(unit_buf, sizeof(unit_buf), "%c/sec", unit); > + ratio = convert_unit_double(1000000000.0 * avg / total, &unit); > + > + if (unit != ' ') > + snprintf(unit_buf, sizeof(unit_buf), "%c/sec", unit); > print_metric(config, ctxp, NULL, "%8.3f", unit_buf, ratio); > } else if (perf_stat_evsel__is(evsel, SMI_NUM)) { > print_smi_cost(config, cpu, out, st, &rsd); > diff --git a/tools/perf/util/units.c b/tools/perf/util/units.c > index a46762aec4c9..32c39cfe209b 100644 > --- a/tools/perf/util/units.c > +++ b/tools/perf/util/units.c > @@ -33,28 +33,35 @@ unsigned long parse_tag_value(const char *str, struct parse_tag *tags) > return (unsigned long) -1; > } > > -unsigned long convert_unit(unsigned long value, char *unit) > +double convert_unit_double(double value, char *unit) > { > *unit = ' '; > > - if (value > 1000) { > - value /= 1000; > + if (value > 1000.0) { > + value /= 1000.0; > *unit = 'K'; > } > > - if (value > 1000) { > - value /= 1000; > + if (value > 1000.0) { > + value /= 1000.0; > *unit = 'M'; > } > > - if (value > 1000) { > - value /= 1000; > + if (value > 1000.0) { > + value /= 1000.0; > *unit = 'G'; > } > > return value; > } > > +unsigned long convert_unit(unsigned long value, char *unit) > +{ > + double v = convert_unit_double((double)value, unit); > + > + return (unsigned long)v; > +} > + > int unit_number__scnprintf(char *buf, size_t size, u64 n) > { > char unit[4] = "BKMG"; > diff --git a/tools/perf/util/units.h b/tools/perf/util/units.h > index 99263b6a23f7..ea43e74e3240 100644 > --- a/tools/perf/util/units.h > +++ b/tools/perf/util/units.h > @@ -12,6 +12,7 @@ struct parse_tag { > > unsigned long parse_tag_value(const char *str, struct parse_tag *tags); > > +double convert_unit_double(double value, char *unit); > unsigned long convert_unit(unsigned long value, char *unit); > int unit_number__scnprintf(char *buf, size_t size, u64 n); > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-22 3:03 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-15 14:30 [RESEND PATCH v2] perf stat: improve readability of shadow stats Changbin Du 2021-03-16 13:53 ` Jiri Olsa 2021-03-18 15:12 ` Changbin Du 2021-03-19 10:00 ` Jiri Olsa 2021-03-22 3:02 ` Andi Kleen 2021-03-19 1:01 ` Namhyung Kim
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.