* [PATCH v2 2/3] perf util: Flexible to set block info output formats
2020-01-06 19:45 [PATCH v2 1/3] perf util: Move block_pair_cmp to block-info Jin Yao
@ 2020-01-06 19:45 ` Jin Yao
2020-01-07 10:06 ` Jiri Olsa
2020-01-06 19:45 ` [PATCH v2 3/3] perf util: Support color ops to print block percents in color Jin Yao
2020-01-07 9:57 ` [PATCH v2 1/3] perf util: Move block_pair_cmp to block-info Jiri Olsa
2 siblings, 1 reply; 7+ messages in thread
From: Jin Yao @ 2020-01-06 19:45 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
Currently we use a predefined array to set the
block info output formats, it's fixed and inflexible.
This patch adds two parameters "block_hpps" and "nr_hpps"
in block_info__create_report and other static functions,
in order to let user decide which columns to report and
with specified report ordering. It should be more flexible.
Buffers will be allocated to contain the new fmts, of course,
we need to release them before perf exits.
v2:
---
New patch created in v2.
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/builtin-report.c | 25 +++++++++++---
tools/perf/util/block-info.c | 64 ++++++++++++++++++++++++++----------
tools/perf/util/block-info.h | 12 +++++--
3 files changed, 76 insertions(+), 25 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index de988589d99b..81ae1f862d11 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -104,6 +104,7 @@ struct report {
bool symbol_ipc;
bool total_cycles_mode;
struct block_report *block_reports;
+ int nr_block_reports;
};
static int report__config(const char *var, const char *value, void *cb)
@@ -503,7 +504,7 @@ static int perf_evlist__tui_block_hists_browse(struct evlist *evlist,
ret = report__browse_block_hists(&rep->block_reports[i++].hist,
rep->min_percent, pos,
&rep->session->header.env,
- &rep->annotation_opts);
+ &rep->annotation_opts, true);
if (ret != 0)
return ret;
}
@@ -536,7 +537,7 @@ static int perf_evlist__tty_browse_hists(struct evlist *evlist,
if (rep->total_cycles_mode) {
report__browse_block_hists(&rep->block_reports[i++].hist,
rep->min_percent, pos,
- NULL, NULL);
+ NULL, NULL, true);
continue;
}
@@ -966,8 +967,19 @@ static int __cmd_report(struct report *rep)
report__output_resort(rep);
if (rep->total_cycles_mode) {
+ int block_hpps[6] = {
+ PERF_HPP_REPORT__BLOCK_TOTAL_CYCLES_PCT,
+ PERF_HPP_REPORT__BLOCK_LBR_CYCLES,
+ PERF_HPP_REPORT__BLOCK_CYCLES_PCT,
+ PERF_HPP_REPORT__BLOCK_AVG_CYCLES,
+ PERF_HPP_REPORT__BLOCK_RANGE,
+ PERF_HPP_REPORT__BLOCK_DSO,
+ };
+
rep->block_reports = block_info__create_report(session->evlist,
- rep->total_cycles);
+ rep->total_cycles,
+ block_hpps, 6,
+ &rep->nr_block_reports);
if (!rep->block_reports)
return -1;
}
@@ -1543,8 +1555,11 @@ int cmd_report(int argc, const char **argv)
zfree(&report.ptime_range);
}
- if (report.block_reports)
- zfree(&report.block_reports);
+ if (report.block_reports) {
+ block_info__free_report(report.block_reports,
+ report.nr_block_reports);
+ report.block_reports = NULL;
+ }
zstd_fini(&(session->zstd_data));
perf_session__delete(session);
diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c
index 18a445938681..0818db17b3f2 100644
--- a/tools/perf/util/block-info.c
+++ b/tools/perf/util/block-info.c
@@ -376,33 +376,42 @@ static void hpp_register(struct block_fmt *block_fmt, int idx,
}
static void register_block_columns(struct perf_hpp_list *hpp_list,
- struct block_fmt *block_fmts)
+ struct block_fmt *block_fmts,
+ int *block_hpps, int nr_hpps)
{
- for (int i = 0; i < PERF_HPP_REPORT__BLOCK_MAX_INDEX; i++)
- hpp_register(&block_fmts[i], i, hpp_list);
+ for (int i = 0; i < nr_hpps; i++)
+ hpp_register(&block_fmts[i], block_hpps[i], hpp_list);
}
-static void init_block_hist(struct block_hist *bh, struct block_fmt *block_fmts)
+static void init_block_hist(struct block_hist *bh, struct block_fmt *block_fmts,
+ int *block_hpps, int nr_hpps)
{
__hists__init(&bh->block_hists, &bh->block_list);
perf_hpp_list__init(&bh->block_list);
bh->block_list.nr_header_lines = 1;
- register_block_columns(&bh->block_list, block_fmts);
+ register_block_columns(&bh->block_list, block_fmts,
+ block_hpps, nr_hpps);
- perf_hpp_list__register_sort_field(&bh->block_list,
- &block_fmts[PERF_HPP_REPORT__BLOCK_TOTAL_CYCLES_PCT].fmt);
+ /* Sort by the first fmt */
+ perf_hpp_list__register_sort_field(&bh->block_list, &block_fmts[0].fmt);
}
-static void process_block_report(struct hists *hists,
- struct block_report *block_report,
- u64 total_cycles)
+static int process_block_report(struct hists *hists,
+ struct block_report *block_report,
+ u64 total_cycles, int *block_hpps,
+ int nr_hpps)
{
struct rb_node *next = rb_first_cached(&hists->entries);
struct block_hist *bh = &block_report->hist;
struct hist_entry *he;
- init_block_hist(bh, block_report->fmts);
+ block_report->fmts = calloc(nr_hpps, sizeof(struct block_fmt));
+ if (!block_report->fmts)
+ return -1;
+
+ block_report->nr_fmts = nr_hpps;
+ init_block_hist(bh, block_report->fmts, block_hpps, nr_hpps);
while (next) {
he = rb_entry(next, struct hist_entry, rb_node);
@@ -411,16 +420,19 @@ static void process_block_report(struct hists *hists,
next = rb_next(&he->rb_node);
}
- for (int i = 0; i < PERF_HPP_REPORT__BLOCK_MAX_INDEX; i++) {
+ for (int i = 0; i < nr_hpps; i++) {
block_report->fmts[i].total_cycles = total_cycles;
block_report->fmts[i].block_cycles = block_report->cycles;
}
hists__output_resort(&bh->block_hists, NULL);
+ return 0;
}
struct block_report *block_info__create_report(struct evlist *evlist,
- u64 total_cycles)
+ u64 total_cycles,
+ int *block_hpps, int nr_hpps,
+ int *nr_reps)
{
struct block_report *block_reports;
int nr_hists = evlist->core.nr_entries, i = 0;
@@ -433,16 +445,30 @@ struct block_report *block_info__create_report(struct evlist *evlist,
evlist__for_each_entry(evlist, pos) {
struct hists *hists = evsel__hists(pos);
- process_block_report(hists, &block_reports[i], total_cycles);
+ process_block_report(hists, &block_reports[i], total_cycles,
+ block_hpps, nr_hpps);
i++;
}
+ *nr_reps = nr_hists;
return block_reports;
}
+void block_info__free_report(struct block_report *reps, int nr_reps)
+{
+ for (int i = 0; i < nr_reps; i++) {
+ hists__delete_entries(&reps[i].hist.block_hists);
+ if (reps[i].fmts)
+ free(reps[i].fmts);
+ }
+
+ free(reps);
+}
+
int report__browse_block_hists(struct block_hist *bh, float min_percent,
struct evsel *evsel, struct perf_env *env,
- struct annotation_options *annotation_opts)
+ struct annotation_options *annotation_opts,
+ bool release)
{
int ret;
@@ -451,13 +477,17 @@ int report__browse_block_hists(struct block_hist *bh, float min_percent,
symbol_conf.report_individual_block = true;
hists__fprintf(&bh->block_hists, true, 0, 0, min_percent,
stdout, true);
- hists__delete_entries(&bh->block_hists);
+ if (release)
+ hists__delete_entries(&bh->block_hists);
+
return 0;
case 1:
symbol_conf.report_individual_block = true;
ret = block_hists_tui_browse(bh, evsel, min_percent,
env, annotation_opts);
- hists__delete_entries(&bh->block_hists);
+ if (release)
+ hists__delete_entries(&bh->block_hists);
+
return ret;
default:
return -1;
diff --git a/tools/perf/util/block-info.h b/tools/perf/util/block-info.h
index bfa22c59195d..0bf01e3a423d 100644
--- a/tools/perf/util/block-info.h
+++ b/tools/perf/util/block-info.h
@@ -44,7 +44,8 @@ enum {
struct block_report {
struct block_hist hist;
u64 cycles;
- struct block_fmt fmts[PERF_HPP_REPORT__BLOCK_MAX_INDEX];
+ struct block_fmt *fmts;
+ int nr_fmts;
};
struct block_hist;
@@ -68,11 +69,16 @@ int block_info__process_sym(struct hist_entry *he, struct block_hist *bh,
u64 *block_cycles_aggr, u64 total_cycles);
struct block_report *block_info__create_report(struct evlist *evlist,
- u64 total_cycles);
+ u64 total_cycles,
+ int *block_hpps, int nr_hpps,
+ int *nr_reps);
+
+void block_info__free_report(struct block_report *reps, int nr_reps);
int report__browse_block_hists(struct block_hist *bh, float min_percent,
struct evsel *evsel, struct perf_env *env,
- struct annotation_options *annotation_opts);
+ struct annotation_options *annotation_opts,
+ bool release);
float block_info__total_cycles_percent(struct hist_entry *he);
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v2 3/3] perf util: Support color ops to print block percents in color
2020-01-06 19:45 [PATCH v2 1/3] perf util: Move block_pair_cmp to block-info Jin Yao
2020-01-06 19:45 ` [PATCH v2 2/3] perf util: Flexible to set block info output formats Jin Yao
@ 2020-01-06 19:45 ` Jin Yao
2020-01-07 9:57 ` [PATCH v2 1/3] perf util: Move block_pair_cmp to block-info Jiri Olsa
2 siblings, 0 replies; 7+ messages in thread
From: Jin Yao @ 2020-01-06 19:45 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
It would be nice to print the block percents with colors.
This patch supports the 'Sampled Cycles%' and 'Avg Cycles%'
printed in colors.
For example,
perf record -b ...
perf report --total-cycles or perf report --total-cycles --stdio
percent > 5%, colored in red
percent > 0.5%, colored in green
percent < 0.5%, default color
v2:
---
No functional change
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/util/block-info.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c
index 0818db17b3f2..60b3a0107177 100644
--- a/tools/perf/util/block-info.c
+++ b/tools/perf/util/block-info.c
@@ -185,6 +185,17 @@ static int block_column_width(struct perf_hpp_fmt *fmt,
return block_fmt->width;
}
+static int color_pct(struct perf_hpp *hpp, int width, double pct)
+{
+#ifdef HAVE_SLANG_SUPPORT
+ if (use_browser) {
+ return __hpp__slsmg_color_printf(hpp, "%*.2f%%",
+ width - 1, pct);
+ }
+#endif
+ return hpp_color_scnprintf(hpp, "%*.2f%%", width - 1, pct);
+}
+
static int block_total_cycles_pct_entry(struct perf_hpp_fmt *fmt,
struct perf_hpp *hpp,
struct hist_entry *he)
@@ -192,14 +203,11 @@ static int block_total_cycles_pct_entry(struct perf_hpp_fmt *fmt,
struct block_fmt *block_fmt = container_of(fmt, struct block_fmt, fmt);
struct block_info *bi = he->block_info;
double ratio = 0.0;
- char buf[16];
if (block_fmt->total_cycles)
ratio = (double)bi->cycles / (double)block_fmt->total_cycles;
- sprintf(buf, "%.2f%%", 100.0 * ratio);
-
- return scnprintf(hpp->buf, hpp->size, "%*s", block_fmt->width, buf);
+ return color_pct(hpp, block_fmt->width, 100.0 * ratio);
}
static int64_t block_total_cycles_pct_sort(struct perf_hpp_fmt *fmt,
@@ -252,16 +260,13 @@ static int block_cycles_pct_entry(struct perf_hpp_fmt *fmt,
struct block_info *bi = he->block_info;
double ratio = 0.0;
u64 avg;
- char buf[16];
if (block_fmt->block_cycles && bi->num_aggr) {
avg = bi->cycles_aggr / bi->num_aggr;
ratio = (double)avg / (double)block_fmt->block_cycles;
}
- sprintf(buf, "%.2f%%", 100.0 * ratio);
-
- return scnprintf(hpp->buf, hpp->size, "%*s", block_fmt->width, buf);
+ return color_pct(hpp, block_fmt->width, 100.0 * ratio);
}
static int block_avg_cycles_entry(struct perf_hpp_fmt *fmt,
@@ -348,7 +353,7 @@ static void hpp_register(struct block_fmt *block_fmt, int idx,
switch (idx) {
case PERF_HPP_REPORT__BLOCK_TOTAL_CYCLES_PCT:
- fmt->entry = block_total_cycles_pct_entry;
+ fmt->color = block_total_cycles_pct_entry;
fmt->cmp = block_info__cmp;
fmt->sort = block_total_cycles_pct_sort;
break;
@@ -356,7 +361,7 @@ static void hpp_register(struct block_fmt *block_fmt, int idx,
fmt->entry = block_cycles_lbr_entry;
break;
case PERF_HPP_REPORT__BLOCK_CYCLES_PCT:
- fmt->entry = block_cycles_pct_entry;
+ fmt->color = block_cycles_pct_entry;
break;
case PERF_HPP_REPORT__BLOCK_AVG_CYCLES:
fmt->entry = block_avg_cycles_entry;
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 1/3] perf util: Move block_pair_cmp to block-info
2020-01-06 19:45 [PATCH v2 1/3] perf util: Move block_pair_cmp to block-info Jin Yao
2020-01-06 19:45 ` [PATCH v2 2/3] perf util: Flexible to set block info output formats Jin Yao
2020-01-06 19:45 ` [PATCH v2 3/3] perf util: Support color ops to print block percents in color Jin Yao
@ 2020-01-07 9:57 ` Jiri Olsa
2020-01-07 13:57 ` Jin, Yao
2 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2020-01-07 9:57 UTC (permalink / raw)
To: Jin Yao
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On Tue, Jan 07, 2020 at 03:45:23AM +0800, Jin Yao wrote:
> block_pair_cmp() is a function which is used to compare
> two blocks. Moving it from builtin-diff.c to block-info.c
> to let it be used by other builtins.
>
> In block_pair_cmp, there is a minor change. It checks valid
> for map, dso and sym first. If they are invalid, we will not
> compare the address because the address might not make sense.
please separate the change as well, it's hard to track
what you did when the whole function is moved
>
> v2:
> ---
> New patch created in v2
>
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
> tools/perf/builtin-diff.c | 17 -----------------
> tools/perf/util/block-info.c | 23 +++++++++++++++++++++++
> tools/perf/util/block-info.h | 2 ++
> 3 files changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index f8b6ae557d8b..5ff1e21082cb 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -572,23 +572,6 @@ static void init_block_hist(struct block_hist *bh)
> bh->valid = true;
> }
>
> -static int block_pair_cmp(struct hist_entry *a, struct hist_entry *b)
> -{
> - struct block_info *bi_a = a->block_info;
> - struct block_info *bi_b = b->block_info;
> - int cmp;
> -
> - if (!bi_a->sym || !bi_b->sym)
> - return -1;
> -
> - cmp = strcmp(bi_a->sym->name, bi_b->sym->name);
> -
> - if ((!cmp) && (bi_a->start == bi_b->start) && (bi_a->end == bi_b->end))
> - return 0;
> -
> - return -1;
> -}
> -
> static struct hist_entry *get_block_pair(struct hist_entry *he,
> struct hists *hists_pair)
> {
> diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c
> index c4b030bf6ec2..18a445938681 100644
> --- a/tools/perf/util/block-info.c
> +++ b/tools/perf/util/block-info.c
> @@ -475,3 +475,26 @@ float block_info__total_cycles_percent(struct hist_entry *he)
>
> return 0.0;
> }
> +
> +int block_pair_cmp(struct hist_entry *pair, struct hist_entry *he)
> +{
> + struct block_info *bi_p = pair->block_info;
> + struct block_info *bi_h = he->block_info;
> + struct map_symbol *ms_p = &pair->ms;
> + struct map_symbol *ms_h = &he->ms;
> + int cmp;
> +
> + if (!ms_p->map || !ms_p->map->dso || !ms_p->sym ||
> + !ms_h->map || !ms_h->map->dso || !ms_h->sym) {
> + return -1;
> + }
> +
> + cmp = strcmp(ms_p->sym->name, ms_h->sym->name);
> + if (cmp)
> + return -1;
should this return cmp? also you don't mention this change in the changelog
thanks,
jirka
^ permalink raw reply [flat|nested] 7+ messages in thread