Re: [PATCH] perf, tools, report: Add support for srcfile sort key

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

Em Sun, Aug 09, 2015 at 12:30:49PM +0900, Namhyung Kim escreveu:
> Hi Andi,
> 
> On Sat, Aug 08, 2015 at 04:27:35AM +0200, Andi Kleen wrote:
> > On Fri, Aug 07, 2015 at 09:02:15PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Fri, Aug 07, 2015 at 08:51:45PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Fri, Aug 07, 2015 at 03:54:24PM -0700, Andi Kleen escreveu:
> > > > > From: Andi Kleen <ak@linux.intel.com>
> > > > > 
> > > > > In some cases it's useful to characterize samples by file. This is useful
> > > > > to get a higher level categorization, for example to map cost to
> > > > > subsystems.
> > > > > 
> > > > > Add a srcfile sort key to perf report. It builds on top of the existing
> > > > > srcline support.
> > > > 
> > > > Applied
> > > 
> > > Humm, holding this up a bit, further testing showed some oddities,
> > > fedora21, the width of the column is being limited to the lenght of the
> > > header
> > 
> > Yes I've seen that, I just use -w normally. It also happens with --sort
> > srcline. The column sizing code could probably be somewhat smarter and
> > always allow the last column to become as wide as needed. But that's
> > something that should be done separately; I don't think it belongs
> > into this patch.
> 
> Maybe something like this?

Maybe, but it crashes when right->hists == NULL, perhaps that is some sort of
placeholder? See the value of right (0x7fffffffc0a0)? looks like some static
guard for something, will continue after lunch...

(gdb) fr 2
#2  0x00000000004bb812 in sort__srcline_cmp (left=0x1bfa090, right=0x7fffffffc0a0) at util/sort.c:306
306				hists__new_col_len(right->hists, HISTC_SRCLINE,
(gdb) p right->hists
$7 = (struct hists *) 0x0
(gdb) p left->hists
$8 = (struct hists *) 0x18d66e0
(gdb)

Due to this both "-s srcline" and "-s srcfile" crashes here.

- Arnaldo
 
> 
> 
> >From c052d17ae45ac08a381192191473ed3c4308c0c5 Mon Sep 17 00:00:00 2001
> From: Namhyung Kim <namhyung@kernel.org>
> Date: Sun, 9 Aug 2015 12:28:01 +0900
> Subject: [PATCH] perf tools: Update srcline column length
> 
> It didn't calculate the column length so the default of header length
> used only.  Update the length when we get the srcline actually.
> 
> Cc: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/sort.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 5177088a71d3..85e7e3e75d39 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -291,6 +291,8 @@ sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)
>  			left->srcline = get_srcline(map->dso,
>  					   map__rip_2objdump(map, left->ip),
>  						    left->ms.sym, true);
> +			hists__new_col_len(left->hists, HISTC_SRCLINE,
> +					   strlen(left->srcline));
>  		}
>  	}
>  	if (!right->srcline) {
> @@ -301,6 +303,8 @@ sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)
>  			right->srcline = get_srcline(map->dso,
>  					     map__rip_2objdump(map, right->ip),
>  						     right->ms.sym, true);
> +			hists__new_col_len(right->hists, HISTC_SRCLINE,
> +					   strlen(right->srcline));
>  		}
>  	}
>  	return strcmp(right->srcline, left->srcline);
> -- 
> 2.5.0