Messages in this thread | | | Subject | Re: [PATCH] perf annotate: Remove hist__account_cycles from callback | From | "Jin, Yao" <> | Date | Thu, 14 Mar 2019 22:17:07 +0800 |
| |
On 3/14/2019 8:04 PM, Jiri Olsa wrote: > On Wed, Mar 13, 2019 at 04:22:54AM +0800, Jin Yao wrote: >> The hist__account_cycles is executed when the hist_iter__branch_callback >> is called. But it looks it's not necessary. In hist__account_cycles, it >> already walks on all branch entries. >> >> This patch moves the hist__account_cycles out of callback, now the data >> processing is much faster than before. >> >> For example, >> perf record -b ... >> perf annotate >> >> The before/after output should be no change. > > so in the old code it was called multiple times per sample, > while after your change it's called just once per sample > > it increases cycles stats for symbol, so it's strange > the behaviour is the same.. could you please exaplin this > in more detail? >
In __symbol__account_cycles, ch[offset].num_aggr++; ch[offset].cycles_aggr += cycles;
In annotation__compute_ipc, al = notes->offsets[offset]; al->cycles = ch->cycles_aggr / ch->num_aggr;
So the num_aggr and cycles_aggr are both increased n times, then the final result is the same.
>> >> Signed-off-by: Jin Yao <yao.jin@linux.intel.com> >> --- >> tools/perf/builtin-annotate.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c >> index 67f9d9f..77deb3a 100644 >> --- a/tools/perf/builtin-annotate.c >> +++ b/tools/perf/builtin-annotate.c >> @@ -159,8 +159,6 @@ static int hist_iter__branch_callback(struct hist_entry_iter *iter, >> struct perf_evsel *evsel = iter->evsel; >> int err; >> >> - hist__account_cycles(sample->branch_stack, al, sample, false); >> - > > there's similar code in perf report, could you please cover > that as well? >
Sure, let me have a try.
Thanks Jin Yao
> thanks, > jirka > >> bi = he->branch_info; >> err = addr_map_symbol__inc_samples(&bi->from, sample, evsel); >> >> @@ -199,6 +197,8 @@ static int process_branch_callback(struct perf_evsel *evsel, >> if (a.map != NULL) >> a.map->dso->hit = 1; >> >> + hist__account_cycles(sample->branch_stack, al, sample, false); >> + >> ret = hist_entry_iter__add(&iter, &a, PERF_MAX_STACK_DEPTH, ann); >> return ret; >> } >> -- >> 2.7.4 >>
| |