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