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

["Jin, Yao" <yao.jin@linux.intel.com>]

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