Re: [PATCH v2 3/5] perf report: Sort by sampled cycles percent per block for stdio

[Jiri Olsa <jolsa@redhat.com>]

On Mon, Oct 21, 2019 at 04:04:39PM +0200, Jiri Olsa wrote:
> On Mon, Oct 21, 2019 at 02:56:57PM +0800, Jin, Yao wrote:
> 
> SNIP
> 
> > > > Does it seem like what the c2c does?
> > > 
> > > well c2c has its own data output with multiline column titles,
> > > hence it has its own separate dimension stuff, but your code
> > > output is within the standard perf report right? single column
> > > output.. why couldn't you use just sort_entry ?
> > > 
> > > jirka
> > > 
> > 
> > Hi Jiri,
> > 
> > I've being thinking how to use sort_entry but I have some troubles.
> > 
> > In v2, I used "struct perf_hpp_fmt" to pass extra argument. For example,
> > 
> > static int64_t block_cycles_cov_sort(struct perf_hpp_fmt *fmt,
> > 				     struct hist_entry *left,
> > 				     struct hist_entry *right)
> > {
> > 	struct block_fmt *block_fmt = container_of(fmt, ...);
> > 	struct report *rep = block_fmt->rep;
> > 	...
> > }
> > 
> > But if I just use sort_entry, I can't pass extra argument (it's not a good
> > idea to add more fields in struct hist_entry).
> > 
> > int64_t sort__xxx_sort(struct hist_entry *left,
> > 		       struct hist_entry *right)
> > 
> > And for entry print it's similar, I can't pass extra argument in.
> > 
> > In v2,
> > static int block_cycles_pct_entry(struct perf_hpp_fmt *fmt,
> > 				  struct perf_hpp *hpp,
> > 				  struct hist_entry *he)
> > {
> > 	struct block_fmt *block_fmt = container_of(fmt,...);
> > 	struct report *rep = block_fmt->rep;
> > 	...
> > }
> > 
> > But for se_snprintf, I can't pass extra argument in.
> > 
> > hist_entry__xxx_snprintf(struct hist_entry *he, char *bf,
> > 			 size_t size, unsigned int width)
> > 
> > That's why I feel headache for just using the sort_entry. :(
> 
> you might be right, I just want to omit another field output framework ;-) 
> I'm checking on this and will let you know if I find some way

ok, because it's based on branch data the sort entry would not be
convenient.. I'm sending some other comments for the patch

thanks,
jirka