Re: [PATCH v3] perf hists browser: Consolidate callchain print functions in TUI

From: Arnaldo Carvalho de Melo
Date: Fri Aug 22 2014 - 11:21:58 EST

Em Fri, Aug 22, 2014 at 09:13:21AM +0900, Namhyung Kim escreveu:
> Currently there're two callchain print functions in TUI - one for the
> hists browser and another for file dump. They do almost same job so
> it'd be better consolidate the codes.
>
> To do that, provide two callbacks to the generic logic - one for
> printing and another for checking whether it should stop.

Thanks a lot, I just changed the name of the check_output_full_fn
parameter to "is_output_full", as "check" is way too generic to help one
understand what is it that is being checked.

I also aligned aligned the members of the struct callchain_print_arg, as
suggested by Ingo.

Applied.

- Arnaldo

> Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> ---
> tools/perf/ui/browsers/hists.c | 203 ++++++++++++++++-------------------------
> 1 file changed, 80 insertions(+), 123 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 519353d9f5fb..026421e0d53d 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -477,20 +477,37 @@ static char *callchain_list__sym_name(struct callchain_list *cl,
> return bf;
> }
>
> +struct callchain_print_arg {
> + /* for hists browser */
> + off_t row_offset;
> + bool is_current_entry;
> +
> + /* for file dump */
> + FILE *fp;
> + int printed;
> +};
> +
> +typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
> + struct callchain_list *chain,
> + const char *str, int offset,
> + unsigned short row,
> + struct callchain_print_arg *arg);
> +
> static void hist_browser__show_callchain_entry(struct hist_browser *browser,
> struct callchain_list *chain,
> - unsigned short row, int offset,
> - char folded_sign, const char *str,
> - bool *is_current_entry)
> + const char *str, int offset,
> + unsigned short row,
> + struct callchain_print_arg *arg)
> {
> int color, width;
> + char folded_sign = callchain_list__folded(chain);
>
> color = HE_COLORSET_NORMAL;
> width = browser->b.width - (offset + 2);
> if (ui_browser__is_current_entry(&browser->b, row)) {
> browser->selection = &chain->ms;
> color = HE_COLORSET_SELECTED;
> - *is_current_entry = true;
> + arg->is_current_entry = true;
> }
>
> ui_browser__set_color(&browser->b, color);
> @@ -500,12 +517,41 @@ static void hist_browser__show_callchain_entry(struct hist_browser *browser,
> slsmg_write_nstring(str, width);
> }
>
> +static void hist_browser__fprintf_callchain_entry(struct hist_browser *b __maybe_unused,
> + struct callchain_list *chain,
> + const char *str, int offset,
> + unsigned short row __maybe_unused,
> + struct callchain_print_arg *arg)
> +{
> + char folded_sign = callchain_list__folded(chain);
> +
> + arg->printed += fprintf(arg->fp, "%*s%c %s\n", offset, " ",
> + folded_sign, str);
> +}
> +
> +typedef bool (*check_output_full_fn)(struct hist_browser *browser,
> + unsigned short row);
> +
> +static bool hist_browser__check_output_full(struct hist_browser *browser,
> + unsigned short row)
> +{
> + return browser->b.rows == row;
> +}
> +
> +static bool hist_browser__check_dump_full(struct hist_browser *browser __maybe_unused,
> + unsigned short row __maybe_unused)
> +{
> + return false;
> +}
> +
> #define LEVEL_OFFSET_STEP 3
>
> static int hist_browser__show_callchain(struct hist_browser *browser,
> struct rb_root *root, int level,
> - unsigned short row, off_t *row_offset,
> - u64 total, bool *is_current_entry)
> + unsigned short row, u64 total,
> + print_callchain_entry_fn print,
> + struct callchain_print_arg *arg,
> + check_output_full_fn check)
> {
> struct rb_node *node;
> int first_row = row, offset = level * LEVEL_OFFSET_STEP;
> @@ -532,8 +578,8 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
> extra_offset = LEVEL_OFFSET_STEP;
>
> folded_sign = callchain_list__folded(chain);
> - if (*row_offset != 0) {
> - --*row_offset;
> + if (arg->row_offset != 0) {
> + arg->row_offset--;
> goto do_next;
> }
>
> @@ -550,13 +596,11 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
> str = alloc_str;
> }
>
> - hist_browser__show_callchain_entry(browser, chain, row,
> - offset + extra_offset,
> - folded_sign, str,
> - is_current_entry);
> + print(browser, chain, str, offset + extra_offset, row, arg);
> +
> free(alloc_str);
>
> - if (++row == browser->b.rows)
> + if (check(browser, ++row))
> goto out;
> do_next:
> if (folded_sign == '+')
> @@ -572,12 +616,10 @@ do_next:
> new_total = total;
>
> row += hist_browser__show_callchain(browser, &child->rb_root,
> - new_level,
> - row, row_offset,
> - new_total,
> - is_current_entry);
> + new_level, row, new_total,
> + print, arg, check);
> }
> - if (row == browser->b.rows)
> + if (check(browser, row))
> break;
> node = next;
> }
> @@ -757,16 +799,20 @@ static int hist_browser__show_entry(struct hist_browser *browser,
>
> if (folded_sign == '-' && row != browser->b.rows) {
> u64 total = hists__total_period(entry->hists);
> + struct callchain_print_arg arg = {
> + .row_offset = row_offset,
> + .is_current_entry = current_entry,
> + };
>
> if (symbol_conf.cumulate_callchain)
> total = entry->stat_acc->period;
>
> printed += hist_browser__show_callchain(browser,
> - &entry->sorted_chain,
> - 1, row, &row_offset,
> - total, &current_entry);
> + &entry->sorted_chain, 1, row, total,
> + hist_browser__show_callchain_entry, &arg,
> + hist_browser__check_output_full);
>
> - if (current_entry)
> + if (arg.is_current_entry)
> browser->he_selection = entry;
> }
>
> @@ -1022,110 +1068,21 @@ do_offset:
> }
> }
>
> -static int hist_browser__fprintf_callchain_node_rb_tree(struct hist_browser *browser,
> - struct callchain_node *chain_node,
> - u64 total, int level,
> - FILE *fp)
> -{
> - struct rb_node *node;
> - int offset = level * LEVEL_OFFSET_STEP;
> - u64 new_total;
> - int printed = 0;
> -
> - if (callchain_param.mode == CHAIN_GRAPH_REL)
> - new_total = chain_node->children_hit;
> - else
> - new_total = total;
> -
> - node = rb_first(&chain_node->rb_root);
> - while (node) {
> - struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
> - struct rb_node *next = rb_next(node);
> - u64 cumul = callchain_cumul_hits(child);
> - struct callchain_list *chain;
> - char folded_sign = ' ';
> - int first = true;
> - int extra_offset = 0;
> -
> - list_for_each_entry(chain, &child->val, list) {
> - char bf[1024], *alloc_str;
> - const char *str;
> - bool was_first = first;
> -
> - if (first)
> - first = false;
> - else
> - extra_offset = LEVEL_OFFSET_STEP;
> -
> - folded_sign = callchain_list__folded(chain);
> -
> - alloc_str = NULL;
> - str = callchain_list__sym_name(chain, bf, sizeof(bf),
> - browser->show_dso);
> - if (was_first) {
> - double percent = cumul * 100.0 / new_total;
> -
> - if (asprintf(&alloc_str, "%2.2f%% %s", percent, str) < 0)
> - str = "Not enough memory!";
> - else
> - str = alloc_str;
> - }
> -
> - printed += fprintf(fp, "%*s%c %s\n", offset + extra_offset, " ", folded_sign, str);
> - free(alloc_str);
> - if (folded_sign == '+')
> - break;
> - }
> -
> - if (folded_sign == '-') {
> - const int new_level = level + (extra_offset ? 2 : 1);
> - printed += hist_browser__fprintf_callchain_node_rb_tree(browser, child, new_total,
> - new_level, fp);
> - }
> -
> - node = next;
> - }
> -
> - return printed;
> -}
> -
> -static int hist_browser__fprintf_callchain_node(struct hist_browser *browser,
> - struct callchain_node *node,
> - int level, FILE *fp)
> -{
> - struct callchain_list *chain;
> - int offset = level * LEVEL_OFFSET_STEP;
> - char folded_sign = ' ';
> - int printed = 0;
> -
> - list_for_each_entry(chain, &node->val, list) {
> - char bf[1024], *s;
> -
> - folded_sign = callchain_list__folded(chain);
> - s = callchain_list__sym_name(chain, bf, sizeof(bf), browser->show_dso);
> - printed += fprintf(fp, "%*s%c %s\n", offset, " ", folded_sign, s);
> - }
> -
> - if (folded_sign == '-')
> - printed += hist_browser__fprintf_callchain_node_rb_tree(browser, node,
> - browser->hists->stats.total_period,
> - level + 1, fp);
> - return printed;
> -}
> -
> static int hist_browser__fprintf_callchain(struct hist_browser *browser,
> - struct rb_root *chain, int level, FILE *fp)
> + struct hist_entry *he, FILE *fp)
> {
> - struct rb_node *nd;
> - int printed = 0;
> -
> - for (nd = rb_first(chain); nd; nd = rb_next(nd)) {
> - struct callchain_node *node = rb_entry(nd, struct callchain_node, rb_node);
> + u64 total = hists__total_period(he->hists);
> + struct callchain_print_arg arg = {
> + .fp = fp,
> + };
>
> - printed += hist_browser__fprintf_callchain_node(browser, node, level, fp);
> - }
> + if (symbol_conf.cumulate_callchain)
> + total = he->stat_acc->period;
>
> - return printed;
> + hist_browser__show_callchain(browser, &he->sorted_chain, 1, 0, total,
> + hist_browser__fprintf_callchain_entry, &arg,
> + hist_browser__check_dump_full);
> + return arg.printed;
> }
>
> static int hist_browser__fprintf_entry(struct hist_browser *browser,
> @@ -1164,7 +1121,7 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser,
> printed += fprintf(fp, "%s\n", rtrim(s));
>
> if (folded_sign == '-')
> - printed += hist_browser__fprintf_callchain(browser, &he->sorted_chain, 1, fp);
> + printed += hist_browser__fprintf_callchain(browser, he, fp);
>
> return printed;
> }
> --
> 2.0.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/