[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6882f3ae-0f8d-5a01-7fd5-5b9f9c93f9ac@linux.intel.com>
Date: Tue, 15 Oct 2019 22:53:18 +0800
From: "Jin, Yao" <yao.jin@...ux.intel.com>
To: Jiri Olsa <jolsa@...hat.com>
Cc: acme@...nel.org, jolsa@...nel.org, peterz@...radead.org,
mingo@...hat.com, alexander.shishkin@...ux.intel.com,
Linux-kernel@...r.kernel.org, ak@...ux.intel.com,
kan.liang@...el.com, yao.jin@...el.com
Subject: Re: [PATCH v2 3/5] perf report: Sort by sampled cycles percent per
block for stdio
On 10/15/2019 4:41 PM, Jiri Olsa wrote:
> On Tue, Oct 15, 2019 at 01:33:48PM +0800, Jin Yao wrote:
>
> SNIP
>
>> +enum {
>> + PERF_HPP_REPORT__BLOCK_TOTAL_CYCLES_COV,
>> + PERF_HPP_REPORT__BLOCK_LBR_CYCLES,
>> + PERF_HPP_REPORT__BLOCK_CYCLES_PCT,
>> + PERF_HPP_REPORT__BLOCK_AVG_CYCLES,
>> + PERF_HPP_REPORT__BLOCK_RANGE,
>> + PERF_HPP_REPORT__BLOCK_DSO,
>> + PERF_HPP_REPORT__BLOCK_MAX_INDEX
>> +};
>> +
>> +static struct block_fmt block_fmts[PERF_HPP_REPORT__BLOCK_MAX_INDEX];
>> +
>> +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.
> SNIP
>
>> +{
>> + struct block_hist *bh = &rep->block_hist;
>> +
>> + get_block_hists(hists, bh, rep);
>> + symbol_conf.report_individual_block = true;
>> + hists__fprintf(&bh->block_hists, true, 0, 0, 0,
>> + stdout, true);
>> + hists__delete_entries(&bh->block_hists);
>> + return 0;
>> +}
>> +
>> static int perf_evlist__tty_browse_hists(struct evlist *evlist,
>> struct report *rep,
>> const char *help)
>> @@ -500,6 +900,12 @@ static int perf_evlist__tty_browse_hists(struct evlist *evlist,
>> continue;
>>
>> hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
>> +
>> + if (rep->total_cycles) {
>> + hists__fprintf_all_blocks(hists, rep);
>
> so this call kicks all the block info setup/count/print, right?
>
Yes, all in this call.
> I thingk it shouldn't be in the output code, but in the code before..
> from what I see you could count block_info counts during the sample
> processing, no?
>
In sample processing, we just get all symbols and account the cycles per
symbol. We need to create/count the block_info at some points after the
sample processing.
Maybe it's not very good to put block info setup/count/print in a call,
but it's really not easy to process the block_info during the sample
processing.
Thanks
Jin Yao
> jirka
>
Powered by blists - more mailing lists