Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1323452 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains  also Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-02-01 21:20 +0100
    Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains  also Namhyung Kim <namhyung@kernel.org> - 2016-02-02 14:10 +0100
      Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains  also Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-02-02 15:00 +0100
        Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains  also Namhyung Kim <namhyung@kernel.org> - 2016-02-02 15:20 +0100
          Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains  also Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-02-02 15:30 +0100
            Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains  also Namhyung Kim <namhyung@kernel.org> - 2016-02-02 15:40 +0100
              Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains  also Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-02-02 16:00 +0100
                Re: [PATCH 03/10] perf report: Apply --percent-limit to callchains  also Namhyung Kim <namhyung@kernel.org> - 2016-02-02 16:10 +0100

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

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

[toc] | [next] | [standalone]

#1324059

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

[toc] | [prev] | [next] | [standalone]

#1324105

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

[toc] | [prev] | [next] | [standalone]

#1324116

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

[toc] | [prev] | [next] | [standalone]

#1324119

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

[toc] | [prev] | [next] | [standalone]

#1324136

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

[toc] | [prev] | [next] | [standalone]

#1324179

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

[toc] | [prev] | [next] | [standalone]

#1324180

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web