Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751865AbdGRQYl (ORCPT ); Tue, 18 Jul 2017 12:24:41 -0400 Received: from mail-pf0-f195.google.com ([209.85.192.195]:34339 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751725AbdGRQYi (ORCPT ); Tue, 18 Jul 2017 12:24:38 -0400 Date: Wed, 19 Jul 2017 01:23:44 +0900 From: Namhyung Kim To: Taeung Song Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Jiri Olsa , kernel-team@lge.com Subject: Re: [PATCH v2 9/9] perf annotate: Use the sample period when calculating the percentage Message-ID: <20170718162344.GC12241@danjae.aot.lge.com> References: <1499967980-15718-1-git-send-email-treeze.taeung@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1499967980-15718-1-git-send-email-treeze.taeung@gmail.com> User-Agent: Mutt/1.8.2 (2017-04-18) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2220 Lines: 76 On Fri, Jul 14, 2017 at 02:46:20AM +0900, Taeung Song wrote: > Currently the percentages of perf-annotate are calculated > with number of samples, not the sample period. > So fix it to correspond with perf-report using the sample period > for the calculation. Not sure someone still wants the old behavior. Maybe it'd be better to honor --show-nr-samples option. > > Cc: Namhyung Kim > Cc: Jiri Olsa > Signed-off-by: Taeung Song > --- > tools/perf/util/annotate.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index d9bdedf..28a6d11 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -988,7 +988,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, > if (h->total_samples) { It seems checking total_period here is clearer and safer. But I guess it would have same effect 99.9% but still.. > sample->nr_samples = hits; > sample->period = p; > - percent = 100.0 * hits / h->total_samples; > + percent = 100.0 * p / h->total_period; > } > } > > @@ -1730,8 +1730,9 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map, > start = map__rip_2objdump(map, sym->start); > > for (i = 0; i < len; i++) { > - u64 offset, nr_samples; > + u64 offset; > double percent_max = 0.0; > + struct sym_hist_entry sample; > > src_line->nr_pcnt = nr_pcnt; > > @@ -1739,15 +1740,15 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map, > double percent = 0.0; > > h = annotation__histogram(notes, evidx + k); > - nr_samples = h->addr[i].nr_samples; > + sample = h->addr[i]; > > if (h->total_samples) Ditto. Thanks, Namhyung > - percent = 100.0 * nr_samples / h->total_samples; > + percent = 100.0 * sample.period / h->total_period; > > if (percent > percent_max) > percent_max = percent; > src_line->samples[k].percent = percent; > - src_line->samples[k].nr = nr_samples; > + src_line->samples[k].nr = sample.nr_samples; > } > > if (percent_max <= 0.5) > -- > 2.7.4 >