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

[Jin, Yao]

On 10/16/2019 8:53 PM, Jiri Olsa wrote:
> On Wed, Oct 16, 2019 at 06:51:07PM +0800, Jin, Yao wrote:
> > On 10/16/2019 6:15 PM, Jiri Olsa wrote:
> > > On Tue, Oct 15, 2019 at 10:53:18PM +0800, Jin, Yao wrote:
> > >
> > > SNIP
> > >
> > > > > > +static struct block_header_column{
> > > > > > +	const char *name;
> > > > > > +	int width;
> > > > > > +} block_columns[PERF_HPP_REPORT__BLOCK_MAX_INDEX] = {
> > > > > > +	[PERF_HPP_REPORT__BLOCK_TOTAL_CYCLES_COV] = {
> > > > > > +		.name = "Sampled Cycles%",
> > > > > > +		.width = 15,
> > > > > > +	},
> > > > > > +	[PERF_HPP_REPORT__BLOCK_LBR_CYCLES] = {
> > > > > > +		.name = "Sampled Cycles",
> > > > > > +		.width = 14,
> > > > > > +	},
> > > > > > +	[PERF_HPP_REPORT__BLOCK_CYCLES_PCT] = {
> > > > > > +		.name = "Avg Cycles%",
> > > > > > +		.width = 11,
> > > > > > +	},
> > > > > > +	[PERF_HPP_REPORT__BLOCK_AVG_CYCLES] = {
> > > > > > +		.name = "Avg Cycles",
> > > > > > +		.width = 10,
> > > > > > +	},
> > > > > > +	[PERF_HPP_REPORT__BLOCK_RANGE] = {
> > > > > > +		.name = "[Program Block Range]",
> > > > > > +		.width = 70,
> > > > > > +	},
> > > > > > +	[PERF_HPP_REPORT__BLOCK_DSO] = {
> > > > > > +		.name = "Shared Object",
> > > > > > +		.width = 20,
> > > > > > +	}
> > > > > >     };
> > > > >
> > > > > so we already have support for multiple columns,
> > > > > why don't you add those as 'struct sort_entry' objects?
> > > >
> > > > For 'struct sort_entry' objects, do you mean I should reuse the "sort_dso"
> > > > which has been implemented yet in util/sort.c?
> > > >
> > > > For other columns, it looks we can't reuse the existing sort_entry objects.
> > >
> > > I did not mean reuse, just add new sort entries
> > > to current sort framework
> >
> > 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. :(

Thanks
Jin Yao