Re: [PATCH v5 2/9] perf callchain: Abstract callchain print function

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Mon, Nov 09, 2015 at 02:45:39PM +0900, Namhyung Kim escreveu:
> This is a preparation to support for printing other type of callchain
> value like count or period.
> 
> Acked-by: Brendan Gregg <brendan.d.gregg@gmail.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/browsers/hists.c |  8 +++++---
>  tools/perf/ui/gtk/hists.c      |  8 ++------
>  tools/perf/ui/stdio/hist.c     | 35 +++++++++++++++++------------------
>  tools/perf/util/callchain.c    | 29 +++++++++++++++++++++++++++++
>  tools/perf/util/callchain.h    |  4 ++++
>  5 files changed, 57 insertions(+), 27 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index e5afb8936040..a8897aab4c4a 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -592,7 +592,6 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
>  	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;
> @@ -619,9 +618,12 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
>  						       browser->show_dso);
>  
>  			if (was_first && need_percent) {
> -				double percent = cumul * 100.0 / total;
> +				char buf[64];
>  
> -				if (asprintf(&alloc_str, "%2.2f%% %s", percent, str) < 0)
> +				callchain_node__sprintf_value(child, buf, sizeof(buf),
> +							      total);
> +
> +				if (asprintf(&alloc_str, "%s %s", buf, str) < 0)
>  					str = "Not enough memory!";
>  				else
>  					str = alloc_str;
> diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
> index 4b3585eed1e8..d8037b7023e8 100644
> --- a/tools/perf/ui/gtk/hists.c
> +++ b/tools/perf/ui/gtk/hists.c
> @@ -100,14 +100,10 @@ static void perf_gtk__add_callchain(struct rb_root *root, GtkTreeStore *store,
>  		struct callchain_list *chain;
>  		GtkTreeIter iter, new_parent;
>  		bool need_new_parent;
> -		double percent;
> -		u64 hits, child_total;
> +		u64 child_total;
>  
>  		node = rb_entry(nd, struct callchain_node, rb_node);
>  
> -		hits = callchain_cumul_hits(node);
> -		percent = 100.0 * hits / total;
> -
>  		new_parent = *parent;
>  		need_new_parent = !has_single_node && (node->val_nr > 1);
>  
> @@ -116,7 +112,7 @@ static void perf_gtk__add_callchain(struct rb_root *root, GtkTreeStore *store,
>  
>  			gtk_tree_store_append(store, &iter, &new_parent);
>  
> -			scnprintf(buf, sizeof(buf), "%5.2f%%", percent);
> +			callchain_node__sprintf_value(node, buf, sizeof(buf), total);
>  			gtk_tree_store_set(store, &iter, 0, buf, -1);
>  
>  			callchain_list__sym_name(chain, buf, sizeof(buf), false);
> diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
> index ea7984932d9a..f4de055cab9b 100644
> --- a/tools/perf/ui/stdio/hist.c
> +++ b/tools/perf/ui/stdio/hist.c
> @@ -34,10 +34,10 @@ static size_t ipchain__fprintf_graph_line(FILE *fp, int depth, int depth_mask,
>  	return ret;
>  }
>  
> -static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain,
> +static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_node *node,
> +				     struct callchain_list *chain,
>  				     int depth, int depth_mask, int period,
> -				     u64 total_samples, u64 hits,
> -				     int left_margin)
> +				     u64 total_samples, int left_margin)
>  {
>  	int i;
>  	size_t ret = 0;
> @@ -50,10 +50,9 @@ static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain,
>  		else
>  			ret += fprintf(fp, " ");
>  		if (!period && i == depth - 1) {
> -			double percent;
> -
> -			percent = hits * 100.0 / total_samples;
> -			ret += percent_color_fprintf(fp, "--%2.2f%%-- ", percent);
> +			ret += fprintf(fp, "--");
> +			ret += callchain_node__fprintf_value(node, fp, total_samples);
> +			ret += fprintf(fp, "--");
>  		} else
>  			ret += fprintf(fp, "%s", "          ");
>  	}
> @@ -120,10 +119,9 @@ static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
>  						   left_margin);
>  		i = 0;
>  		list_for_each_entry(chain, &child->val, list) {
> -			ret += ipchain__fprintf_graph(fp, chain, depth,
> +			ret += ipchain__fprintf_graph(fp, child, chain, depth,
>  						      new_depth_mask, i++,
>  						      total_samples,
> -						      cumul,
>  						      left_margin);
>  		}
>  
> @@ -143,14 +141,17 @@ static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
>  
>  	if (callchain_param.mode == CHAIN_GRAPH_REL &&
>  		remaining && remaining != total_samples) {
> +		struct callchain_node rem_node = {
> +			.hit = remaining,
> +		};
>  
>  		if (!rem_sq_bracket)
>  			return ret;
>  
>  		new_depth_mask &= ~(1 << (depth - 1));
> -		ret += ipchain__fprintf_graph(fp, &rem_hits, depth,
> +		ret += ipchain__fprintf_graph(fp, &rem_node, &rem_hits, depth,
>  					      new_depth_mask, 0, total_samples,
> -					      remaining, left_margin);
> +					      left_margin);
>  	}
>  
>  	return ret;
> @@ -243,12 +244,11 @@ static size_t callchain__fprintf_flat(FILE *fp, struct rb_root *tree,
>  	struct rb_node *rb_node = rb_first(tree);
>  
>  	while (rb_node) {
> -		double percent;
> -
>  		chain = rb_entry(rb_node, struct callchain_node, rb_node);
> -		percent = chain->hit * 100.0 / total_samples;
>  
> -		ret = percent_color_fprintf(fp, "           %6.2f%%\n", percent);
> +		ret += fprintf(fp, "           ");
> +		ret += callchain_node__fprintf_value(chain, fp, total_samples);
> +		ret += fprintf(fp, "\n");
>  		ret += __callchain__fprintf_flat(fp, chain, total_samples);
>  		ret += fprintf(fp, "\n");
>  		if (++entries_printed == callchain_param.print_limit)
> @@ -295,12 +295,11 @@ static size_t callchain__fprintf_folded(FILE *fp, struct rb_root *tree,
>  	struct rb_node *rb_node = rb_first(tree);
>  
>  	while (rb_node) {
> -		double percent;
>  
>  		chain = rb_entry(rb_node, struct callchain_node, rb_node);
> -		percent = chain->hit * 100.0 / total_samples;
>  
> -		ret += fprintf(fp, "%.2f%% ", percent);
> +		ret += callchain_node__fprintf_value(chain, fp, total_samples);
> +		ret += fprintf(fp, " ");
>  		ret += __callchain__fprintf_folded(fp, chain);
>  		ret += fprintf(fp, "\n");
>  		if (++entries_printed == callchain_param.print_limit)
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index 08cb220ba5ea..e2ef9b38acb6 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -805,6 +805,35 @@ char *callchain_list__sym_name(struct callchain_list *cl,
>  	return bf;
>  }
>  
> +char *callchain_node__sprintf_value(struct callchain_node *node,
> +				    char *bf, size_t bfsize, u64 total)

 sprintf doesn't require a bfsize, snprintf does, but we don't use that,
so renaming it to callchain_node__scnprintf_value() so that we recall
the semantic associated with this operation.

> +{
> +	double percent = 0.0;
> +	u64 period = callchain_cumul_hits(node);
> +
> +	if (callchain_param.mode == CHAIN_FOLDED)
> +		period = node->hit;
> +	if (total)
> +		percent = period * 100.0 / total;
> +
> +	scnprintf(bf, bfsize, "%.2f%%", percent);
> +	return bf;
> +}
> +
> +int callchain_node__fprintf_value(struct callchain_node *node,
> +				 FILE *fp, u64 total)
> +{
> +	double percent = 0.0;
> +	u64 period = callchain_cumul_hits(node);
> +
> +	if (callchain_param.mode == CHAIN_FOLDED)
> +		period = node->hit;
> +	if (total)
> +		percent = period * 100.0 / total;
> +
> +	return percent_color_fprintf(fp, "%.2f%%", percent);
> +}
> +
>  static void free_callchain_node(struct callchain_node *node)
>  {
>  	struct callchain_list *list, *tmp;
> diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
> index 544d99ac169c..f9e00e3d1243 100644
> --- a/tools/perf/util/callchain.h
> +++ b/tools/perf/util/callchain.h
> @@ -230,6 +230,10 @@ static inline int arch_skip_callchain_idx(struct thread *thread __maybe_unused,
>  
>  char *callchain_list__sym_name(struct callchain_list *cl,
>  			       char *bf, size_t bfsize, bool show_dso);
> +char *callchain_node__sprintf_value(struct callchain_node *node,
> +				    char *bf, size_t bfsize, u64 total);
> +int callchain_node__fprintf_value(struct callchain_node *node,
> +				  FILE *fp, u64 total);
>  
>  void free_callchain(struct callchain_root *root);
>  
> -- 
> 2.6.2