[PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Namhyung Kim]

Hello,

This patchset tries to implement percent limit to callchains which was
requested by Andi Kleen.  For some reason, limiting callchains by
(overhead) percentage didn't work well.  This patch fixes it and make
--percent-limit also works for callchains as well as hist entries.

 * Changes from v1)
  - fix insertion path instead of changing all UI code
  - show percent value even on single path (if needed)
  - change default callchain percent limit
  
This is available on 'perf/callchain-limit-v2' branch in my tree:

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Any comments are welcome,

Thanks,
Namhyung
*** BLURB HERE ***

Namhyung Kim (10):
  perf hists: Fix min callchain hits calculation
  perf hists: Update hists' total period when adding entries
  perf report: Apply --percent-limit to callchains also
  perf report: Get rid of hist_entry__callchain_fprintf()
  perf tools: Pass parent_samples to __callchain__fprintf_graph()
  perf report: Fix percent display in callchains on --stdio
  perf hists browser: Fix dump to show correct callchain style
  perf hists browser: Pass parent_total to callchain print functions
  perf hists browser: Fix percent display in callchains
  perf tools: Change default calchain percent limit to 0.005%

 tools/perf/Documentation/perf-report.txt |   2 +-
 tools/perf/builtin-report.c              |  11 ++-
 tools/perf/builtin-top.c                 |   2 +-
 tools/perf/ui/browsers/hists.c           | 113 +++++++++++++++++++------------
 tools/perf/ui/stdio/hist.c               |  72 ++++++++++----------
 tools/perf/util/hist.c                   |  24 +++++--
 tools/perf/util/util.c                   |   2 +-
 7 files changed, 136 insertions(+), 90 deletions(-)

-- 
2.6.4

[PATCH 01/10] perf hists: Fix min callchain hits calculation

[Namhyung Kim]

The total period should be get using hists__total_period() since it
takes filtered entries into account.  In addition, if callchain mode is
'fractal', the total period should be the entry's period.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 81ce0aff69d1..b96194676c91 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1163,9 +1163,18 @@ static void __hists__insert_output_entry(struct rb_root *entries,
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter;
 
-	if (use_callchain)
+	if (use_callchain) {
+		if (callchain_param.mode == CHAIN_GRAPH_REL) {
+			u64 total = he->stat.period;
+
+			if (symbol_conf.cumulate_callchain)
+				total = he->stat_acc->period;
+
+			min_callchain_hits = total * (callchain_param.min_percent / 100);
+		}
 		callchain_param.sort(&he->sorted_chain, he->callchain,
 				      min_callchain_hits, &callchain_param);
+	}
 
 	while (*p != NULL) {
 		parent = *p;
@@ -1195,7 +1204,7 @@ void hists__output_resort(struct hists *hists, struct ui_progress *prog)
 	else
 		use_callchain = symbol_conf.use_callchain;
 
-	min_callchain_hits = hists->stats.total_period * (callchain_param.min_percent / 100);
+	min_callchain_hits = hists__total_period(hists) * (callchain_param.min_percent / 100);
 
 	if (sort__need_collapse)
 		root = &hists->entries_collapsed;
-- 
2.6.4

[tip:perf/core] perf hists: Fix min callchain hits calculation

[tip-bot for Namhyung Kim]

Commit-ID:  744070e0e4ac691bb43608f7bf46a9641a9cf342
Gitweb:     http://git.kernel.org/tip/744070e0e4ac691bb43608f7bf46a9641a9cf342
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:48 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 16:42:31 -0300

perf hists: Fix min callchain hits calculation

The total period should be get using hists__total_period() since it
takes filtered entries into account.  In addition, if callchain mode is
'fractal', the total period should be the entry's period.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 81ce0af..b961946 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1163,9 +1163,18 @@ static void __hists__insert_output_entry(struct rb_root *entries,
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter;
 
-	if (use_callchain)
+	if (use_callchain) {
+		if (callchain_param.mode == CHAIN_GRAPH_REL) {
+			u64 total = he->stat.period;
+
+			if (symbol_conf.cumulate_callchain)
+				total = he->stat_acc->period;
+
+			min_callchain_hits = total * (callchain_param.min_percent / 100);
+		}
 		callchain_param.sort(&he->sorted_chain, he->callchain,
 				      min_callchain_hits, &callchain_param);
+	}
 
 	while (*p != NULL) {
 		parent = *p;
@@ -1195,7 +1204,7 @@ void hists__output_resort(struct hists *hists, struct ui_progress *prog)
 	else
 		use_callchain = symbol_conf.use_callchain;
 
-	min_callchain_hits = hists->stats.total_period * (callchain_param.min_percent / 100);
+	min_callchain_hits = hists__total_period(hists) * (callchain_param.min_percent / 100);
 
 	if (sort__need_collapse)
 		root = &hists->entries_collapsed;

[PATCH 02/10] perf hists: Update hists' total period when adding entries

[Namhyung Kim]

Currently the hist entry addition path doesn't update total_period of
hists and it's calculated during 'resort' path.  But the resort path
needs to know the total period before doing its job because it's used
for calculating percent limit of callchains in hist entries.

So this patch update the total period during the addition path.  It
makes the percent limit of callchains working (again).

Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b96194676c91..098310bc4489 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -432,8 +432,12 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 		cmp = hist_entry__cmp(he, entry);
 
 		if (!cmp) {
-			if (sample_self)
+			if (sample_self) {
 				he_stat__add_period(&he->stat, period, weight);
+				hists->stats.total_period += period;
+				if (!he->filtered)
+					hists->stats.total_non_filtered_period += period;
+			}
 			if (symbol_conf.cumulate_callchain)
 				he_stat__add_period(he->stat_acc, period, weight);
 
@@ -466,7 +470,10 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 	if (!he)
 		return NULL;
 
-	hists->nr_entries++;
+	if (sample_self)
+		hists__inc_stats(hists, he);
+	else
+		hists->nr_entries++;
 
 	rb_link_node(&he->rb_node_in, parent, p);
 	rb_insert_color(&he->rb_node_in, hists->entries_in);
-- 
2.6.4

[tip:perf/core] perf hists: Update hists' total period when adding entries

[tip-bot for Namhyung Kim]

Commit-ID:  0f58474ec835f6fc80af2cde2c7ed5495cd212ba
Gitweb:     http://git.kernel.org/tip/0f58474ec835f6fc80af2cde2c7ed5495cd212ba
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:49 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 16:45:44 -0300

perf hists: Update hists' total period when adding entries

Currently the hist entry addition path doesn't update total_period of
hists and it's calculated during 'resort' path.  But the resort path
needs to know the total period before doing its job because it's used
for calculating percent limit of callchains in hist entries.

So this patch update the total period during the addition path.  It
makes the percent limit of callchains working (again).

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b961946..098310b 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -432,8 +432,12 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 		cmp = hist_entry__cmp(he, entry);
 
 		if (!cmp) {
-			if (sample_self)
+			if (sample_self) {
 				he_stat__add_period(&he->stat, period, weight);
+				hists->stats.total_period += period;
+				if (!he->filtered)
+					hists->stats.total_non_filtered_period += period;
+			}
 			if (symbol_conf.cumulate_callchain)
 				he_stat__add_period(he->stat_acc, period, weight);
 
@@ -466,7 +470,10 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 	if (!he)
 		return NULL;
 
-	hists->nr_entries++;
+	if (sample_self)
+		hists__inc_stats(hists, he);
+	else
+		hists->nr_entries++;
 
 	rb_link_node(&he->rb_node_in, parent, p);
 	rb_insert_color(&he->rb_node_in, hists->entries_in);

[PATCH 03/10] perf report: Apply --percent-limit to callchains also

[Namhyung Kim]

Currently --percent-limit option only works for hist entries.  However
it'd be better to have same effect to callchains as well

Requested-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2bf537f190a0..72ed0b46d5a1 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -75,7 +75,10 @@ static int report__config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (!strcmp(var, "report.percent-limit")) {
-		rep->min_percent = strtof(value, NULL);
+		double pcnt = strtof(value, NULL);
+
+		rep->min_percent = pcnt;
+		callchain_param.min_percent = pcnt;
 		return 0;
 	}
 	if (!strcmp(var, "report.children")) {
@@ -633,8 +636,10 @@ parse_percent_limit(const struct option *opt, const char *str,
 		    int unset __maybe_unused)
 {
 	struct report *rep = opt->value;
+	double pcnt = strtof(str, NULL);
 
-	rep->min_percent = strtof(str, NULL);
+	rep->min_percent = pcnt;
+	callchain_param.min_percent = pcnt;
 	return 0;
 }
 
-- 
2.6.4

Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also

[Arnaldo Carvalho de Melo]

Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> Currently --percent-limit option only works for hist entries.  However
> it'd be better to have same effect to callchains as well

Documentation needs updating? It says:

--percent-limit::
        Do not show entries which have an overhead under that percent.
        (Default: 0).

 
> Requested-by: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/builtin-report.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 2bf537f190a0..72ed0b46d5a1 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -75,7 +75,10 @@ static int report__config(const char *var, const char *value, void *cb)
>  		return 0;
>  	}
>  	if (!strcmp(var, "report.percent-limit")) {
> -		rep->min_percent = strtof(value, NULL);
> +		double pcnt = strtof(value, NULL);
> +
> +		rep->min_percent = pcnt;
> +		callchain_param.min_percent = pcnt;
>  		return 0;
>  	}
>  	if (!strcmp(var, "report.children")) {
> @@ -633,8 +636,10 @@ parse_percent_limit(const struct option *opt, const char *str,
>  		    int unset __maybe_unused)
>  {
>  	struct report *rep = opt->value;
> +	double pcnt = strtof(str, NULL);
>  
> -	rep->min_percent = strtof(str, NULL);
> +	rep->min_percent = pcnt;
> +	callchain_param.min_percent = pcnt;
>  	return 0;
>  }
>  
> -- 
> 2.6.4

Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also

[Namhyung Kim]

Hi Arnaldo,

On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > Currently --percent-limit option only works for hist entries.  However
> > it'd be better to have same effect to callchains as well
> 
> Documentation needs updating? It says:
> 
> --percent-limit::
>         Do not show entries which have an overhead under that percent.
>         (Default: 0).

Right.  Is it ok to you?

--percent-limit::
        Do not show entries and callchains which have an overhead under that
        percent.  (Default: 0).


Thanks,
Namhyung


> 
>  
> > Requested-by: Andi Kleen <andi@firstfloor.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/builtin-report.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index 2bf537f190a0..72ed0b46d5a1 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -75,7 +75,10 @@ static int report__config(const char *var, const char *value, void *cb)
> >  		return 0;
> >  	}
> >  	if (!strcmp(var, "report.percent-limit")) {
> > -		rep->min_percent = strtof(value, NULL);
> > +		double pcnt = strtof(value, NULL);
> > +
> > +		rep->min_percent = pcnt;
> > +		callchain_param.min_percent = pcnt;
> >  		return 0;
> >  	}
> >  	if (!strcmp(var, "report.children")) {
> > @@ -633,8 +636,10 @@ parse_percent_limit(const struct option *opt, const char *str,
> >  		    int unset __maybe_unused)
> >  {
> >  	struct report *rep = opt->value;
> > +	double pcnt = strtof(str, NULL);
> >  
> > -	rep->min_percent = strtof(str, NULL);
> > +	rep->min_percent = pcnt;
> > +	callchain_param.min_percent = pcnt;
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.6.4

Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also

[Arnaldo Carvalho de Melo]

Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> Hi Arnaldo,
> 
> On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > Currently --percent-limit option only works for hist entries.  However
> > > it'd be better to have same effect to callchains as well
> > 
> > Documentation needs updating? It says:
> > 
> > --percent-limit::
> >         Do not show entries which have an overhead under that percent.
> >         (Default: 0).
> 
> Right.  Is it ok to you?
> 
> --percent-limit::
>         Do not show entries and callchains which have an overhead under that
>         percent.  (Default: 0).

Ok, but is the default zero?

That was what I was alluding to, as as soon as I applied the patch that
made callchains honour this limit, about 60% of the entries in the
particular perf.data file I was 'perf report'ing lost its '+' (callchain
expansion) signs.

- Arnaldo

> 
> 
> Thanks,
> Namhyung
> 
> 
> > 
> >  
> > > Requested-by: Andi Kleen <andi@firstfloor.org>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > >  tools/perf/builtin-report.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > > index 2bf537f190a0..72ed0b46d5a1 100644
> > > --- a/tools/perf/builtin-report.c
> > > +++ b/tools/perf/builtin-report.c
> > > @@ -75,7 +75,10 @@ static int report__config(const char *var, const char *value, void *cb)
> > >  		return 0;
> > >  	}
> > >  	if (!strcmp(var, "report.percent-limit")) {
> > > -		rep->min_percent = strtof(value, NULL);
> > > +		double pcnt = strtof(value, NULL);
> > > +
> > > +		rep->min_percent = pcnt;
> > > +		callchain_param.min_percent = pcnt;
> > >  		return 0;
> > >  	}
> > >  	if (!strcmp(var, "report.children")) {
> > > @@ -633,8 +636,10 @@ parse_percent_limit(const struct option *opt, const char *str,
> > >  		    int unset __maybe_unused)
> > >  {
> > >  	struct report *rep = opt->value;
> > > +	double pcnt = strtof(str, NULL);
> > >  
> > > -	rep->min_percent = strtof(str, NULL);
> > > +	rep->min_percent = pcnt;
> > > +	callchain_param.min_percent = pcnt;
> > >  	return 0;
> > >  }
> > >  
> > > -- 
> > > 2.6.4

Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also

[Namhyung Kim]

On Tue, Feb 02, 2016 at 10:55:34AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> > Hi Arnaldo,
> > 
> > On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > > Currently --percent-limit option only works for hist entries.  However
> > > > it'd be better to have same effect to callchains as well
> > > 
> > > Documentation needs updating? It says:
> > > 
> > > --percent-limit::
> > >         Do not show entries which have an overhead under that percent.
> > >         (Default: 0).
> > 
> > Right.  Is it ok to you?
> > 
> > --percent-limit::
> >         Do not show entries and callchains which have an overhead under that
> >         percent.  (Default: 0).
> 
> Ok, but is the default zero?
> 
> That was what I was alluding to, as as soon as I applied the patch that
> made callchains honour this limit, about 60% of the entries in the
> particular perf.data file I was 'perf report'ing lost its '+' (callchain
> expansion) signs.

Yes, and this is what I want to say too. :)

The default value of percent limit is different for hist entry and
callchains.  For hist entry the default is 0, and for callchains it's
0.5%.  But using --percent-limit option, we can set both at once from
now on.

Before this patchset, percent limit of callchains didn't checked.
Once applied, users can see callchains are disppeared like your case.
 This is just because 0.5% of the default limit for callchains is too
high IMHO - I guess it was originally set for 'fractal' mode which
calculates relative percents.  So I proposed to change the default in
the patch 10/10 but Andi thoughts 0.005% was too small.  We need to
choose other value like 0.05% ?

Thanks,
Namhyung

Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also

[Arnaldo Carvalho de Melo]

Em Tue, Feb 02, 2016 at 11:15:35PM +0900, Namhyung Kim escreveu:
> On Tue, Feb 02, 2016 at 10:55:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> > > Hi Arnaldo,
> > > 
> > > On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > > > Currently --percent-limit option only works for hist entries.  However
> > > > > it'd be better to have same effect to callchains as well
> > > > 
> > > > Documentation needs updating? It says:
> > > > 
> > > > --percent-limit::
> > > >         Do not show entries which have an overhead under that percent.
> > > >         (Default: 0).
> > > 
> > > Right.  Is it ok to you?
> > > 
> > > --percent-limit::
> > >         Do not show entries and callchains which have an overhead under that
> > >         percent.  (Default: 0).
> > 
> > Ok, but is the default zero?
> > 
> > That was what I was alluding to, as as soon as I applied the patch that
> > made callchains honour this limit, about 60% of the entries in the
> > particular perf.data file I was 'perf report'ing lost its '+' (callchain
> > expansion) signs.
> 
> Yes, and this is what I want to say too. :)
> 
> The default value of percent limit is different for hist entry and
> callchains.  For hist entry the default is 0, and for callchains it's
> 0.5%.  But using --percent-limit option, we can set both at once from
> now on.
> 
> Before this patchset, percent limit of callchains didn't checked.
> Once applied, users can see callchains are disppeared like your case.
>  This is just because 0.5% of the default limit for callchains is too
> high IMHO - I guess it was originally set for 'fractal' mode which
> calculates relative percents.  So I proposed to change the default in
> the patch 10/10 but Andi thoughts 0.005% was too small.  We need to
> choose other value like 0.05% ?

Unsure about the limit, but please fold the nice explanation you gave
about the limits for hist entries and callchains and how the defaults
are different, and how they can be set at once using --percent-limit.

Also, would it be overengineering to allow optionally set two limits,
one for hists and one for callchains? I.e. like:

 --percent-limit 1,0.07

Then:

  --percent-limit 0.8

would be the same as:

  --percent-limit 0.8,0.8

- Arnaldo

Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also

[Namhyung Kim]

On Tue, Feb 02, 2016 at 11:27:34AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 02, 2016 at 11:15:35PM +0900, Namhyung Kim escreveu:
> > On Tue, Feb 02, 2016 at 10:55:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> > > > Hi Arnaldo,
> > > > 
> > > > On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > > > > Currently --percent-limit option only works for hist entries.  However
> > > > > > it'd be better to have same effect to callchains as well
> > > > > 
> > > > > Documentation needs updating? It says:
> > > > > 
> > > > > --percent-limit::
> > > > >         Do not show entries which have an overhead under that percent.
> > > > >         (Default: 0).
> > > > 
> > > > Right.  Is it ok to you?
> > > > 
> > > > --percent-limit::
> > > >         Do not show entries and callchains which have an overhead under that
> > > >         percent.  (Default: 0).
> > > 
> > > Ok, but is the default zero?
> > > 
> > > That was what I was alluding to, as as soon as I applied the patch that
> > > made callchains honour this limit, about 60% of the entries in the
> > > particular perf.data file I was 'perf report'ing lost its '+' (callchain
> > > expansion) signs.
> > 
> > Yes, and this is what I want to say too. :)
> > 
> > The default value of percent limit is different for hist entry and
> > callchains.  For hist entry the default is 0, and for callchains it's
> > 0.5%.  But using --percent-limit option, we can set both at once from
> > now on.
> > 
> > Before this patchset, percent limit of callchains didn't checked.
> > Once applied, users can see callchains are disppeared like your case.
> >  This is just because 0.5% of the default limit for callchains is too
> > high IMHO - I guess it was originally set for 'fractal' mode which
> > calculates relative percents.  So I proposed to change the default in
> > the patch 10/10 but Andi thoughts 0.005% was too small.  We need to
> > choose other value like 0.05% ?
> 
> Unsure about the limit, but please fold the nice explanation you gave
> about the limits for hist entries and callchains and how the defaults
> are different, and how they can be set at once using --percent-limit.

Ok, will do.

> 
> Also, would it be overengineering to allow optionally set two limits,
> one for hists and one for callchains? I.e. like:
> 
>  --percent-limit 1,0.07
> 
> Then:
> 
>   --percent-limit 0.8
> 
> would be the same as:
> 
>   --percent-limit 0.8,0.8

Originally, the percent limit (threshold) for callchains can be set
using -g/--call-graph option.  What Andi asked is to set both limits
using a single option.  Not sure it's worth adding another way to
specify the callchain limit.

Thanks,
Namhyung

Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also

[Arnaldo Carvalho de Melo]

Em Tue, Feb 02, 2016 at 11:35:13PM +0900, Namhyung Kim escreveu:
> On Tue, Feb 02, 2016 at 11:27:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Feb 02, 2016 at 11:15:35PM +0900, Namhyung Kim escreveu:
> > > On Tue, Feb 02, 2016 at 10:55:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> > > > > Hi Arnaldo,
> > > > > 
> > > > > On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > > > > > Currently --percent-limit option only works for hist entries.  However
> > > > > > > it'd be better to have same effect to callchains as well
> > > > > > 
> > > > > > Documentation needs updating? It says:
> > > > > > 
> > > > > > --percent-limit::
> > > > > >         Do not show entries which have an overhead under that percent.
> > > > > >         (Default: 0).
> > > > > 
> > > > > Right.  Is it ok to you?
> > > > > 
> > > > > --percent-limit::
> > > > >         Do not show entries and callchains which have an overhead under that
> > > > >         percent.  (Default: 0).
> > > > 
> > > > Ok, but is the default zero?
> > > > 
> > > > That was what I was alluding to, as as soon as I applied the patch that
> > > > made callchains honour this limit, about 60% of the entries in the
> > > > particular perf.data file I was 'perf report'ing lost its '+' (callchain
> > > > expansion) signs.
> > > 
> > > Yes, and this is what I want to say too. :)
> > > 
> > > The default value of percent limit is different for hist entry and
> > > callchains.  For hist entry the default is 0, and for callchains it's
> > > 0.5%.  But using --percent-limit option, we can set both at once from
> > > now on.
> > > 
> > > Before this patchset, percent limit of callchains didn't checked.
> > > Once applied, users can see callchains are disppeared like your case.
> > >  This is just because 0.5% of the default limit for callchains is too
> > > high IMHO - I guess it was originally set for 'fractal' mode which
> > > calculates relative percents.  So I proposed to change the default in
> > > the patch 10/10 but Andi thoughts 0.005% was too small.  We need to
> > > choose other value like 0.05% ?
> > 
> > Unsure about the limit, but please fold the nice explanation you gave
> > about the limits for hist entries and callchains and how the defaults
> > are different, and how they can be set at once using --percent-limit.
> 
> Ok, will do.
> 
> > 
> > Also, would it be overengineering to allow optionally set two limits,
> > one for hists and one for callchains? I.e. like:
> > 
> >  --percent-limit 1,0.07
> > 
> > Then:
> > 
> >   --percent-limit 0.8
> > 
> > would be the same as:
> > 
> >   --percent-limit 0.8,0.8
> 
> Originally, the percent limit (threshold) for callchains can be set
> using -g/--call-graph option.  What Andi asked is to set both limits
> using a single option.  Not sure it's worth adding another way to
> specify the callchain limit.

Ok, having that in the docs, i.e. when documenting --percent-limit
mention that the callchain limit can be set via the -g option as well
will help sort out confusion when trying to set those limits.

And have you considered setting these limits dynamicly? I.e. in the TUI?

- Arnaldo

Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains also

[Namhyung Kim]

On Tue, Feb 02, 2016 at 11:57:54AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 02, 2016 at 11:35:13PM +0900, Namhyung Kim escreveu:
> > On Tue, Feb 02, 2016 at 11:27:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Feb 02, 2016 at 11:15:35PM +0900, Namhyung Kim escreveu:
> > > > On Tue, Feb 02, 2016 at 10:55:34AM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > Em Tue, Feb 02, 2016 at 10:05:37PM +0900, Namhyung Kim escreveu:
> > > > > > Hi Arnaldo,
> > > > > > 
> > > > > > On Mon, Feb 01, 2016 at 05:19:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > > > Em Thu, Jan 28, 2016 at 12:40:50AM +0900, Namhyung Kim escreveu:
> > > > > > > > Currently --percent-limit option only works for hist entries.  However
> > > > > > > > it'd be better to have same effect to callchains as well
> > > > > > > 
> > > > > > > Documentation needs updating? It says:
> > > > > > > 
> > > > > > > --percent-limit::
> > > > > > >         Do not show entries which have an overhead under that percent.
> > > > > > >         (Default: 0).
> > > > > > 
> > > > > > Right.  Is it ok to you?
> > > > > > 
> > > > > > --percent-limit::
> > > > > >         Do not show entries and callchains which have an overhead under that
> > > > > >         percent.  (Default: 0).
> > > > > 
> > > > > Ok, but is the default zero?
> > > > > 
> > > > > That was what I was alluding to, as as soon as I applied the patch that
> > > > > made callchains honour this limit, about 60% of the entries in the
> > > > > particular perf.data file I was 'perf report'ing lost its '+' (callchain
> > > > > expansion) signs.
> > > > 
> > > > Yes, and this is what I want to say too. :)
> > > > 
> > > > The default value of percent limit is different for hist entry and
> > > > callchains.  For hist entry the default is 0, and for callchains it's
> > > > 0.5%.  But using --percent-limit option, we can set both at once from
> > > > now on.
> > > > 
> > > > Before this patchset, percent limit of callchains didn't checked.
> > > > Once applied, users can see callchains are disppeared like your case.
> > > >  This is just because 0.5% of the default limit for callchains is too
> > > > high IMHO - I guess it was originally set for 'fractal' mode which
> > > > calculates relative percents.  So I proposed to change the default in
> > > > the patch 10/10 but Andi thoughts 0.005% was too small.  We need to
> > > > choose other value like 0.05% ?
> > > 
> > > Unsure about the limit, but please fold the nice explanation you gave
> > > about the limits for hist entries and callchains and how the defaults
> > > are different, and how they can be set at once using --percent-limit.
> > 
> > Ok, will do.
> > 
> > > 
> > > Also, would it be overengineering to allow optionally set two limits,
> > > one for hists and one for callchains? I.e. like:
> > > 
> > >  --percent-limit 1,0.07
> > > 
> > > Then:
> > > 
> > >   --percent-limit 0.8
> > > 
> > > would be the same as:
> > > 
> > >   --percent-limit 0.8,0.8
> > 
> > Originally, the percent limit (threshold) for callchains can be set
> > using -g/--call-graph option.  What Andi asked is to set both limits
> > using a single option.  Not sure it's worth adding another way to
> > specify the callchain limit.
> 
> Ok, having that in the docs, i.e. when documenting --percent-limit
> mention that the callchain limit can be set via the -g option as well
> will help sort out confusion when trying to set those limits.

OK.  How about this?

--percent-limit::
	Do not show entries which have an overhead under that percent.
	(Default: 0).  Note that this option also sets percent limit (threshold)
	of callchains at once.  However the default value of callchain threshold
	is different than the default value of hist entries.  Please see
	--call-graph option for details.


> 
> And have you considered setting these limits dynamicly? I.e. in the TUI?

Oh, I forgot about it.  Will try to implement.

Thanks,
Namhyung

[tip:perf/core] perf report: Apply --percent-limit to callchains also

[tip-bot for Namhyung Kim]

Commit-ID:  2665b4528d0522ef073c2bde33cf9a7bd7391164
Gitweb:     http://git.kernel.org/tip/2665b4528d0522ef073c2bde33cf9a7bd7391164
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:50 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:12:00 -0300

perf report: Apply --percent-limit to callchains also

Currently --percent-limit option only works for hist entries.  However
it'd be better to have same effect to callchains as well

Requested-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2bf537f..72ed0b4 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -75,7 +75,10 @@ static int report__config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 	if (!strcmp(var, "report.percent-limit")) {
-		rep->min_percent = strtof(value, NULL);
+		double pcnt = strtof(value, NULL);
+
+		rep->min_percent = pcnt;
+		callchain_param.min_percent = pcnt;
 		return 0;
 	}
 	if (!strcmp(var, "report.children")) {
@@ -633,8 +636,10 @@ parse_percent_limit(const struct option *opt, const char *str,
 		    int unset __maybe_unused)
 {
 	struct report *rep = opt->value;
+	double pcnt = strtof(str, NULL);
 
-	rep->min_percent = strtof(str, NULL);
+	rep->min_percent = pcnt;
+	callchain_param.min_percent = pcnt;
 	return 0;
 }
 

[PATCH 04/10] perf report: Get rid of hist_entry__callchain_fprintf()

[Namhyung Kim]

It's just a wrapper function to align the start position ofcallchains to
'comm' of each thread if it's a first sort key.  But it doesn't not work
with tracepoint events and also with upcoming hierarchy view.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/stdio/hist.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 387110d50b00..8e25f7dd6e84 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -349,30 +349,6 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 	return 0;
 }
 
-static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
-					    struct hists *hists,
-					    FILE *fp)
-{
-	int left_margin = 0;
-	u64 total_period = hists->stats.total_period;
-
-	if (field_order == NULL && (sort_order == NULL ||
-				    !prefixcmp(sort_order, "comm"))) {
-		struct perf_hpp_fmt *fmt;
-
-		perf_hpp__for_each_format(fmt) {
-			if (!perf_hpp__is_sort_entry(fmt))
-				continue;
-
-			/* must be 'comm' sort entry */
-			left_margin = fmt->width(fmt, NULL, hists_to_evsel(hists));
-			left_margin -= thread__comm_len(he->thread);
-			break;
-		}
-	}
-	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
-}
-
 static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp)
 {
 	const char *sep = symbol_conf.field_sep;
@@ -418,6 +394,7 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 		.buf		= bf,
 		.size		= size,
 	};
+	u64 total_period = hists->stats.total_period;
 
 	if (size == 0 || size > bfsz)
 		size = hpp.size = bfsz;
@@ -427,7 +404,7 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	ret = fprintf(fp, "%s\n", bf);
 
 	if (symbol_conf.use_callchain)
-		ret += hist_entry__callchain_fprintf(he, hists, fp);
+		ret += hist_entry_callchain__fprintf(he, total_period, 0, fp);
 
 	return ret;
 }
-- 
2.6.4

[tip:perf/core] perf report: Get rid of hist_entry__callchain_fprintf()

[tip-bot for Namhyung Kim]

Commit-ID:  7e597d327eca3d92a759542ff707cba61af3a718
Gitweb:     http://git.kernel.org/tip/7e597d327eca3d92a759542ff707cba61af3a718
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:51 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:20:31 -0300

perf report: Get rid of hist_entry__callchain_fprintf()

It's just a wrapper function to align the start position ofcallchains to
'comm' of each thread if it's a first sort key.  But it doesn't not work
with tracepoint events and also with upcoming hierarchy view.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/stdio/hist.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 387110d..8e25f7d 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -349,30 +349,6 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 	return 0;
 }
 
-static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
-					    struct hists *hists,
-					    FILE *fp)
-{
-	int left_margin = 0;
-	u64 total_period = hists->stats.total_period;
-
-	if (field_order == NULL && (sort_order == NULL ||
-				    !prefixcmp(sort_order, "comm"))) {
-		struct perf_hpp_fmt *fmt;
-
-		perf_hpp__for_each_format(fmt) {
-			if (!perf_hpp__is_sort_entry(fmt))
-				continue;
-
-			/* must be 'comm' sort entry */
-			left_margin = fmt->width(fmt, NULL, hists_to_evsel(hists));
-			left_margin -= thread__comm_len(he->thread);
-			break;
-		}
-	}
-	return hist_entry_callchain__fprintf(he, total_period, left_margin, fp);
-}
-
 static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp)
 {
 	const char *sep = symbol_conf.field_sep;
@@ -418,6 +394,7 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 		.buf		= bf,
 		.size		= size,
 	};
+	u64 total_period = hists->stats.total_period;
 
 	if (size == 0 || size > bfsz)
 		size = hpp.size = bfsz;
@@ -427,7 +404,7 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	ret = fprintf(fp, "%s\n", bf);
 
 	if (symbol_conf.use_callchain)
-		ret += hist_entry__callchain_fprintf(he, hists, fp);
+		ret += hist_entry_callchain__fprintf(he, total_period, 0, fp);
 
 	return ret;
 }

[PATCH 05/10] perf tools: Pass parent_samples to __callchain__fprintf_graph()

[Namhyung Kim]

Pass hist entry's period to graph callchain print function.  This info
is needed by later patch to determine whether it can omit percentage of
top-level node or not.

No functional change intended.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/stdio/hist.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 8e25f7dd6e84..96188ea12771 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -166,7 +166,8 @@ static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 }
 
 static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
-				       u64 total_samples, int left_margin)
+				       u64 total_samples, u64 parent_samples,
+				       int left_margin)
 {
 	struct callchain_node *cnode;
 	struct callchain_list *chain;
@@ -213,6 +214,9 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 		root = &cnode->rb_root;
 	}
 
+	if (callchain_param.mode == CHAIN_GRAPH_REL)
+		total_samples = parent_samples;
+
 	ret += __callchain__fprintf_graph(fp, root, total_samples,
 					  1, 1, left_margin);
 	ret += fprintf(fp, "\n");
@@ -323,16 +327,19 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 					    u64 total_samples, int left_margin,
 					    FILE *fp)
 {
+	u64 parent_samples = he->stat.period;
+
+	if (symbol_conf.cumulate_callchain)
+		parent_samples = he->stat_acc->period;
+
 	switch (callchain_param.mode) {
 	case CHAIN_GRAPH_REL:
-		return callchain__fprintf_graph(fp, &he->sorted_chain,
-						symbol_conf.cumulate_callchain ?
-						he->stat_acc->period : he->stat.period,
-						left_margin);
+		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
+						parent_samples, left_margin);
 		break;
 	case CHAIN_GRAPH_ABS:
 		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
-						left_margin);
+						parent_samples, left_margin);
 		break;
 	case CHAIN_FLAT:
 		return callchain__fprintf_flat(fp, &he->sorted_chain, total_samples);
-- 
2.6.4

[tip:perf/core] perf callchain: Pass parent_samples to __callchain__fprintf_graph()

[tip-bot for Namhyung Kim]

Commit-ID:  54d27b3119e2eecbb3dfbf821db90fab25f6c523
Gitweb:     http://git.kernel.org/tip/54d27b3119e2eecbb3dfbf821db90fab25f6c523
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:52 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:39:52 -0300

perf callchain: Pass parent_samples to __callchain__fprintf_graph()

Pass hist entry's period to graph callchain print function.  This info
is needed by later patch to determine whether it can omit percentage of
top-level node or not.

No functional change intended.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-6-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/stdio/hist.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 8e25f7d..96188ea 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -166,7 +166,8 @@ static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 }
 
 static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
-				       u64 total_samples, int left_margin)
+				       u64 total_samples, u64 parent_samples,
+				       int left_margin)
 {
 	struct callchain_node *cnode;
 	struct callchain_list *chain;
@@ -213,6 +214,9 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 		root = &cnode->rb_root;
 	}
 
+	if (callchain_param.mode == CHAIN_GRAPH_REL)
+		total_samples = parent_samples;
+
 	ret += __callchain__fprintf_graph(fp, root, total_samples,
 					  1, 1, left_margin);
 	ret += fprintf(fp, "\n");
@@ -323,16 +327,19 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
 					    u64 total_samples, int left_margin,
 					    FILE *fp)
 {
+	u64 parent_samples = he->stat.period;
+
+	if (symbol_conf.cumulate_callchain)
+		parent_samples = he->stat_acc->period;
+
 	switch (callchain_param.mode) {
 	case CHAIN_GRAPH_REL:
-		return callchain__fprintf_graph(fp, &he->sorted_chain,
-						symbol_conf.cumulate_callchain ?
-						he->stat_acc->period : he->stat.period,
-						left_margin);
+		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
+						parent_samples, left_margin);
 		break;
 	case CHAIN_GRAPH_ABS:
 		return callchain__fprintf_graph(fp, &he->sorted_chain, total_samples,
-						left_margin);
+						parent_samples, left_margin);
 		break;
 	case CHAIN_FLAT:
 		return callchain__fprintf_flat(fp, &he->sorted_chain, total_samples);

[PATCH 06/10] perf report: Fix percent display in callchains on --stdio

[Namhyung Kim]

When there's only a single callchain, perf doesn't print its percentage
in front of the symbols.  This is because it assumes that the percentage
is same as parents.  But if a percent limit is applied, it's possible
that there are actually a couple of child nodes but only one of them is
shown.  In this case it should display the percent to prevent
misunderstanding of its percentage is same as the parent's.

For example, let's see the following callchain.

  $ perf report -s comm --percent-limit 0.01 --stdio
  ...
     9.95%  swapper
            |
            |--7.57%--intel_idle
            |          cpuidle_enter_state
            |          cpuidle_enter
            |          call_cpuidle
            |          cpu_startup_entry
            |          |
            |          |--4.89%--start_secondary
            |          |
            |           --2.68%--rest_init
            |                     start_kernel
            |                     x86_64_start_reservations
            |                     x86_64_start_kernel
	    |
	    |--0.15%--__schedule
	    |          |
	    |          |--0.13%--schedule
	    |          |          schedule_preempt_disable
	    |          |          cpu_startup_entry
            |          |          |
            |          |          |--0.09%--start_secondary
            |          |          |
            |          |           --0.04%--rest_init
            |          |                     start_kernel
            |          |                     x86_64_start_reservations
            |          |                     x86_64_start_kernel
            |          |
            |           --0.01%--schedule_preempt_disabled
            |                     cpu_startup_entry
  ...

Current code omits the percent if 'intel_idle' becomes the only node
when percent limit is set to 0.5%, its percent is not 9.95% but users
will assume it incorrectly.

Before:

  $ perf report --percent-limit 0.5 --stdio
  ...
     9.95%  swapper
            |
            ---intel_idle
               cpuidle_enter_state
               cpuidle_enter
               call_cpuidle
               cpu_startup_entry
               |
               |--4.89%--start_secondary
               |
                --2.68%--rest_init
                          start_kernel
                          x86_64_start_reservations
                          x86_64_start_kernel

After:

  $ perf report --percent-limit 0.5 --stdio
  ...
     9.95%  swapper
            |
             --7.57%--intel_idle
                       cpuidle_enter_state
                       cpuidle_enter
                       call_cpuidle
                       cpu_startup_entry
                       |
                       |--4.89%--start_secondary
                       |
                        --2.68%--rest_init
                                  start_kernel
                                  x86_64_start_reservations
                                  x86_64_start_kernel

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/stdio/hist.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 96188ea12771..76ff46becac8 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -165,6 +165,25 @@ static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 	return ret;
 }
 
+/*
+ * If have one single callchain root, don't bother printing
+ * its percentage (100 % in fractal mode and the same percentage
+ * than the hist in graph mode). This also avoid one level of column.
+ *
+ * However when percent-limit applied, it's possible that single callchain
+ * node have different (non-100% in fractal mode) percentage.
+ */
+static bool need_percent_display(struct rb_node *node, u64 parent_samples)
+{
+	struct callchain_node *cnode;
+
+	if (rb_next(node))
+		return true;
+
+	cnode = rb_entry(node, struct callchain_node, rb_node);
+	return callchain_cumul_hits(cnode) != parent_samples;
+}
+
 static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 				       u64 total_samples, u64 parent_samples,
 				       int left_margin)
@@ -178,13 +197,8 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 	int ret = 0;
 	char bf[1024];
 
-	/*
-	 * If have one single callchain root, don't bother printing
-	 * its percentage (100 % in fractal mode and the same percentage
-	 * than the hist in graph mode). This also avoid one level of column.
-	 */
 	node = rb_first(root);
-	if (node && !rb_next(node)) {
+	if (node && !need_percent_display(node, parent_samples)) {
 		cnode = rb_entry(node, struct callchain_node, rb_node);
 		list_for_each_entry(chain, &cnode->val, list) {
 			/*
-- 
2.6.4

[tip:perf/core] perf report: Fix percent display in callchains on --stdio

[tip-bot for Namhyung Kim]

Commit-ID:  7ed5d6e28a0a1a54f554b0ab9c38a6061e7cac9e
Gitweb:     http://git.kernel.org/tip/7ed5d6e28a0a1a54f554b0ab9c38a6061e7cac9e
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:53 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:41:32 -0300

perf report: Fix percent display in callchains on --stdio

When there's only a single callchain, perf doesn't print its percentage
in front of the symbols.  This is because it assumes that the percentage
is same as parents.  But if a percent limit is applied, it's possible
that there are actually a couple of child nodes but only one of them is
shown.  In this case it should display the percent to prevent
misunderstanding of its percentage is same as the parent's.

For example, let's see the following callchain.

  $ perf report -s comm --percent-limit 0.01 --stdio
  ...
     9.95%  swapper
            |
            |--7.57%--intel_idle
            |          cpuidle_enter_state
            |          cpuidle_enter
            |          call_cpuidle
            |          cpu_startup_entry
            |          |
            |          |--4.89%--start_secondary
            |          |
            |           --2.68%--rest_init
            |                     start_kernel
            |                     x86_64_start_reservations
            |                     x86_64_start_kernel
	    |
	    |--0.15%--__schedule
	    |          |
	    |          |--0.13%--schedule
	    |          |          schedule_preempt_disable
	    |          |          cpu_startup_entry
            |          |          |
            |          |          |--0.09%--start_secondary
            |          |          |
            |          |           --0.04%--rest_init
            |          |                     start_kernel
            |          |                     x86_64_start_reservations
            |          |                     x86_64_start_kernel
            |          |
            |           --0.01%--schedule_preempt_disabled
            |                     cpu_startup_entry
  ...

Current code omits the percent if 'intel_idle' becomes the only node
when percent limit is set to 0.5%, its percent is not 9.95% but users
will assume it incorrectly.

Before:

  $ perf report --percent-limit 0.5 --stdio
  ...
     9.95%  swapper
            |
            ---intel_idle
               cpuidle_enter_state
               cpuidle_enter
               call_cpuidle
               cpu_startup_entry
               |
               |--4.89%--start_secondary
               |
                --2.68%--rest_init
                          start_kernel
                          x86_64_start_reservations
                          x86_64_start_kernel

After:

  $ perf report --percent-limit 0.5 --stdio
  ...
     9.95%  swapper
            |
             --7.57%--intel_idle
                       cpuidle_enter_state
                       cpuidle_enter
                       call_cpuidle
                       cpu_startup_entry
                       |
                       |--4.89%--start_secondary
                       |
                        --2.68%--rest_init
                                  start_kernel
                                  x86_64_start_reservations
                                  x86_64_start_kernel

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-7-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/stdio/hist.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 96188ea..76ff46b 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -165,6 +165,25 @@ static size_t __callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 	return ret;
 }
 
+/*
+ * If have one single callchain root, don't bother printing
+ * its percentage (100 % in fractal mode and the same percentage
+ * than the hist in graph mode). This also avoid one level of column.
+ *
+ * However when percent-limit applied, it's possible that single callchain
+ * node have different (non-100% in fractal mode) percentage.
+ */
+static bool need_percent_display(struct rb_node *node, u64 parent_samples)
+{
+	struct callchain_node *cnode;
+
+	if (rb_next(node))
+		return true;
+
+	cnode = rb_entry(node, struct callchain_node, rb_node);
+	return callchain_cumul_hits(cnode) != parent_samples;
+}
+
 static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 				       u64 total_samples, u64 parent_samples,
 				       int left_margin)
@@ -178,13 +197,8 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 	int ret = 0;
 	char bf[1024];
 
-	/*
-	 * If have one single callchain root, don't bother printing
-	 * its percentage (100 % in fractal mode and the same percentage
-	 * than the hist in graph mode). This also avoid one level of column.
-	 */
 	node = rb_first(root);
-	if (node && !rb_next(node)) {
+	if (node && !need_percent_display(node, parent_samples)) {
 		cnode = rb_entry(node, struct callchain_node, rb_node);
 		list_for_each_entry(chain, &cnode->val, list) {
 			/*

[PATCH 07/10] perf hists browser: Fix dump to show correct callchain style

[Namhyung Kim]

The commit 8c430a348699 ("perf hists browser: Support folded
callchains") missed to update hist_browser__dump() so it always shows
graph-style callchains regardless of current setting.

To fix that, factor out callchain printing code and rename the existing
function which prints graph-style callchain.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 73 ++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 32 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d07e6be42ab1..91804c5b0130 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -844,7 +844,7 @@ next:
 	return row - first_row;
 }
 
-static int hist_browser__show_callchain(struct hist_browser *browser,
+static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 					struct rb_root *root, int level,
 					unsigned short row, u64 total,
 					print_callchain_entry_fn print,
@@ -898,7 +898,7 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 			else
 				new_total = total;
 
-			row += hist_browser__show_callchain(browser, &child->rb_root,
+			row += hist_browser__show_callchain_graph(browser, &child->rb_root,
 							    new_level, row, new_total,
 							    print, arg, is_output_full);
 		}
@@ -910,6 +910,43 @@ out:
 	return row - first_row;
 }
 
+static int hist_browser__show_callchain(struct hist_browser *browser,
+					struct hist_entry *entry, int level,
+					unsigned short row,
+					print_callchain_entry_fn print,
+					struct callchain_print_arg *arg,
+					check_output_full_fn is_output_full)
+{
+	u64 total = hists__total_period(entry->hists);
+	int printed;
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL) {
+		if (symbol_conf.cumulate_callchain)
+			total = entry->stat_acc->period;
+		else
+			total = entry->stat.period;
+	}
+
+	if (callchain_param.mode == CHAIN_FLAT) {
+		printed = hist_browser__show_callchain_flat(browser,
+						&entry->sorted_chain, row, total,
+						print, arg, is_output_full);
+	} else if (callchain_param.mode == CHAIN_FOLDED) {
+		printed = hist_browser__show_callchain_folded(browser,
+						&entry->sorted_chain, row, total,
+						print, arg, is_output_full);
+	} else {
+		printed = hist_browser__show_callchain_graph(browser,
+						&entry->sorted_chain, level, row, total,
+						print, arg, is_output_full);
+	}
+
+	if (arg->is_current_entry)
+		browser->he_selection = entry;
+
+	return printed;
+}
+
 struct hpp_arg {
 	struct ui_browser *b;
 	char folded_sign;
@@ -1084,38 +1121,14 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 		--row_offset;
 
 	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 (callchain_param.mode == CHAIN_GRAPH_REL) {
-			if (symbol_conf.cumulate_callchain)
-				total = entry->stat_acc->period;
-			else
-				total = entry->stat.period;
-		}
-
-		if (callchain_param.mode == CHAIN_FLAT) {
-			printed += hist_browser__show_callchain_flat(browser,
-					&entry->sorted_chain, row, total,
+		printed += hist_browser__show_callchain(browser, entry, 1, row,
 					hist_browser__show_callchain_entry, &arg,
 					hist_browser__check_output_full);
-		} else if (callchain_param.mode == CHAIN_FOLDED) {
-			printed += hist_browser__show_callchain_folded(browser,
-					&entry->sorted_chain, row, total,
-					hist_browser__show_callchain_entry, &arg,
-					hist_browser__check_output_full);
-		} else {
-			printed += hist_browser__show_callchain(browser,
-					&entry->sorted_chain, 1, row, total,
-					hist_browser__show_callchain_entry, &arg,
-					hist_browser__check_output_full);
-		}
-
-		if (arg.is_current_entry)
-			browser->he_selection = entry;
 	}
 
 	return printed;
@@ -1380,15 +1393,11 @@ do_offset:
 static int hist_browser__fprintf_callchain(struct hist_browser *browser,
 					   struct hist_entry *he, FILE *fp)
 {
-	u64 total = hists__total_period(he->hists);
 	struct callchain_print_arg arg  = {
 		.fp = fp,
 	};
 
-	if (symbol_conf.cumulate_callchain)
-		total = he->stat_acc->period;
-
-	hist_browser__show_callchain(browser, &he->sorted_chain, 1, 0, total,
+	hist_browser__show_callchain(browser, he, 1, 0,
 				     hist_browser__fprintf_callchain_entry, &arg,
 				     hist_browser__check_dump_full);
 	return arg.printed;
-- 
2.6.4

[tip:perf/core] perf hists browser: Fix dump to show correct callchain style

[tip-bot for Namhyung Kim]

Commit-ID:  0c841c6c16f320704f75970bbe6a9800c53e6cf5
Gitweb:     http://git.kernel.org/tip/0c841c6c16f320704f75970bbe6a9800c53e6cf5
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:54 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:44:30 -0300

perf hists browser: Fix dump to show correct callchain style

The commit 8c430a348699 ("perf hists browser: Support folded
callchains") missed to update hist_browser__dump() so it always shows
graph-style callchains regardless of current setting.

To fix that, factor out callchain printing code and rename the existing
function which prints graph-style callchain.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Fixes: 8c430a348699 ("perf hists browser: Support folded callchains")
Link: http://lkml.kernel.org/r/1453909257-26015-8-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 73 ++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 32 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 1da30f8..6b22baf 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -844,7 +844,7 @@ next:
 	return row - first_row;
 }
 
-static int hist_browser__show_callchain(struct hist_browser *browser,
+static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 					struct rb_root *root, int level,
 					unsigned short row, u64 total,
 					print_callchain_entry_fn print,
@@ -898,7 +898,7 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 			else
 				new_total = total;
 
-			row += hist_browser__show_callchain(browser, &child->rb_root,
+			row += hist_browser__show_callchain_graph(browser, &child->rb_root,
 							    new_level, row, new_total,
 							    print, arg, is_output_full);
 		}
@@ -910,6 +910,43 @@ out:
 	return row - first_row;
 }
 
+static int hist_browser__show_callchain(struct hist_browser *browser,
+					struct hist_entry *entry, int level,
+					unsigned short row,
+					print_callchain_entry_fn print,
+					struct callchain_print_arg *arg,
+					check_output_full_fn is_output_full)
+{
+	u64 total = hists__total_period(entry->hists);
+	int printed;
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL) {
+		if (symbol_conf.cumulate_callchain)
+			total = entry->stat_acc->period;
+		else
+			total = entry->stat.period;
+	}
+
+	if (callchain_param.mode == CHAIN_FLAT) {
+		printed = hist_browser__show_callchain_flat(browser,
+						&entry->sorted_chain, row, total,
+						print, arg, is_output_full);
+	} else if (callchain_param.mode == CHAIN_FOLDED) {
+		printed = hist_browser__show_callchain_folded(browser,
+						&entry->sorted_chain, row, total,
+						print, arg, is_output_full);
+	} else {
+		printed = hist_browser__show_callchain_graph(browser,
+						&entry->sorted_chain, level, row, total,
+						print, arg, is_output_full);
+	}
+
+	if (arg->is_current_entry)
+		browser->he_selection = entry;
+
+	return printed;
+}
+
 struct hpp_arg {
 	struct ui_browser *b;
 	char folded_sign;
@@ -1084,38 +1121,14 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 		--row_offset;
 
 	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 (callchain_param.mode == CHAIN_GRAPH_REL) {
-			if (symbol_conf.cumulate_callchain)
-				total = entry->stat_acc->period;
-			else
-				total = entry->stat.period;
-		}
-
-		if (callchain_param.mode == CHAIN_FLAT) {
-			printed += hist_browser__show_callchain_flat(browser,
-					&entry->sorted_chain, row, total,
+		printed += hist_browser__show_callchain(browser, entry, 1, row,
 					hist_browser__show_callchain_entry, &arg,
 					hist_browser__check_output_full);
-		} else if (callchain_param.mode == CHAIN_FOLDED) {
-			printed += hist_browser__show_callchain_folded(browser,
-					&entry->sorted_chain, row, total,
-					hist_browser__show_callchain_entry, &arg,
-					hist_browser__check_output_full);
-		} else {
-			printed += hist_browser__show_callchain(browser,
-					&entry->sorted_chain, 1, row, total,
-					hist_browser__show_callchain_entry, &arg,
-					hist_browser__check_output_full);
-		}
-
-		if (arg.is_current_entry)
-			browser->he_selection = entry;
 	}
 
 	return printed;
@@ -1380,15 +1393,11 @@ do_offset:
 static int hist_browser__fprintf_callchain(struct hist_browser *browser,
 					   struct hist_entry *he, FILE *fp)
 {
-	u64 total = hists__total_period(he->hists);
 	struct callchain_print_arg arg  = {
 		.fp = fp,
 	};
 
-	if (symbol_conf.cumulate_callchain)
-		total = he->stat_acc->period;
-
-	hist_browser__show_callchain(browser, &he->sorted_chain, 1, 0, total,
+	hist_browser__show_callchain(browser, he, 1, 0,
 				     hist_browser__fprintf_callchain_entry, &arg,
 				     hist_browser__check_dump_full);
 	return arg.printed;

[PATCH 08/10] perf hists browser: Pass parent_total to callchain print functions

[Namhyung Kim]

Pass parent node's total period to callchain print functions.  This info
is needed by later patch to determine whether it can omit percent or
not correctly.

No functional change intended.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 44 +++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 91804c5b0130..726c79def88d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -660,6 +660,7 @@ static int hist_browser__show_callchain_list(struct hist_browser *browser,
 static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 					     struct rb_root *root,
 					     unsigned short row, u64 total,
+					     u64 parent_total __maybe_unused,
 					     print_callchain_entry_fn print,
 					     struct callchain_print_arg *arg,
 					     check_output_full_fn is_output_full)
@@ -763,6 +764,7 @@ static char *hist_browser__folded_callchain_str(struct hist_browser *browser,
 static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 					       struct rb_root *root,
 					       unsigned short row, u64 total,
+					       u64 parent_total __maybe_unused,
 					       print_callchain_entry_fn print,
 					       struct callchain_print_arg *arg,
 					       check_output_full_fn is_output_full)
@@ -847,14 +849,18 @@ next:
 static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 					struct rb_root *root, int level,
 					unsigned short row, u64 total,
+					u64 parent_total,
 					print_callchain_entry_fn print,
 					struct callchain_print_arg *arg,
 					check_output_full_fn is_output_full)
 {
 	struct rb_node *node;
 	int first_row = row, offset = level * LEVEL_OFFSET_STEP;
-	u64 new_total;
 	bool need_percent;
+	u64 percent_total = total;
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL)
+		percent_total = parent_total;
 
 	node = rb_first(root);
 	need_percent = node && rb_next(node);
@@ -878,7 +884,7 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 			folded_sign = callchain_list__folded(chain);
 
 			row += hist_browser__show_callchain_list(browser, child,
-							chain, row, total,
+							chain, row, percent_total,
 							was_first && need_percent,
 							offset + extra_offset,
 							print, arg);
@@ -893,13 +899,9 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 		if (folded_sign == '-') {
 			const int new_level = level + (extra_offset ? 2 : 1);
 
-			if (callchain_param.mode == CHAIN_GRAPH_REL)
-				new_total = child->children_hit;
-			else
-				new_total = total;
-
 			row += hist_browser__show_callchain_graph(browser, &child->rb_root,
-							    new_level, row, new_total,
+							    new_level, row, total,
+							    child->children_hit,
 							    print, arg, is_output_full);
 		}
 		if (is_output_full(browser, row))
@@ -918,27 +920,29 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 					check_output_full_fn is_output_full)
 {
 	u64 total = hists__total_period(entry->hists);
+	u64 parent_total;
 	int printed;
 
-	if (callchain_param.mode == CHAIN_GRAPH_REL) {
-		if (symbol_conf.cumulate_callchain)
-			total = entry->stat_acc->period;
-		else
-			total = entry->stat.period;
-	}
+	if (symbol_conf.cumulate_callchain)
+		parent_total = entry->stat_acc->period;
+	else
+		parent_total = entry->stat.period;
 
 	if (callchain_param.mode == CHAIN_FLAT) {
 		printed = hist_browser__show_callchain_flat(browser,
-						&entry->sorted_chain, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	} else if (callchain_param.mode == CHAIN_FOLDED) {
 		printed = hist_browser__show_callchain_folded(browser,
-						&entry->sorted_chain, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	} else {
 		printed = hist_browser__show_callchain_graph(browser,
-						&entry->sorted_chain, level, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, level, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	}
 
 	if (arg->is_current_entry)
-- 
2.6.4

[tip:perf/core] perf hists browser: Pass parent_total to callchain print functions

[tip-bot for Namhyung Kim]

Commit-ID:  5eca104eee7edfe7155523849750ced539b16e94
Gitweb:     http://git.kernel.org/tip/5eca104eee7edfe7155523849750ced539b16e94
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:55 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:45:42 -0300

perf hists browser: Pass parent_total to callchain print functions

Pass parent node's total period to callchain print functions.  This info
is needed by later patch to determine whether it can omit percent or not
correctly.

No functional change intended.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-9-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 44 +++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 6b22baf..41dbb79 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -660,6 +660,7 @@ static int hist_browser__show_callchain_list(struct hist_browser *browser,
 static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 					     struct rb_root *root,
 					     unsigned short row, u64 total,
+					     u64 parent_total __maybe_unused,
 					     print_callchain_entry_fn print,
 					     struct callchain_print_arg *arg,
 					     check_output_full_fn is_output_full)
@@ -763,6 +764,7 @@ static char *hist_browser__folded_callchain_str(struct hist_browser *browser,
 static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 					       struct rb_root *root,
 					       unsigned short row, u64 total,
+					       u64 parent_total __maybe_unused,
 					       print_callchain_entry_fn print,
 					       struct callchain_print_arg *arg,
 					       check_output_full_fn is_output_full)
@@ -847,14 +849,18 @@ next:
 static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 					struct rb_root *root, int level,
 					unsigned short row, u64 total,
+					u64 parent_total,
 					print_callchain_entry_fn print,
 					struct callchain_print_arg *arg,
 					check_output_full_fn is_output_full)
 {
 	struct rb_node *node;
 	int first_row = row, offset = level * LEVEL_OFFSET_STEP;
-	u64 new_total;
 	bool need_percent;
+	u64 percent_total = total;
+
+	if (callchain_param.mode == CHAIN_GRAPH_REL)
+		percent_total = parent_total;
 
 	node = rb_first(root);
 	need_percent = node && rb_next(node);
@@ -878,7 +884,7 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 			folded_sign = callchain_list__folded(chain);
 
 			row += hist_browser__show_callchain_list(browser, child,
-							chain, row, total,
+							chain, row, percent_total,
 							was_first && need_percent,
 							offset + extra_offset,
 							print, arg);
@@ -893,13 +899,9 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 		if (folded_sign == '-') {
 			const int new_level = level + (extra_offset ? 2 : 1);
 
-			if (callchain_param.mode == CHAIN_GRAPH_REL)
-				new_total = child->children_hit;
-			else
-				new_total = total;
-
 			row += hist_browser__show_callchain_graph(browser, &child->rb_root,
-							    new_level, row, new_total,
+							    new_level, row, total,
+							    child->children_hit,
 							    print, arg, is_output_full);
 		}
 		if (is_output_full(browser, row))
@@ -918,27 +920,29 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 					check_output_full_fn is_output_full)
 {
 	u64 total = hists__total_period(entry->hists);
+	u64 parent_total;
 	int printed;
 
-	if (callchain_param.mode == CHAIN_GRAPH_REL) {
-		if (symbol_conf.cumulate_callchain)
-			total = entry->stat_acc->period;
-		else
-			total = entry->stat.period;
-	}
+	if (symbol_conf.cumulate_callchain)
+		parent_total = entry->stat_acc->period;
+	else
+		parent_total = entry->stat.period;
 
 	if (callchain_param.mode == CHAIN_FLAT) {
 		printed = hist_browser__show_callchain_flat(browser,
-						&entry->sorted_chain, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	} else if (callchain_param.mode == CHAIN_FOLDED) {
 		printed = hist_browser__show_callchain_folded(browser,
-						&entry->sorted_chain, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	} else {
 		printed = hist_browser__show_callchain_graph(browser,
-						&entry->sorted_chain, level, row, total,
-						print, arg, is_output_full);
+						&entry->sorted_chain, level, row,
+						total, parent_total, print, arg,
+						is_output_full);
 	}
 
 	if (arg->is_current_entry)

[PATCH 09/10] perf hists browser: Fix percent display in callchains

[Namhyung Kim]

When there's only a single callchain, perf doesn't print its percentage
in front of the symbols.  This is because it assumes that the percentage
is same as parents.  But if a percent limit is applied, it's possible
that there are actually a couple of child nodes but only one of them is
shown.  In this case it should display the percent to prevent
misunderstanding of its percentage is same as the parent's.

For example, let's see the following callchain.

  $ perf report --percent-limit 0.01 --tui
  ...
  -    0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - 0.04% mmap_region
             do_mmap_pgoff
           - vm_mmap_pgoff
              + 0.02% sys_mmap_pgoff
              + 0.02% vm_mmap
           + 0.02% mprotect_fixup

Current code omits the percent if 'mmap_region' becomes the only node
when percent limit is set to 0.03%, its percent is not 0.06% but users
will assume it incorrectly.

Before:

  $ perf report --percent-limit 0.03 --tui
  ...
     0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - mmap_region
          do_mmap_pgoff
          vm_mmap_pgoff

After:

  $ perf report --percent-limit 0.03 --stdio
  ...
     0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - 0.04% mmap_region
             do_mmap_pgoff
             vm_mmap_pgoff

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 726c79def88d..4019221f6755 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -657,10 +657,24 @@ static int hist_browser__show_callchain_list(struct hist_browser *browser,
 	return 1;
 }
 
+static bool check_percent_display(struct rb_node *node, u64 parent_total)
+{
+	struct callchain_node *child;
+
+	if (node == NULL)
+		return false;
+
+	if (rb_next(node))
+		return true;
+
+	child = rb_entry(node, struct callchain_node, rb_node);
+	return callchain_cumul_hits(child) != parent_total;
+}
+
 static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 					     struct rb_root *root,
 					     unsigned short row, u64 total,
-					     u64 parent_total __maybe_unused,
+					     u64 parent_total,
 					     print_callchain_entry_fn print,
 					     struct callchain_print_arg *arg,
 					     check_output_full_fn is_output_full)
@@ -670,7 +684,7 @@ static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 	bool need_percent;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
@@ -764,7 +778,7 @@ static char *hist_browser__folded_callchain_str(struct hist_browser *browser,
 static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 					       struct rb_root *root,
 					       unsigned short row, u64 total,
-					       u64 parent_total __maybe_unused,
+					       u64 parent_total,
 					       print_callchain_entry_fn print,
 					       struct callchain_print_arg *arg,
 					       check_output_full_fn is_output_full)
@@ -774,7 +788,7 @@ static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 	bool need_percent;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
@@ -863,7 +877,7 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 		percent_total = parent_total;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
-- 
2.6.4

[tip:perf/core] perf hists browser: Fix percent display in callchains

[tip-bot for Namhyung Kim]

Commit-ID:  59c624e2391080fa6315a376a4ee74d0eb393d1d
Gitweb:     http://git.kernel.org/tip/59c624e2391080fa6315a376a4ee74d0eb393d1d
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 00:40:56 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:46:27 -0300

perf hists browser: Fix percent display in callchains

When there's only a single callchain, perf doesn't print its percentage
in front of the symbols.  This is because it assumes that the percentage
is same as parents.  But if a percent limit is applied, it's possible
that there are actually a couple of child nodes but only one of them is
shown.  In this case it should display the percent to prevent
misunderstanding of its percentage is same as the parent's.

For example, let's see the following callchain.

  $ perf report --no-children --percent-limit 0.01 --tui
  ...
  -    0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - 0.04% mmap_region
             do_mmap_pgoff
           - vm_mmap_pgoff
              + 0.02% sys_mmap_pgoff
              + 0.02% vm_mmap
           + 0.02% mprotect_fixup

Current code omits the percent if 'mmap_region' becomes the only node
when percent limit is set to 0.03%, its percent is not 0.06% but users
will assume it incorrectly.

Before:

  $ perf report --no-children --percent-limit 0.03 --tui
  ...
     0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - mmap_region
          do_mmap_pgoff
          vm_mmap_pgoff

After:

  $ perf report --no-children --percent-limit 0.03 --tui
  ...
     0.06%  sleep    [kernel.vmlinux]    [k] kmem_cache_alloc_trace
       kmem_cache_alloc_trace
     - perf_event_mmap
        - 0.04% mmap_region
             do_mmap_pgoff
             vm_mmap_pgoff

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1453909257-26015-10-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 41dbb79..61d578b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -657,10 +657,24 @@ static int hist_browser__show_callchain_list(struct hist_browser *browser,
 	return 1;
 }
 
+static bool check_percent_display(struct rb_node *node, u64 parent_total)
+{
+	struct callchain_node *child;
+
+	if (node == NULL)
+		return false;
+
+	if (rb_next(node))
+		return true;
+
+	child = rb_entry(node, struct callchain_node, rb_node);
+	return callchain_cumul_hits(child) != parent_total;
+}
+
 static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 					     struct rb_root *root,
 					     unsigned short row, u64 total,
-					     u64 parent_total __maybe_unused,
+					     u64 parent_total,
 					     print_callchain_entry_fn print,
 					     struct callchain_print_arg *arg,
 					     check_output_full_fn is_output_full)
@@ -670,7 +684,7 @@ static int hist_browser__show_callchain_flat(struct hist_browser *browser,
 	bool need_percent;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
@@ -764,7 +778,7 @@ static char *hist_browser__folded_callchain_str(struct hist_browser *browser,
 static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 					       struct rb_root *root,
 					       unsigned short row, u64 total,
-					       u64 parent_total __maybe_unused,
+					       u64 parent_total,
 					       print_callchain_entry_fn print,
 					       struct callchain_print_arg *arg,
 					       check_output_full_fn is_output_full)
@@ -774,7 +788,7 @@ static int hist_browser__show_callchain_folded(struct hist_browser *browser,
 	bool need_percent;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);
@@ -863,7 +877,7 @@ static int hist_browser__show_callchain_graph(struct hist_browser *browser,
 		percent_total = parent_total;
 
 	node = rb_first(root);
-	need_percent = node && rb_next(node);
+	need_percent = check_percent_display(node, parent_total);
 
 	while (node) {
 		struct callchain_node *child = rb_entry(node, struct callchain_node, rb_node);

[RFC/PATCH 10/10] perf tools: Change default calchain percent limit to 0.005%

[Namhyung Kim]

The current default limit of 0.5% is (mostly) for 'fractal' mode which
calculates the percentage relatively.  As we changed the default
callchain mode to 'graph', the existing limit is too high IMHO.
Normally there're many entries under 0.5% overhead, it'd be supprising
that they don't show callchains.

Cc: Taeung Song <treeze.taeung@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-report.txt | 2 +-
 tools/perf/builtin-report.c              | 2 +-
 tools/perf/builtin-top.c                 | 2 +-
 tools/perf/util/util.c                   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 8a301f6afb37..1591b45c79e5 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -209,7 +209,7 @@ OPTIONS
 	- none: disable call chain display.
 
 	threshold is a percentage value which specifies a minimum percent to be
-	included in the output call graph.  Default is 0.5 (%).
+	included in the output call graph.  Default is 0.005 (%).
 
 	print_limit is only applied when stdio interface is used.  It's to limit
 	number of call graph entries in a single hist entry.  Note that it needs
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 72ed0b46d5a1..ba6eba5f5d69 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -643,7 +643,7 @@ parse_percent_limit(const struct option *opt, const char *str,
 	return 0;
 }
 
-#define CALLCHAIN_DEFAULT_OPT  "graph,0.5,caller,function,percent"
+#define CALLCHAIN_DEFAULT_OPT  "graph,0.005,caller,function,percent"
 
 const char report_callchain_help[] = "Display call graph (stack chain/backtrace):\n\n"
 				     CALLCHAIN_REPORT_HELP
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index bf01cbb0ef23..1b471135ae2d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1086,7 +1086,7 @@ parse_percent_limit(const struct option *opt, const char *arg,
 }
 
 const char top_callchain_help[] = CALLCHAIN_RECORD_HELP CALLCHAIN_REPORT_HELP
-	"\n\t\t\t\tDefault: fp,graph,0.5,caller,function";
+	"\n\t\t\t\tDefault: fp,graph,0.005,caller,function";
 
 int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 {
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 7a2da7ef556e..25ae6989c89d 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -20,7 +20,7 @@
 
 struct callchain_param	callchain_param = {
 	.mode	= CHAIN_GRAPH_ABS,
-	.min_percent = 0.5,
+	.min_percent = 0.005,
 	.order  = ORDER_CALLEE,
 	.key	= CCKEY_FUNCTION,
 	.value	= CCVAL_PERCENT,
-- 
2.6.4

Re: [RFC/PATCH 10/10] perf tools: Change default calchain percent limit to 0.005%

[Andi Kleen]

On Thu, Jan 28, 2016 at 12:40:57AM +0900, Namhyung Kim wrote:
> The current default limit of 0.5% is (mostly) for 'fractal' mode which
> calculates the percentage relatively.  As we changed the default
> callchain mode to 'graph', the existing limit is too high IMHO.
> Normally there're many entries under 0.5% overhead, it'd be supprising
> that they don't show callchains.

It's too small imho. On more complex workloads with a lot of user space
code we often end up with default perf report --stdio callgraph output of several
hundred KB, and most of it is useless as it is only very small.

-Andi

Re: [RFC/PATCH 10/10] perf tools: Change default calchain percent limit to 0.005%

[Namhyung Kim]

Hi Andi,

On Wed, 2016-01-27 at 17:06 +0100, Andi Kleen wrote:
> On Thu, Jan 28, 2016 at 12:40:57AM +0900, Namhyung Kim wrote:
> > The current default limit of 0.5% is (mostly) for 'fractal' mode
> > which
> > calculates the percentage relatively.  As we changed the default
> > callchain mode to 'graph', the existing limit is too high IMHO.
> > Normally there're many entries under 0.5% overhead, it'd be
> > supprising
> > that they don't show callchains.
> 
> It's too small imho. On more complex workloads with a lot of user
> space
> code we often end up with default perf report --stdio callgraph output
> of several
> hundred KB, and most of it is useless as it is only very small.

Do you think it's better to keep current default?  Or do you suggest
different value?

Thanks,
Namhyung

Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Jiri Olsa]

On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
> Hello,
> 
> This patchset tries to implement percent limit to callchains which was
> requested by Andi Kleen.  For some reason, limiting callchains by
> (overhead) percentage didn't work well.  This patch fixes it and make
> --percent-limit also works for callchains as well as hist entries.
> 
>  * Changes from v1)
>   - fix insertion path instead of changing all UI code
>   - show percent value even on single path (if needed)
>   - change default callchain percent limit
>   
> This is available on 'perf/callchain-limit-v2' branch in my tree:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Any comments are welcome,
> 
> Thanks,
> Namhyung
> *** BLURB HERE ***
> 
> Namhyung Kim (10):
>   perf hists: Fix min callchain hits calculation
>   perf hists: Update hists' total period when adding entries
>   perf report: Apply --percent-limit to callchains also
>   perf report: Get rid of hist_entry__callchain_fprintf()
>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
>   perf report: Fix percent display in callchains on --stdio
>   perf hists browser: Fix dump to show correct callchain style
>   perf hists browser: Pass parent_total to callchain print functions
>   perf hists browser: Fix percent display in callchains
>   perf tools: Change default calchain percent limit to 0.005%

heya,
do I get it right that:

'perf report' display all entries, but only callchains above default limit

'perf report --percent-limit X' display only entries above limit X plus
only callchains above limit X


thanks,
jirka

Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Namhyung Kim]

On January 28, 2016 5:14:39 PM GMT+09:00, Jiri Olsa <jolsa@redhat.com> wrote:
>On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
>> Hello,
>> 
>> This patchset tries to implement percent limit to callchains which
>was
>> requested by Andi Kleen.  For some reason, limiting callchains by
>> (overhead) percentage didn't work well.  This patch fixes it and make
>> --percent-limit also works for callchains as well as hist entries.
>> 
>>  * Changes from v1)
>>   - fix insertion path instead of changing all UI code
>>   - show percent value even on single path (if needed)
>>   - change default callchain percent limit
>>   
>> This is available on 'perf/callchain-limit-v2' branch in my tree:
>> 
>>  
>git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>> 
>> Any comments are welcome,
>> 
>> Thanks,
>> Namhyung
>> *** BLURB HERE ***
>> 
>> Namhyung Kim (10):
>>   perf hists: Fix min callchain hits calculation
>>   perf hists: Update hists' total period when adding entries
>>   perf report: Apply --percent-limit to callchains also
>>   perf report: Get rid of hist_entry__callchain_fprintf()
>>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
>>   perf report: Fix percent display in callchains on --stdio
>>   perf hists browser: Fix dump to show correct callchain style
>>   perf hists browser: Pass parent_total to callchain print functions
>>   perf hists browser: Fix percent display in callchains
>>   perf tools: Change default calchain percent limit to 0.005%
>
>heya,
>do I get it right that:
>
>'perf report' display all entries, but only callchains above default
>limit
>
>'perf report --percent-limit X' display only entries above limit X plus
>only callchains above limit X
>
>
>thanks,
>jirka

Yes, actually there're two limits and they have different defaults. The --percent-limit sets both limits while -g only sets callchain limit.

Thanks
Namhyung
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Jiri Olsa]

On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
> Hello,
> 
> This patchset tries to implement percent limit to callchains which was
> requested by Andi Kleen.  For some reason, limiting callchains by
> (overhead) percentage didn't work well.  This patch fixes it and make
> --percent-limit also works for callchains as well as hist entries.
> 
>  * Changes from v1)
>   - fix insertion path instead of changing all UI code
>   - show percent value even on single path (if needed)
>   - change default callchain percent limit
>   
> This is available on 'perf/callchain-limit-v2' branch in my tree:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Any comments are welcome,
> 
> Thanks,
> Namhyung
> *** BLURB HERE ***
> 
> Namhyung Kim (10):
>   perf hists: Fix min callchain hits calculation
>   perf hists: Update hists' total period when adding entries
>   perf report: Apply --percent-limit to callchains also
>   perf report: Get rid of hist_entry__callchain_fprintf()
>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
>   perf report: Fix percent display in callchains on --stdio
>   perf hists browser: Fix dump to show correct callchain style
>   perf hists browser: Pass parent_total to callchain print functions
>   perf hists browser: Fix percent display in callchains
>   perf tools: Change default calchain percent limit to 0.005%

also I see extra fo entries with callchain filtered out in stdio mode

jirka


---
     8.41%  yes      libc-2.21.so      [.] fputs_unlocked                
            |
            ---fputs_unlocked
               |          
               |--5.67%--0x757074756f206472
               |          
                --2.74%--0x3ba8e0
                          0x21e000

     2.47%  yes      yes               [.] fputs_unlocked@plt            
            |
            ---fputs_unlocked@plt
               0x3ba8e0
               0x21e000

     0.12%  yes      [kernel.vmlinux]  [k] vfs_write                     

     0.09%  yes      libc-2.21.so      [.] _IO_do_write@@GLIBC_2.2.5     

     0.08%  yes      [kernel.vmlinux]  [k] entry_SYSCALL_64              

     0.07%  yes      [kernel.vmlinux]  [k] fsnotify                      

     0.06%  yes      [kernel.vmlinux]  [k] sys_write                     

Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Namhyung Kim]

On Thu, Jan 28, 2016 at 5:16 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
>> Hello,
>>
>> This patchset tries to implement percent limit to callchains which was
>> requested by Andi Kleen.  For some reason, limiting callchains by
>> (overhead) percentage didn't work well.  This patch fixes it and make
>> --percent-limit also works for callchains as well as hist entries.
>>
>>  * Changes from v1)
>>   - fix insertion path instead of changing all UI code
>>   - show percent value even on single path (if needed)
>>   - change default callchain percent limit
>>
>> This is available on 'perf/callchain-limit-v2' branch in my tree:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>>
>> Any comments are welcome,
>>
>> Thanks,
>> Namhyung
>> *** BLURB HERE ***
>>
>> Namhyung Kim (10):
>>   perf hists: Fix min callchain hits calculation
>>   perf hists: Update hists' total period when adding entries
>>   perf report: Apply --percent-limit to callchains also
>>   perf report: Get rid of hist_entry__callchain_fprintf()
>>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
>>   perf report: Fix percent display in callchains on --stdio
>>   perf hists browser: Fix dump to show correct callchain style
>>   perf hists browser: Pass parent_total to callchain print functions
>>   perf hists browser: Fix percent display in callchains
>>   perf tools: Change default calchain percent limit to 0.005%
>
> also I see extra fo entries with callchain filtered out in stdio mode
>
> jirka
>
>
> ---
>      8.41%  yes      libc-2.21.so      [.] fputs_unlocked
>             |
>             ---fputs_unlocked
>                |
>                |--5.67%--0x757074756f206472
>                |
>                 --2.74%--0x3ba8e0
>                           0x21e000
>
>      2.47%  yes      yes               [.] fputs_unlocked@plt
>             |
>             ---fputs_unlocked@plt
>                0x3ba8e0
>                0x21e000
>
>      0.12%  yes      [kernel.vmlinux]  [k] vfs_write
>
>      0.09%  yes      libc-2.21.so      [.] _IO_do_write@@GLIBC_2.2.5
>
>      0.08%  yes      [kernel.vmlinux]  [k] entry_SYSCALL_64
>
>      0.07%  yes      [kernel.vmlinux]  [k] fsnotify
>
>      0.06%  yes      [kernel.vmlinux]  [k] sys_write
>

I guess it's same for other UI outputs too.

The default limit of hist entries is 0 so it basically shows all
entries.  But default callchain limit is 0.5% so hist entries under
0.5% won't show callchains.

Thanks,
Namhyung

Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Namhyung Kim]

On Thu, Jan 28, 2016 at 7:14 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> On Thu, Jan 28, 2016 at 5:16 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>> On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
>>> Hello,
>>>
>>> This patchset tries to implement percent limit to callchains which was
>>> requested by Andi Kleen.  For some reason, limiting callchains by
>>> (overhead) percentage didn't work well.  This patch fixes it and make
>>> --percent-limit also works for callchains as well as hist entries.
>>>
>>>  * Changes from v1)
>>>   - fix insertion path instead of changing all UI code
>>>   - show percent value even on single path (if needed)
>>>   - change default callchain percent limit
>>>
>>> This is available on 'perf/callchain-limit-v2' branch in my tree:
>>>
>>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>>>
>>> Any comments are welcome,
>>>
>>> Thanks,
>>> Namhyung
>>> *** BLURB HERE ***
>>>
>>> Namhyung Kim (10):
>>>   perf hists: Fix min callchain hits calculation
>>>   perf hists: Update hists' total period when adding entries
>>>   perf report: Apply --percent-limit to callchains also
>>>   perf report: Get rid of hist_entry__callchain_fprintf()
>>>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
>>>   perf report: Fix percent display in callchains on --stdio
>>>   perf hists browser: Fix dump to show correct callchain style
>>>   perf hists browser: Pass parent_total to callchain print functions
>>>   perf hists browser: Fix percent display in callchains
>>>   perf tools: Change default calchain percent limit to 0.005%
>>
>> also I see extra fo entries with callchain filtered out in stdio mode
>>
>> jirka
>>
>>
>> ---
>>      8.41%  yes      libc-2.21.so      [.] fputs_unlocked
>>             |
>>             ---fputs_unlocked
>>                |
>>                |--5.67%--0x757074756f206472
>>                |
>>                 --2.74%--0x3ba8e0
>>                           0x21e000
>>
>>      2.47%  yes      yes               [.] fputs_unlocked@plt
>>             |
>>             ---fputs_unlocked@plt
>>                0x3ba8e0
>>                0x21e000
>>
>>      0.12%  yes      [kernel.vmlinux]  [k] vfs_write
>>
>>      0.09%  yes      libc-2.21.so      [.] _IO_do_write@@GLIBC_2.2.5
>>
>>      0.08%  yes      [kernel.vmlinux]  [k] entry_SYSCALL_64
>>
>>      0.07%  yes      [kernel.vmlinux]  [k] fsnotify
>>
>>      0.06%  yes      [kernel.vmlinux]  [k] sys_write
>>
>
> I guess it's same for other UI outputs too.
>
> The default limit of hist entries is 0 so it basically shows all
> entries.  But default callchain limit is 0.5% so hist entries under
> 0.5% won't show callchains.

Btw, I changed it to 0.005% in this patchset.  Did you apply all the
patches and run 'perf report' with default value?

Thanks,
Namhyung

Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Jiri Olsa]

On Thu, Jan 28, 2016 at 07:16:43PM +0900, Namhyung Kim wrote:
> On Thu, Jan 28, 2016 at 7:14 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> > On Thu, Jan 28, 2016 at 5:16 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> >> On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
> >>> Hello,
> >>>
> >>> This patchset tries to implement percent limit to callchains which was
> >>> requested by Andi Kleen.  For some reason, limiting callchains by
> >>> (overhead) percentage didn't work well.  This patch fixes it and make
> >>> --percent-limit also works for callchains as well as hist entries.
> >>>
> >>>  * Changes from v1)
> >>>   - fix insertion path instead of changing all UI code
> >>>   - show percent value even on single path (if needed)
> >>>   - change default callchain percent limit
> >>>
> >>> This is available on 'perf/callchain-limit-v2' branch in my tree:
> >>>
> >>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> >>>
> >>> Any comments are welcome,
> >>>
> >>> Thanks,
> >>> Namhyung
> >>> *** BLURB HERE ***
> >>>
> >>> Namhyung Kim (10):
> >>>   perf hists: Fix min callchain hits calculation
> >>>   perf hists: Update hists' total period when adding entries
> >>>   perf report: Apply --percent-limit to callchains also
> >>>   perf report: Get rid of hist_entry__callchain_fprintf()
> >>>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
> >>>   perf report: Fix percent display in callchains on --stdio
> >>>   perf hists browser: Fix dump to show correct callchain style
> >>>   perf hists browser: Pass parent_total to callchain print functions
> >>>   perf hists browser: Fix percent display in callchains
> >>>   perf tools: Change default calchain percent limit to 0.005%
> >>
> >> also I see extra fo entries with callchain filtered out in stdio mode
> >>
> >> jirka
> >>
> >>
> >> ---
> >>      8.41%  yes      libc-2.21.so      [.] fputs_unlocked
> >>             |
> >>             ---fputs_unlocked
> >>                |
> >>                |--5.67%--0x757074756f206472
> >>                |
> >>                 --2.74%--0x3ba8e0
> >>                           0x21e000
> >>
> >>      2.47%  yes      yes               [.] fputs_unlocked@plt
> >>             |
> >>             ---fputs_unlocked@plt
> >>                0x3ba8e0
> >>                0x21e000
> >>
> >>      0.12%  yes      [kernel.vmlinux]  [k] vfs_write
> >>
> >>      0.09%  yes      libc-2.21.so      [.] _IO_do_write@@GLIBC_2.2.5
> >>
> >>      0.08%  yes      [kernel.vmlinux]  [k] entry_SYSCALL_64
> >>
> >>      0.07%  yes      [kernel.vmlinux]  [k] fsnotify
> >>
> >>      0.06%  yes      [kernel.vmlinux]  [k] sys_write
> >>
> >
> > I guess it's same for other UI outputs too.
> >
> > The default limit of hist entries is 0 so it basically shows all
> > entries.  But default callchain limit is 0.5% so hist entries under
> > 0.5% won't show callchains.
> 
> Btw, I changed it to 0.005% in this patchset.  Did you apply all the
> patches and run 'perf report' with default value?

yep, I had it and then reverted ;-) but I made typo
in the previous email.. what I meant was:

also I see extra LINE for entries...  ;-)

thanks,
jirka

Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Namhyung Kim]

On Thu, Jan 28, 2016 at 01:12:45PM +0100, Jiri Olsa wrote:
> On Thu, Jan 28, 2016 at 07:16:43PM +0900, Namhyung Kim wrote:
> > On Thu, Jan 28, 2016 at 7:14 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> > > On Thu, Jan 28, 2016 at 5:16 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> > >> On Thu, Jan 28, 2016 at 12:40:47AM +0900, Namhyung Kim wrote:
> > >>> Hello,
> > >>>
> > >>> This patchset tries to implement percent limit to callchains which was
> > >>> requested by Andi Kleen.  For some reason, limiting callchains by
> > >>> (overhead) percentage didn't work well.  This patch fixes it and make
> > >>> --percent-limit also works for callchains as well as hist entries.
> > >>>
> > >>>  * Changes from v1)
> > >>>   - fix insertion path instead of changing all UI code
> > >>>   - show percent value even on single path (if needed)
> > >>>   - change default callchain percent limit
> > >>>
> > >>> This is available on 'perf/callchain-limit-v2' branch in my tree:
> > >>>
> > >>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> > >>>
> > >>> Any comments are welcome,
> > >>>
> > >>> Thanks,
> > >>> Namhyung
> > >>> *** BLURB HERE ***
> > >>>
> > >>> Namhyung Kim (10):
> > >>>   perf hists: Fix min callchain hits calculation
> > >>>   perf hists: Update hists' total period when adding entries
> > >>>   perf report: Apply --percent-limit to callchains also
> > >>>   perf report: Get rid of hist_entry__callchain_fprintf()
> > >>>   perf tools: Pass parent_samples to __callchain__fprintf_graph()
> > >>>   perf report: Fix percent display in callchains on --stdio
> > >>>   perf hists browser: Fix dump to show correct callchain style
> > >>>   perf hists browser: Pass parent_total to callchain print functions
> > >>>   perf hists browser: Fix percent display in callchains
> > >>>   perf tools: Change default calchain percent limit to 0.005%
> > >>
> > >> also I see extra fo entries with callchain filtered out in stdio mode
> > >>
> > >> jirka
> > >>
> > >>
> > >> ---
> > >>      8.41%  yes      libc-2.21.so      [.] fputs_unlocked
> > >>             |
> > >>             ---fputs_unlocked
> > >>                |
> > >>                |--5.67%--0x757074756f206472
> > >>                |
> > >>                 --2.74%--0x3ba8e0
> > >>                           0x21e000
> > >>
> > >>      2.47%  yes      yes               [.] fputs_unlocked@plt
> > >>             |
> > >>             ---fputs_unlocked@plt
> > >>                0x3ba8e0
> > >>                0x21e000
> > >>
> > >>      0.12%  yes      [kernel.vmlinux]  [k] vfs_write
> > >>
> > >>      0.09%  yes      libc-2.21.so      [.] _IO_do_write@@GLIBC_2.2.5
> > >>
> > >>      0.08%  yes      [kernel.vmlinux]  [k] entry_SYSCALL_64
> > >>
> > >>      0.07%  yes      [kernel.vmlinux]  [k] fsnotify
> > >>
> > >>      0.06%  yes      [kernel.vmlinux]  [k] sys_write
> > >>
> > >
> > > I guess it's same for other UI outputs too.
> > >
> > > The default limit of hist entries is 0 so it basically shows all
> > > entries.  But default callchain limit is 0.5% so hist entries under
> > > 0.5% won't show callchains.
> > 
> > Btw, I changed it to 0.005% in this patchset.  Did you apply all the
> > patches and run 'perf report' with default value?
> 
> yep, I had it and then reverted ;-) but I made typo
> in the previous email.. what I meant was:
> 
> also I see extra LINE for entries...  ;-)

Ah, so you meant the blank lines..  The fix would be like following



>From 62ac44405797275aed35acb38cfe3d1afa6b709c Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Thu, 28 Jan 2016 21:18:53 +0900
Subject: [PATCH 11/10] perf report: Don't show blank lines if entry has no
 callchain

When all callchains of a hist entry is percent-limited, do not add a
blank line at the end.  It makes the entry look like it doesn't have
callchains.

Reported-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/stdio/hist.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 76ff46becac8..691e52ce7510 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -233,7 +233,10 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 
 	ret += __callchain__fprintf_graph(fp, root, total_samples,
 					  1, 1, left_margin);
-	ret += fprintf(fp, "\n");
+	if (ret) {
+		/* do not add a blank line if it printed nothing */
+		ret += fprintf(fp, "\n");
+	}
 
 	return ret;
 }
-- 
2.6.4

Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Jiri Olsa]

On Thu, Jan 28, 2016 at 09:24:54PM +0900, Namhyung Kim wrote:

SNIP

> > > > The default limit of hist entries is 0 so it basically shows all
> > > > entries.  But default callchain limit is 0.5% so hist entries under
> > > > 0.5% won't show callchains.
> > > 
> > > Btw, I changed it to 0.005% in this patchset.  Did you apply all the
> > > patches and run 'perf report' with default value?
> > 
> > yep, I had it and then reverted ;-) but I made typo
> > in the previous email.. what I meant was:
> > 
> > also I see extra LINE for entries...  ;-)
> 
> Ah, so you meant the blank lines..  The fix would be like following

yep, tested.. works ;-)

thanks,
jirka

Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Arnaldo Carvalho de Melo]

Em Thu, Jan 28, 2016 at 08:52:25PM +0100, Jiri Olsa escreveu:
> On Thu, Jan 28, 2016 at 09:24:54PM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> > > > > The default limit of hist entries is 0 so it basically shows all
> > > > > entries.  But default callchain limit is 0.5% so hist entries under
> > > > > 0.5% won't show callchains.
> > > > 
> > > > Btw, I changed it to 0.005% in this patchset.  Did you apply all the
> > > > patches and run 'perf report' with default value?
> > > 
> > > yep, I had it and then reverted ;-) but I made typo
> > > in the previous email.. what I meant was:
> > > 
> > > also I see extra LINE for entries...  ;-)
> > 
> > Ah, so you meant the blank lines..  The fix would be like following
> 
> yep, tested.. works ;-)

Namhyung, can I try to process this patchkit just by reading these
commends and making the adjustments? Or is there something outstanding
that warrants you to push a v2?

- Arnaldo

Re: [PATCHSET 00/10] perf tools: Apply percent-limit to callchains (v2)

[Namhyung Kim]

Hi Arnaldo,

On Sat, Jan 30, 2016 at 6:00 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Thu, Jan 28, 2016 at 08:52:25PM +0100, Jiri Olsa escreveu:
>> On Thu, Jan 28, 2016 at 09:24:54PM +0900, Namhyung Kim wrote:
>>
>> SNIP
>>
>> > > > > The default limit of hist entries is 0 so it basically shows all
>> > > > > entries.  But default callchain limit is 0.5% so hist entries under
>> > > > > 0.5% won't show callchains.
>> > > >
>> > > > Btw, I changed it to 0.005% in this patchset.  Did you apply all the
>> > > > patches and run 'perf report' with default value?
>> > >
>> > > yep, I had it and then reverted ;-) but I made typo
>> > > in the previous email.. what I meant was:
>> > >
>> > > also I see extra LINE for entries...  ;-)
>> >
>> > Ah, so you meant the blank lines..  The fix would be like following
>>
>> yep, tested.. works ;-)
>
> Namhyung, can I try to process this patchkit just by reading these
> commends and making the adjustments? Or is there something outstanding
> that warrants you to push a v2?

It'd great for me if you process this (v1).  I cannot work on the v2
for a couple of days..

Thanks,
Namhyung

[tip:perf/core] perf report: Don' t show blank lines if entry has no callchain

[tip-bot for Namhyung Kim]

Commit-ID:  3848c23b19e07188bfa15e3d9a2ac27692f2ff3c
Gitweb:     http://git.kernel.org/tip/3848c23b19e07188bfa15e3d9a2ac27692f2ff3c
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 28 Jan 2016 21:24:54 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 1 Feb 2016 17:51:09 -0300

perf report: Don't show blank lines if entry has no callchain

When all callchains of a hist entry is percent-limited, do not add a
blank line at the end.  It makes the entry look like it doesn't have
callchains.

Reported-and-Tested-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/20160128122454.GA27446@danjae.kornet
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/stdio/hist.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 76ff46b..691e52c 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -233,7 +233,10 @@ static size_t callchain__fprintf_graph(FILE *fp, struct rb_root *root,
 
 	ret += __callchain__fprintf_graph(fp, root, total_samples,
 					  1, 1, left_margin);
-	ret += fprintf(fp, "\n");
+	if (ret) {
+		/* do not add a blank line if it printed nothing */
+		ret += fprintf(fp, "\n");
+	}
 
 	return ret;
 }