[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAM9d7cgQWLdec063U+c1su_O9jchv5HSTQ0S0tQJ_q96hjgjXw@mail.gmail.com>
Date: Fri, 2 Aug 2024 17:18:07 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: kan.liang@...ux.intel.com
Cc: acme@...nel.org, irogers@...gle.com, peterz@...radead.org,
mingo@...nel.org, linux-kernel@...r.kernel.org, adrian.hunter@...el.com,
ak@...ux.intel.com, eranian@...gle.com
Subject: Re: [PATCH 6/9] perf report: Display the branch counter histogram
On Wed, Jul 3, 2024 at 1:03 PM <kan.liang@...ux.intel.com> wrote:
>
> From: Kan Liang <kan.liang@...ux.intel.com>
>
> Reusing the existing --total-cycles option to display the branch
> counters. Add a new PERF_HPP_REPORT__BLOCK_BRANCH_COUNTER to display
> the logged branch counter events. They are shown right after all the
> cycle-related annotations.
> Extend the struct block_info to store and pass the branch counter
> related information.
>
> The annotation_br_cntr_entry() is to print the histogram of each branch
> counter event.
> The annotation_br_cntr_abbr_list() prints the branch counter's
> abbreviation list. Press 'B' to display the list in the TUI mode.
>
> $perf record -e "{branch-instructions:ppp,branch-misses}:S" -j any,counter
> $perf report --total-cycles --stdio
>
> # To display the perf.data header info, please use --header/--header-only options.
> #
> #
> # Total Lost Samples: 0
> #
> # Samples: 1M of events 'anon group { branch-instructions:ppp, branch-misses }'
> # Event count (approx.): 1610046
> #
> # Branch counter abbr list:
> # branch-instructions:ppp = A
> # branch-misses = B
> # '-' No event occurs
> # '+' Event occurrences may be lost due to branch counter saturated
> #
> # Sampled Cycles% Sampled Cycles Avg Cycles% Avg Cycles Branch Counter [Program Block Range]
> # ............... .............. ........... .......... ...................... ..................
> #
> 57.55% 2.5M 0.00% 3 |A |- | ...
> 25.27% 1.1M 0.00% 2 |AA |- | ...
> 15.61% 667.2K 0.00% 1 |A |- | ...
> 0.16% 6.9K 0.81% 575 |A |- | ...
> 0.16% 6.8K 1.38% 977 |AA |- | ...
> 0.16% 6.8K 0.04% 28 |AA |B | ...
> 0.15% 6.6K 1.33% 946 |A |- | ...
> 0.11% 4.5K 0.06% 46 |AAA+|- | ...
> 0.10% 4.4K 0.88% 624 |A |- | ...
> 0.09% 3.7K 0.74% 524 |AAA+|B | ...
I think this format assumes short width and might not work
well when it has more events with bigger width. Maybe
A=<n>, B=<n> ?
Thanks,
Namhyung
>
> Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
> ---
> tools/perf/Documentation/perf-report.txt | 1 +
> tools/perf/builtin-diff.c | 4 +-
> tools/perf/builtin-report.c | 20 ++++-
> tools/perf/ui/browsers/hists.c | 17 +++-
> tools/perf/util/annotate.c | 101 +++++++++++++++++++++++
> tools/perf/util/annotate.h | 3 +
> tools/perf/util/block-info.c | 66 +++++++++++++--
> tools/perf/util/block-info.h | 8 +-
> 8 files changed, 202 insertions(+), 18 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index d2b1593ef700..f35189d5ff1e 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -614,6 +614,7 @@ include::itrace.txt[]
> 'Avg Cycles%' - block average sampled cycles / sum of total block average
> sampled cycles
> 'Avg Cycles' - block average sampled cycles
> + 'Branch Counter' - block branch counter histogram
>
> --skip-empty::
> Do not print 0 results in the --stat output.
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index 2d9226b1de52..de24892dc7b8 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -705,7 +705,7 @@ static void hists__precompute(struct hists *hists)
> if (compute == COMPUTE_CYCLES) {
> bh = container_of(he, struct block_hist, he);
> init_block_hist(bh);
> - block_info__process_sym(he, bh, NULL, 0);
> + block_info__process_sym(he, bh, NULL, 0, 0);
> }
>
> data__for_each_file_new(i, d) {
> @@ -728,7 +728,7 @@ static void hists__precompute(struct hists *hists)
> pair_bh = container_of(pair, struct block_hist,
> he);
> init_block_hist(pair_bh);
> - block_info__process_sym(pair, pair_bh, NULL, 0);
> + block_info__process_sym(pair, pair_bh, NULL, 0, 0);
>
> bh = container_of(he, struct block_hist, he);
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index da8d13bbb500..a0f864f2e996 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -575,6 +575,13 @@ static int evlist__tty_browse_hists(struct evlist *evlist, struct report *rep, c
> hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
>
> if (rep->total_cycles_mode) {
> + char *buf;
> +
> + if (!annotation_br_cntr_abbr_list(&buf, pos, true)) {
> + fprintf(stdout, "%s", buf);
> + fprintf(stdout, "#\n");
> + free(buf);
> + }
> report__browse_block_hists(&rep->block_reports[i - 1].hist,
> rep->min_percent, pos, NULL);
> continue;
> @@ -1121,18 +1128,23 @@ static int __cmd_report(struct report *rep)
> report__output_resort(rep);
>
> if (rep->total_cycles_mode) {
> - int block_hpps[6] = {
> + int nr_hpps = 4;
> + int block_hpps[PERF_HPP_REPORT__BLOCK_MAX_INDEX] = {
> 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,
> };
>
> + if (session->evlist->nr_br_cntr > 0)
> + block_hpps[nr_hpps++] = PERF_HPP_REPORT__BLOCK_BRANCH_COUNTER;
> +
> + block_hpps[nr_hpps++] = PERF_HPP_REPORT__BLOCK_RANGE;
> + block_hpps[nr_hpps++] = PERF_HPP_REPORT__BLOCK_DSO;
> +
> rep->block_reports = block_info__create_report(session->evlist,
> rep->total_cycles,
> - block_hpps, 6,
> + block_hpps, nr_hpps,
> &rep->nr_block_reports);
> if (!rep->block_reports)
> return -1;
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index b7219df51236..73d766eac75b 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -3684,8 +3684,10 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
> struct hist_browser *browser;
> int key = -1;
> struct popup_action action;
> + char *br_cntr_text = NULL;
> static const char help[] =
> - " q Quit \n";
> + " q Quit \n"
> + " B Branch counter abbr list (Optional)\n";
>
> browser = hist_browser__new(hists);
> if (!browser)
> @@ -3703,6 +3705,8 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
>
> memset(&action, 0, sizeof(action));
>
> + annotation_br_cntr_abbr_list(&br_cntr_text, evsel, false);
> +
> while (1) {
> key = hist_browser__run(browser, "? - help", true, 0);
>
> @@ -3723,6 +3727,16 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
> action.ms.sym = browser->selection->sym;
> do_annotate(browser, &action);
> continue;
> + case 'B':
> + if (br_cntr_text) {
> + ui__question_window("Branch counter abbr list",
> + br_cntr_text, "Press any key...", 0);
> + } else {
> + ui__question_window("Branch counter abbr list",
> + "\n The branch counter is not available.\n",
> + "Press any key...", 0);
> + }
> + continue;
> default:
> break;
> }
> @@ -3730,5 +3744,6 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
>
> out:
> hist_browser__delete(browser);
> + free(br_cntr_text);
> return 0;
> }
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 6baa0671598e..f20f9e40ef0d 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -40,6 +40,7 @@
> #include "namespaces.h"
> #include "thread.h"
> #include "hashmap.h"
> +#include "strbuf.h"
> #include <regex.h>
> #include <linux/bitops.h>
> #include <linux/kernel.h>
> @@ -47,6 +48,7 @@
> #include <linux/zalloc.h>
> #include <subcmd/parse-options.h>
> #include <subcmd/run-command.h>
> +#include <math.h>
>
> /* FIXME: For the HE_COLORSET */
> #include "ui/browser.h"
> @@ -1706,6 +1708,105 @@ static void ipc_coverage_string(char *bf, int size, struct annotation *notes)
> ipc, coverage);
> }
>
> +int annotation_br_cntr_abbr_list(char **str, struct evsel *evsel, bool header)
> +{
> + struct evsel *pos;
> + struct strbuf sb;
> +
> + if (evsel->evlist->nr_br_cntr <= 0)
> + return -ENOTSUP;
> +
> + strbuf_init(&sb, /*hint=*/ 0);
> +
> + if (header && strbuf_addf(&sb, "# Branch counter abbr list:\n"))
> + goto err;
> +
> + evlist__for_each_entry(evsel->evlist, pos) {
> + if (!(pos->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_COUNTERS))
> + continue;
> + if (header && strbuf_addf(&sb, "#"))
> + goto err;
> +
> + if (strbuf_addf(&sb, " %s = %s\n", pos->name, pos->abbr_name))
> + goto err;
> + }
> +
> + if (header && strbuf_addf(&sb, "#"))
> + goto err;
> + if (strbuf_addf(&sb, " '-' No event occurs\n"))
> + goto err;
> +
> + if (header && strbuf_addf(&sb, "#"))
> + goto err;
> + if (strbuf_addf(&sb, " '+' Event occurrences may be lost due to branch counter saturated\n"))
> + goto err;
> +
> + *str = strbuf_detach(&sb, NULL);
> +
> + return 0;
> +err:
> + strbuf_release(&sb);
> + return -ENOMEM;
> +}
> +
> +int annotation_br_cntr_entry(char **str, int br_cntr_nr,
> + u64 *br_cntr, int num_aggr,
> + struct evsel *evsel)
> +{
> + struct evsel *pos = evsel ? evlist__first(evsel->evlist) : NULL;
> + int i, j, avg, used;
> + struct strbuf sb;
> +
> + strbuf_init(&sb, /*hint=*/ 0);
> + for (i = 0; i < br_cntr_nr; i++) {
> + used = 0;
> + avg = ceil((double)(br_cntr[i] & ~ANNOTATION__BR_CNTR_SATURATED_FLAG) /
> + (double)num_aggr);
> +
> + if (strbuf_addch(&sb, '|'))
> + goto err;
> +
> + if (!br_cntr[i]) {
> + if (strbuf_addch(&sb, '-'))
> + goto err;
> + used++;
> + } else {
> + evlist__for_each_entry_from(evsel->evlist, pos) {
> + if ((pos->core.attr.branch_sample_type & PERF_SAMPLE_BRANCH_COUNTERS) &&
> + (pos->br_cntr_idx == i))
> + break;
> + }
> + for (j = 0; j < avg; j++, used++) {
> + if (strbuf_addstr(&sb, pos->abbr_name))
> + goto err;
> + }
> +
> + if (br_cntr[i] & ANNOTATION__BR_CNTR_SATURATED_FLAG) {
> + if (strbuf_addch(&sb, '+'))
> + goto err;
> + used++;
> + }
> + pos = list_next_entry(pos, core.node);
> + }
> +
> + /* Assume the branch counter saturated at 3 */
> + for (j = used; j < 4; j++) {
> + if (strbuf_addch(&sb, ' '))
> + goto err;
> + }
> + }
> +
> + if (strbuf_addch(&sb, br_cntr_nr ? '|' : ' '))
> + goto err;
> +
> + *str = strbuf_detach(&sb, NULL);
> +
> + return 0;
> +err:
> + strbuf_release(&sb);
> + return -ENOMEM;
> +}
> +
> static void __annotation_line__write(struct annotation_line *al, struct annotation *notes,
> bool first_line, bool current_entry, bool change_color, int width,
> void *obj, unsigned int percent_type,
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index f39dd5d7b05e..2ff79a389dc0 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -548,4 +548,7 @@ struct annotated_basic_block {
> int annotate_get_basic_blocks(struct symbol *sym, s64 src, s64 dst,
> struct list_head *head);
>
> +int annotation_br_cntr_entry(char **str, int br_cntr_nr, u64 *br_cntr,
> + int num_aggr, struct evsel *evsel);
> +int annotation_br_cntr_abbr_list(char **str, struct evsel *evsel, bool header);
> #endif /* __PERF_ANNOTATE_H */
> diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c
> index 04068d48683f..649392bee7ed 100644
> --- a/tools/perf/util/block-info.c
> +++ b/tools/perf/util/block-info.c
> @@ -40,16 +40,32 @@ static struct block_header_column {
> [PERF_HPP_REPORT__BLOCK_DSO] = {
> .name = "Shared Object",
> .width = 20,
> + },
> + [PERF_HPP_REPORT__BLOCK_BRANCH_COUNTER] = {
> + .name = "Branch Counter",
> + .width = 30,
> }
> };
>
> -struct block_info *block_info__new(void)
> +static struct block_info *block_info__new(unsigned int br_cntr_nr)
> {
> - return zalloc(sizeof(struct block_info));
> + struct block_info *bi = zalloc(sizeof(struct block_info));
> +
> + if (bi && br_cntr_nr) {
> + bi->br_cntr = calloc(br_cntr_nr, sizeof(u64));
> + if (!bi->br_cntr) {
> + free(bi);
> + return NULL;
> + }
> + }
> +
> + return bi;
> }
>
> void block_info__delete(struct block_info *bi)
> {
> + if (bi)
> + free(bi->br_cntr);
> free(bi);
> }
>
> @@ -86,7 +102,8 @@ int64_t block_info__cmp(struct perf_hpp_fmt *fmt __maybe_unused,
>
> static void init_block_info(struct block_info *bi, struct symbol *sym,
> struct cyc_hist *ch, int offset,
> - u64 total_cycles)
> + u64 total_cycles, unsigned int br_cntr_nr,
> + u64 *br_cntr, struct evsel *evsel)
> {
> bi->sym = sym;
> bi->start = ch->start;
> @@ -99,10 +116,18 @@ static void init_block_info(struct block_info *bi, struct symbol *sym,
>
> memcpy(bi->cycles_spark, ch->cycles_spark,
> NUM_SPARKS * sizeof(u64));
> +
> + if (br_cntr && br_cntr_nr) {
> + bi->br_cntr_nr = br_cntr_nr;
> + memcpy(bi->br_cntr, &br_cntr[offset * br_cntr_nr],
> + br_cntr_nr * sizeof(u64));
> + }
> + bi->evsel = evsel;
> }
>
> int block_info__process_sym(struct hist_entry *he, struct block_hist *bh,
> - u64 *block_cycles_aggr, u64 total_cycles)
> + u64 *block_cycles_aggr, u64 total_cycles,
> + unsigned int br_cntr_nr)
> {
> struct annotation *notes;
> struct cyc_hist *ch;
> @@ -125,12 +150,14 @@ int block_info__process_sym(struct hist_entry *he, struct block_hist *bh,
> struct block_info *bi;
> struct hist_entry *he_block;
>
> - bi = block_info__new();
> + bi = block_info__new(br_cntr_nr);
> if (!bi)
> return -1;
>
> init_block_info(bi, he->ms.sym, &ch[i], i,
> - total_cycles);
> + total_cycles, br_cntr_nr,
> + notes->branch->br_cntr,
> + hists_to_evsel(he->hists));
> cycles += bi->cycles_aggr / bi->num_aggr;
>
> he_block = hists__add_entry_block(&bh->block_hists,
> @@ -327,6 +354,24 @@ static void init_block_header(struct block_fmt *block_fmt)
> fmt->width = block_column_width;
> }
>
> +static int block_branch_counter_entry(struct perf_hpp_fmt *fmt,
> + struct perf_hpp *hpp,
> + struct hist_entry *he)
> +{
> + struct block_fmt *block_fmt = container_of(fmt, struct block_fmt, fmt);
> + struct block_info *bi = he->block_info;
> + char *buf;
> + int ret;
> +
> + if (annotation_br_cntr_entry(&buf, bi->br_cntr_nr, bi->br_cntr,
> + bi->num_aggr, bi->evsel))
> + return 0;
> +
> + ret = scnprintf(hpp->buf, hpp->size, "%*s", block_fmt->width, buf);
> + free(buf);
> + return ret;
> +}
> +
> static void hpp_register(struct block_fmt *block_fmt, int idx,
> struct perf_hpp_list *hpp_list)
> {
> @@ -357,6 +402,9 @@ static void hpp_register(struct block_fmt *block_fmt, int idx,
> case PERF_HPP_REPORT__BLOCK_DSO:
> fmt->entry = block_dso_entry;
> break;
> + case PERF_HPP_REPORT__BLOCK_BRANCH_COUNTER:
> + fmt->entry = block_branch_counter_entry;
> + break;
> default:
> return;
> }
> @@ -390,7 +438,7 @@ static void init_block_hist(struct block_hist *bh, struct block_fmt *block_fmts,
> static int process_block_report(struct hists *hists,
> struct block_report *block_report,
> u64 total_cycles, int *block_hpps,
> - int nr_hpps)
> + int nr_hpps, unsigned int br_cntr_nr)
> {
> struct rb_node *next = rb_first_cached(&hists->entries);
> struct block_hist *bh = &block_report->hist;
> @@ -405,7 +453,7 @@ static int process_block_report(struct hists *hists,
> while (next) {
> he = rb_entry(next, struct hist_entry, rb_node);
> block_info__process_sym(he, bh, &block_report->cycles,
> - total_cycles);
> + total_cycles, br_cntr_nr);
> next = rb_next(&he->rb_node);
> }
>
> @@ -435,7 +483,7 @@ struct block_report *block_info__create_report(struct evlist *evlist,
> struct hists *hists = evsel__hists(pos);
>
> process_block_report(hists, &block_reports[i], total_cycles,
> - block_hpps, nr_hpps);
> + block_hpps, nr_hpps, evlist->nr_br_cntr);
> i++;
> }
>
> diff --git a/tools/perf/util/block-info.h b/tools/perf/util/block-info.h
> index 0b9e1aad4c55..b9329dc3ab59 100644
> --- a/tools/perf/util/block-info.h
> +++ b/tools/perf/util/block-info.h
> @@ -18,6 +18,9 @@ struct block_info {
> u64 total_cycles;
> int num;
> int num_aggr;
> + int br_cntr_nr;
> + u64 *br_cntr;
> + struct evsel *evsel;
> };
>
> struct block_fmt {
> @@ -36,6 +39,7 @@ enum {
> PERF_HPP_REPORT__BLOCK_AVG_CYCLES,
> PERF_HPP_REPORT__BLOCK_RANGE,
> PERF_HPP_REPORT__BLOCK_DSO,
> + PERF_HPP_REPORT__BLOCK_BRANCH_COUNTER,
> PERF_HPP_REPORT__BLOCK_MAX_INDEX
> };
>
> @@ -46,7 +50,6 @@ struct block_report {
> int nr_fmts;
> };
>
> -struct block_info *block_info__new(void);
> void block_info__delete(struct block_info *bi);
>
> int64_t __block_info__cmp(struct hist_entry *left, struct hist_entry *right);
> @@ -55,7 +58,8 @@ int64_t block_info__cmp(struct perf_hpp_fmt *fmt __maybe_unused,
> struct hist_entry *left, struct hist_entry *right);
>
> int block_info__process_sym(struct hist_entry *he, struct block_hist *bh,
> - u64 *block_cycles_aggr, u64 total_cycles);
> + u64 *block_cycles_aggr, u64 total_cycles,
> + unsigned int br_cntr_nr);
>
> struct block_report *block_info__create_report(struct evlist *evlist,
> u64 total_cycles,
> --
> 2.38.1
>
Powered by blists - more mailing lists