Re: [PATCH v1 0/4] perf annotate: Create a new '--tui-dump' option

From: Arnaldo Carvalho de Melo
Date: Tue Mar 13 2018 - 11:21:08 EST


Em Tue, Mar 13, 2018 at 10:16:50PM +0800, Jin Yao escreveu:
> There is a requirement to let perf annotate support displaying the IPC/Cycle.
> In previous patch, this is supported in TUI mode. While it's not convenient
> for users since they have to take screen shots and copy/paste data.
>
> This patch series introduces a new option '--tui-dump' in perf annotate to
> dump the TUI output to stdio.
>
> User can easily use the command line like:
> 'perf annotate --tui-dump > /tmp/log.txt'

My first impression is that this pollutes the code with way too many
ifs, I was thinking more of a:

while (read parsed objdump line) {
ins__fprintf();
}

Going from the refresh routine, I started doing the conversion, but haven't
completed it, there are opportunities for more __scnprintf like routines, also
one to find the percent_max, etc then those would be used both in these two for
--stdio2, that eventually would become --stdio with the old one becoming
--stdio1, till we're satisfied with the new default.

static void annotation_line__fprintf(struct annotate_line *al, FILE *fp)
{
struct browser_line *bl = browser_line(al);
int i;
double percent_max = 0.0;
char bf[256];

for (i = 0; i < browser->nr_events; i++) {
if (al->samples[i].percent > percent_max)
percent_max = al->samples[i].percent;
}

/* the following if/else block should be transformed into a __scnprintf routine
that formats a buffer and then the TUI and --stdio2 use it */

if (al->offset != -1 && percent_max != 0.0) {
for (i = 0; i < ab->nr_events; i++) {
if (annotate_browser__opts.show_total_period) {
fprintf(fp, browser, "%11" PRIu64 " ", al->samples[i].he.period);
} else if (annotate_browser__opts.show_nr_samples) {
fprintf(fp, browser, "%6" PRIu64 " ", al->samples[i].he.nr_samples);
} else {
fprintf(fp, "%6.2f ", al->samples[i].percent);
}
}
} else {
ui_browser__printf(browser, "%*s", pcnt_width,
annotate_browser__opts.show_total_period ? "Period" :
annotate_browser__opts.show_nr_samples ? "Samples" : "Percent");

}

/* The ab->have_cycles should go to a separate struct, outside
* annotation_browser, and the rest should go to something that just does scnprintf on a buffer
* that then is printed on the TUI or with fprintf */

if (ab->have_cycles) {
if (al->ipc)
fprintf(fp, "%*.2f ", IPC_WIDTH - 1, al->ipc);
else if (show_title)
ui_browser__printf(browser, "%*s ", IPC_WIDTH - 1, "IPC");

if (al->cycles)
ui_browser__printf(browser, "%*" PRIu64 " ",
CYCLES_WIDTH - 1, al->cycles);
else if (!show_title)
ui_browser__write_nstring(browser, " ", CYCLES_WIDTH);
else
ui_browser__printf(browser, "%*s ", CYCLES_WIDTH - 1, "Cycle");
}

SLsmg_write_char(' ');

/* The scroll bar isn't being used */
if (!browser->navkeypressed)
width += 1;

if (!*al->line)
fprintf(fp, "\n");
else if (al->offset == -1) {
if (al->line_nr && annotate_browser__opts.show_linenr)
printed = scnprintf(bf, sizeof(bf), "%-*d ",
ab->addr_width + 1, al->line_nr);
else
printed = scnprintf(bf, sizeof(bf), "%*s ",
ab->addr_width, " ");
fprintf(fp, bf);
ui_browser__write_nstring(browser, al->line, width - printed - pcnt_width - cycles_width + 1);
} else {
u64 addr = al->offset;
int color = -1;

if (!annotate_browser__opts.use_offset)
addr += ab->start;

if (!annotate_browser__opts.use_offset) {
printed = scnprintf(bf, sizeof(bf), "%" PRIx64 ": ", addr);
} else {
if (bl->jump_sources) {
if (annotate_browser__opts.show_nr_jumps) {
int prev;
printed = scnprintf(bf, sizeof(bf), "%*d ",
ab->jumps_width,
bl->jump_sources);
prev = annotate_browser__set_jumps_percent_color(ab, bl->jump_sources,
current_entry);
ui_browser__write_nstring(browser, bf, printed);
ui_browser__set_color(browser, prev);
}

printed = scnprintf(bf, sizeof(bf), "%*" PRIx64 ": ",
ab->target_width, addr);
} else {
printed = scnprintf(bf, sizeof(bf), "%*s ",
ab->addr_width, " ");
}
}

fprintf(fp, bf);

disasm_line__write(disasm_line(al), browser, bf, sizeof(bf));

ui_browser__write_nstring(browser, bf, width - pcnt_width - cycles_width - 3 - printed);
}
}

unsigned int annotation_lines__fprintf(struct list_head *lines, FILE *fp)
{
struct list_head *line;
struct annotation_line *al;

list_for_each(line, lines) {
struct annotation_line *al = list_entry(line, struct annotation_line, node);
annotation_line__fprintf(al, line);
}

return row;
}

Then the main code would use the same code that creates the browser->b.entries
and would pass it to annpotation_lines__fprintf().

i.e. we would disentanble the formatting of strings and auxiliary routines to
obtain the max_percent, i.e. nothing of TUI is needed for --stdio2, just the
formatting of strings.

- Arnaldo

> For example:
> $ perf annotate compute_flag --tui-dump
>
> Percent IPC Cycle
>
> Disassembly of section .text:
>
> 0000000000400640 <compute_flag>:
> compute_flag():
> volatile int count;
> static unsigned int s_randseed;
>
> __attribute__((noinline))
> int compute_flag()
> {
> 23.00 1.16 sub $0x8,%rsp
> int i;
>
> i = rand() % 2;
> 23.06 1.16 1 callq rand@plt
>
> return i;
> 27.01 3.38 mov %eax,%edx
> }
> 3.38 add $0x8,%rsp
> {
> int i;
>
> i = rand() % 2;
>
> return i;
> 3.38 shr $0x1f,%edx
> 3.38 add %edx,%eax
> 3.38 and $0x1,%eax
> 3.38 sub %edx,%eax
> }
> 26.93 3.38 2 retq
>
> The '--stdio' option is still kept now. Maybe in future, we can only
> maintain the TUI routines and drop the lagacy stdio code. But right
> now we'd better keep it until the '--tui-dump' option is good enough.
>
> Jin Yao (4):
> perf browser: Add a new 'dump' flag
> perf browser: bypass ui_init if in tui dump mode
> perf annotate: Process the new switch flag tui_dump
> perf annotate: Enable the '--tui-dump' mode
>
> tools/perf/Documentation/perf-annotate.txt | 3 +++
> tools/perf/builtin-annotate.c | 12 +++++++--
> tools/perf/builtin-c2c.c | 2 +-
> tools/perf/builtin-report.c | 2 +-
> tools/perf/builtin-top.c | 2 +-
> tools/perf/ui/browser.c | 38 +++++++++++++++++++++++----
> tools/perf/ui/browser.h | 1 +
> tools/perf/ui/browsers/annotate.c | 41 +++++++++++++++++++++---------
> tools/perf/ui/browsers/hists.c | 2 +-
> tools/perf/ui/setup.c | 9 +++++--
> tools/perf/ui/ui.h | 2 +-
> tools/perf/util/annotate.h | 6 +++--
> tools/perf/util/hist.h | 11 +++++---
> 13 files changed, 99 insertions(+), 32 deletions(-)
>
> --
> 2.7.4