* [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples
@ 2017-03-28 12:12 Taeung Song
2017-03-28 13:48 ` Arnaldo Carvalho de Melo
2017-04-05 5:52 ` [tip:perf/core] " tip-bot for Taeung Song
0 siblings, 2 replies; 8+ messages in thread
From: Taeung Song @ 2017-03-28 12:12 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
Peter Zijlstra, Wang Nan, Masami Hiramatsu, Taeung Song,
Jiri Olsa, Martin Liska
The option 'show-total-period' works fine without a option '-l'.
But if running 'perf annotate --stdio -l --show-total-period',
you can see a problem showing only zero '0' for number of samples.
Before:
$ perf annotate --stdio -l --show-total-period
...
0 : 400816: push %rbp
0 : 400817: mov %rsp,%rbp
0 : 40081a: mov %edi,-0x24(%rbp)
0 : 40081d: mov %rsi,-0x30(%rbp)
0 : 400821: mov -0x24(%rbp),%eax
0 : 400824: mov -0x30(%rbp),%rdx
0 : 400828: mov (%rdx),%esi
0 : 40082a: mov $0x0,%edx
...
The reason is it was missed to set number of samples
of source_line_samples, so set it ordinarily.
After:
$ perf annotate --stdio -l --show-total-period
...
3 : 400816: push %rbp
4 : 400817: mov %rsp,%rbp
0 : 40081a: mov %edi,-0x24(%rbp)
0 : 40081d: mov %rsi,-0x30(%rbp)
1 : 400821: mov -0x24(%rbp),%eax
2 : 400824: mov -0x30(%rbp),%rdx
0 : 400828: mov (%rdx),%esi
1 : 40082a: mov $0x0,%edx
...
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Martin Liska <mliska@suse.cz>
Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
tools/perf/util/annotate.c | 6 ++++--
tools/perf/util/annotate.h | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 11af5f0..a37032b 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1665,7 +1665,7 @@ 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;
+ u64 offset, nr_samples;
double percent_max = 0.0;
src_line->nr_pcnt = nr_pcnt;
@@ -1674,12 +1674,14 @@ 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];
if (h->sum)
- percent = 100.0 * h->addr[i] / h->sum;
+ percent = 100.0 * nr_samples / h->sum;
if (percent > percent_max)
percent_max = percent;
src_line->samples[k].percent = percent;
+ src_line->samples[k].nr = nr_samples;
}
if (percent_max <= 0.5)
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 09776b5..948aa8e 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -98,7 +98,7 @@ struct cyc_hist {
struct source_line_samples {
double percent;
double percent_sum;
- double nr;
+ u64 nr;
};
struct source_line {
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples
2017-03-28 12:12 [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples Taeung Song
@ 2017-03-28 13:48 ` Arnaldo Carvalho de Melo
2017-03-28 14:21 ` Taeung Song
2017-04-05 5:52 ` [tip:perf/core] " tip-bot for Taeung Song
1 sibling, 1 reply; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-03-28 13:48 UTC (permalink / raw)
To: Taeung Song
Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
Peter Zijlstra, Wang Nan, Masami Hiramatsu, Jiri Olsa,
Martin Liska
Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
> The option 'show-total-period' works fine without a option '-l'.
> But if running 'perf annotate --stdio -l --show-total-period',
> you can see a problem showing only zero '0' for number of samples.
If you do that at that point, don't you have to change the TUI not to do
it again?
- Arnaldo
> Before:
> $ perf annotate --stdio -l --show-total-period
> ...
> 0 : 400816: push %rbp
> 0 : 400817: mov %rsp,%rbp
> 0 : 40081a: mov %edi,-0x24(%rbp)
> 0 : 40081d: mov %rsi,-0x30(%rbp)
> 0 : 400821: mov -0x24(%rbp),%eax
> 0 : 400824: mov -0x30(%rbp),%rdx
> 0 : 400828: mov (%rdx),%esi
> 0 : 40082a: mov $0x0,%edx
> ...
>
> The reason is it was missed to set number of samples
> of source_line_samples, so set it ordinarily.
>
> After:
> $ perf annotate --stdio -l --show-total-period
> ...
> 3 : 400816: push %rbp
> 4 : 400817: mov %rsp,%rbp
> 0 : 40081a: mov %edi,-0x24(%rbp)
> 0 : 40081d: mov %rsi,-0x30(%rbp)
> 1 : 400821: mov -0x24(%rbp),%eax
> 2 : 400824: mov -0x30(%rbp),%rdx
> 0 : 400828: mov (%rdx),%esi
> 1 : 40082a: mov $0x0,%edx
> ...
>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Martin Liska <mliska@suse.cz>
> Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
> tools/perf/util/annotate.c | 6 ++++--
> tools/perf/util/annotate.h | 2 +-
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 11af5f0..a37032b 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1665,7 +1665,7 @@ 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;
> + u64 offset, nr_samples;
> double percent_max = 0.0;
>
> src_line->nr_pcnt = nr_pcnt;
> @@ -1674,12 +1674,14 @@ 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];
> if (h->sum)
> - percent = 100.0 * h->addr[i] / h->sum;
> + percent = 100.0 * nr_samples / h->sum;
>
> if (percent > percent_max)
> percent_max = percent;
> src_line->samples[k].percent = percent;
> + src_line->samples[k].nr = nr_samples;
> }
>
> if (percent_max <= 0.5)
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 09776b5..948aa8e 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -98,7 +98,7 @@ struct cyc_hist {
> struct source_line_samples {
> double percent;
> double percent_sum;
> - double nr;
> + u64 nr;
> };
>
> struct source_line {
> --
> 2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples
2017-03-28 13:48 ` Arnaldo Carvalho de Melo
@ 2017-03-28 14:21 ` Taeung Song
2017-03-28 14:26 ` Taeung Song
2017-03-28 15:28 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 8+ messages in thread
From: Taeung Song @ 2017-03-28 14:21 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
Peter Zijlstra, Wang Nan, Masami Hiramatsu, Jiri Olsa,
Martin Liska
On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
>> The option 'show-total-period' works fine without a option '-l'.
>> But if running 'perf annotate --stdio -l --show-total-period',
>> you can see a problem showing only zero '0' for number of samples.
>
> If you do that at that point, don't you have to change the TUI not to do
> it again?
>
> - Arnaldo
>
Yes, there isn't the same problem when using 't' on TUI of 'perf annotate'.
(As you know, 't' is show-total-period option)
In 'annotate' TUI, we can use the 't' functionality without the problem.
Because of the code (util/annotate.c:911 and ui/browsers/annotate.c:412)
as below.
881 double disasm__calc_percent(struct annotation *notes, int evidx,
s64 offset,
882 s64 end, const char **path, u64
*nr_samples)
883 {
...
910 if (h->sum) {
911 *nr_samples = hits;
And,
378 static void annotate_browser__calc_percent(struct annotate_browser
*browser,
379 struct perf_evsel *evsel)
380 {
...
407 bpos->samples[i].percent =
disasm__calc_percent(notes,
408 evsel->idx + i,
409 pos->offset,
410 next ?
next->offset : len,
411 &path, &nr_samples);
412 bpos->samples[i].nr = nr_samples;
And 'perf annotate --stdio --show-total-period' hasn't the problem, too.
Because of the same code (util/annotate.c:911).
But if with '-l', i.e. 'perf annotate --stdio -l --show-total-period',
the problem happen. Because 'nr' of source_line_samples aren't
ordinarily set
in symbol__get_source_line() (util/annotate.c:1634~..)
,so fix it as this patch
Are there something I missed or misunderstand ?
Thanks,
Taeung
>> Before:
>> $ perf annotate --stdio -l --show-total-period
>> ...
>> 0 : 400816: push %rbp
>> 0 : 400817: mov %rsp,%rbp
>> 0 : 40081a: mov %edi,-0x24(%rbp)
>> 0 : 40081d: mov %rsi,-0x30(%rbp)
>> 0 : 400821: mov -0x24(%rbp),%eax
>> 0 : 400824: mov -0x30(%rbp),%rdx
>> 0 : 400828: mov (%rdx),%esi
>> 0 : 40082a: mov $0x0,%edx
>> ...
>>
>> The reason is it was missed to set number of samples
>> of source_line_samples, so set it ordinarily.
>>
>> After:
>> $ perf annotate --stdio -l --show-total-period
>> ...
>> 3 : 400816: push %rbp
>> 4 : 400817: mov %rsp,%rbp
>> 0 : 40081a: mov %edi,-0x24(%rbp)
>> 0 : 40081d: mov %rsi,-0x30(%rbp)
>> 1 : 400821: mov -0x24(%rbp),%eax
>> 2 : 400824: mov -0x30(%rbp),%rdx
>> 0 : 400828: mov (%rdx),%esi
>> 1 : 40082a: mov $0x0,%edx
>> ...
>>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Martin Liska <mliska@suse.cz>
>> Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>> tools/perf/util/annotate.c | 6 ++++--
>> tools/perf/util/annotate.h | 2 +-
>> 2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>> index 11af5f0..a37032b 100644
>> --- a/tools/perf/util/annotate.c
>> +++ b/tools/perf/util/annotate.c
>> @@ -1665,7 +1665,7 @@ 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;
>> + u64 offset, nr_samples;
>> double percent_max = 0.0;
>>
>> src_line->nr_pcnt = nr_pcnt;
>> @@ -1674,12 +1674,14 @@ 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];
>> if (h->sum)
>> - percent = 100.0 * h->addr[i] / h->sum;
>> + percent = 100.0 * nr_samples / h->sum;
>>
>> if (percent > percent_max)
>> percent_max = percent;
>> src_line->samples[k].percent = percent;
>> + src_line->samples[k].nr = nr_samples;
>> }
>>
>> if (percent_max <= 0.5)
>> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
>> index 09776b5..948aa8e 100644
>> --- a/tools/perf/util/annotate.h
>> +++ b/tools/perf/util/annotate.h
>> @@ -98,7 +98,7 @@ struct cyc_hist {
>> struct source_line_samples {
>> double percent;
>> double percent_sum;
>> - double nr;
>> + u64 nr;
>> };
>>
>> struct source_line {
>> --
>> 2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples
2017-03-28 14:21 ` Taeung Song
@ 2017-03-28 14:26 ` Taeung Song
2017-03-28 15:28 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 8+ messages in thread
From: Taeung Song @ 2017-03-28 14:26 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
Peter Zijlstra, Wang Nan, Masami Hiramatsu, Jiri Olsa,
Martin Liska
On 03/28/2017 11:21 PM, Taeung Song wrote:
>
>
> On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
>>> The option 'show-total-period' works fine without a option '-l'.
>>> But if running 'perf annotate --stdio -l --show-total-period',
>>> you can see a problem showing only zero '0' for number of samples.
>>
>> If you do that at that point, don't you have to change the TUI not to do
>> it again?
>>
>> - Arnaldo
>>
>
> Yes, there isn't the same problem when using 't' on TUI of 'perf annotate'.
Sorry, missay the reply.
I don't change the TUI to fix same problem of this patch.
The reason are like below..
> (As you know, 't' is show-total-period option)
>
> In 'annotate' TUI, we can use the 't' functionality without the problem.
> Because of the code (util/annotate.c:911 and ui/browsers/annotate.c:412)
> as below.
>
> 881 double disasm__calc_percent(struct annotation *notes, int evidx,
> s64 offset,
> 882 s64 end, const char **path, u64
> *nr_samples)
> 883 {
> ...
> 910 if (h->sum) {
> 911 *nr_samples = hits;
>
> And,
>
> 378 static void annotate_browser__calc_percent(struct annotate_browser
> *browser,
> 379 struct perf_evsel *evsel)
> 380 {
> ...
> 407 bpos->samples[i].percent =
> disasm__calc_percent(notes,
> 408 evsel->idx + i,
> 409 pos->offset,
> 410 next ? next->offset
> : len,
> 411 &path, &nr_samples);
> 412 bpos->samples[i].nr = nr_samples;
>
>
> And 'perf annotate --stdio --show-total-period' hasn't the problem, too.
> Because of the same code (util/annotate.c:911).
>
>
> But if with '-l', i.e. 'perf annotate --stdio -l --show-total-period',
> the problem happen. Because 'nr' of source_line_samples aren't
> ordinarily set
> in symbol__get_source_line() (util/annotate.c:1634~..)
> ,so fix it as this patch
>
> Are there something I missed or misunderstand ?
>
>
> Thanks,
> Taeung
>
>
>>> Before:
>>> $ perf annotate --stdio -l --show-total-period
>>> ...
>>> 0 : 400816: push %rbp
>>> 0 : 400817: mov %rsp,%rbp
>>> 0 : 40081a: mov %edi,-0x24(%rbp)
>>> 0 : 40081d: mov %rsi,-0x30(%rbp)
>>> 0 : 400821: mov -0x24(%rbp),%eax
>>> 0 : 400824: mov -0x30(%rbp),%rdx
>>> 0 : 400828: mov (%rdx),%esi
>>> 0 : 40082a: mov $0x0,%edx
>>> ...
>>>
>>> The reason is it was missed to set number of samples
>>> of source_line_samples, so set it ordinarily.
>>>
>>> After:
>>> $ perf annotate --stdio -l --show-total-period
>>> ...
>>> 3 : 400816: push %rbp
>>> 4 : 400817: mov %rsp,%rbp
>>> 0 : 40081a: mov %edi,-0x24(%rbp)
>>> 0 : 40081d: mov %rsi,-0x30(%rbp)
>>> 1 : 400821: mov -0x24(%rbp),%eax
>>> 2 : 400824: mov -0x30(%rbp),%rdx
>>> 0 : 400828: mov (%rdx),%esi
>>> 1 : 40082a: mov $0x0,%edx
>>> ...
>>>
>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>> Cc: Jiri Olsa <jolsa@redhat.com>
>>> Cc: Martin Liska <mliska@suse.cz>
>>> Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples
>>> with --show-total-period")
>>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>>> ---
>>> tools/perf/util/annotate.c | 6 ++++--
>>> tools/perf/util/annotate.h | 2 +-
>>> 2 files changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>>> index 11af5f0..a37032b 100644
>>> --- a/tools/perf/util/annotate.c
>>> +++ b/tools/perf/util/annotate.c
>>> @@ -1665,7 +1665,7 @@ 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;
>>> + u64 offset, nr_samples;
>>> double percent_max = 0.0;
>>>
>>> src_line->nr_pcnt = nr_pcnt;
>>> @@ -1674,12 +1674,14 @@ 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];
>>> if (h->sum)
>>> - percent = 100.0 * h->addr[i] / h->sum;
>>> + percent = 100.0 * nr_samples / h->sum;
>>>
>>> if (percent > percent_max)
>>> percent_max = percent;
>>> src_line->samples[k].percent = percent;
>>> + src_line->samples[k].nr = nr_samples;
>>> }
>>>
>>> if (percent_max <= 0.5)
>>> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
>>> index 09776b5..948aa8e 100644
>>> --- a/tools/perf/util/annotate.h
>>> +++ b/tools/perf/util/annotate.h
>>> @@ -98,7 +98,7 @@ struct cyc_hist {
>>> struct source_line_samples {
>>> double percent;
>>> double percent_sum;
>>> - double nr;
>>> + u64 nr;
>>> };
>>>
>>> struct source_line {
>>> --
>>> 2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples
2017-03-28 14:21 ` Taeung Song
2017-03-28 14:26 ` Taeung Song
@ 2017-03-28 15:28 ` Arnaldo Carvalho de Melo
2017-03-28 16:15 ` Taeung Song
1 sibling, 1 reply; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-03-28 15:28 UTC (permalink / raw)
To: Taeung Song
Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Namhyung Kim,
Ingo Molnar, Peter Zijlstra, Wang Nan, Masami Hiramatsu,
Jiri Olsa, Martin Liska
Em Tue, Mar 28, 2017 at 11:21:33PM +0900, Taeung Song escreveu:
>
>
> On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
> > > The option 'show-total-period' works fine without a option '-l'.
> > > But if running 'perf annotate --stdio -l --show-total-period',
> > > you can see a problem showing only zero '0' for number of samples.
> >
> > If you do that at that point, don't you have to change the TUI not to do
> > it again?
> >
> > - Arnaldo
> >
>
> Yes, there isn't the same problem when using 't' on TUI of 'perf annotate'.
> (As you know, 't' is show-total-period option)
Yeah, there is no problem on the TUI, but since you do it in a function
taht the TUI uses, it may not be needed to do this in
tools/perf/util/annotate.c _and_ in tools/perf/ui/browsers/annotate.c,
right?
- Arnaldo
> In 'annotate' TUI, we can use the 't' functionality without the problem.
> Because of the code (util/annotate.c:911 and ui/browsers/annotate.c:412) as
> below.
>
> 881 double disasm__calc_percent(struct annotation *notes, int evidx, s64
> offset,
> 882 s64 end, const char **path, u64
> *nr_samples)
> 883 {
> ...
> 910 if (h->sum) {
> 911 *nr_samples = hits;
>
> And,
>
> 378 static void annotate_browser__calc_percent(struct annotate_browser
> *browser,
> 379 struct perf_evsel *evsel)
> 380 {
> ...
> 407 bpos->samples[i].percent =
> disasm__calc_percent(notes,
> 408 evsel->idx + i,
> 409 pos->offset,
> 410 next ? next->offset :
> len,
> 411 &path, &nr_samples);
> 412 bpos->samples[i].nr = nr_samples;
>
>
> And 'perf annotate --stdio --show-total-period' hasn't the problem, too.
> Because of the same code (util/annotate.c:911).
>
>
> But if with '-l', i.e. 'perf annotate --stdio -l --show-total-period',
> the problem happen. Because 'nr' of source_line_samples aren't ordinarily
> set
> in symbol__get_source_line() (util/annotate.c:1634~..)
> ,so fix it as this patch
>
> Are there something I missed or misunderstand ?
>
>
> Thanks,
> Taeung
>
>
> > > Before:
> > > $ perf annotate --stdio -l --show-total-period
> > > ...
> > > 0 : 400816: push %rbp
> > > 0 : 400817: mov %rsp,%rbp
> > > 0 : 40081a: mov %edi,-0x24(%rbp)
> > > 0 : 40081d: mov %rsi,-0x30(%rbp)
> > > 0 : 400821: mov -0x24(%rbp),%eax
> > > 0 : 400824: mov -0x30(%rbp),%rdx
> > > 0 : 400828: mov (%rdx),%esi
> > > 0 : 40082a: mov $0x0,%edx
> > > ...
> > >
> > > The reason is it was missed to set number of samples
> > > of source_line_samples, so set it ordinarily.
> > >
> > > After:
> > > $ perf annotate --stdio -l --show-total-period
> > > ...
> > > 3 : 400816: push %rbp
> > > 4 : 400817: mov %rsp,%rbp
> > > 0 : 40081a: mov %edi,-0x24(%rbp)
> > > 0 : 40081d: mov %rsi,-0x30(%rbp)
> > > 1 : 400821: mov -0x24(%rbp),%eax
> > > 2 : 400824: mov -0x30(%rbp),%rdx
> > > 0 : 400828: mov (%rdx),%esi
> > > 1 : 40082a: mov $0x0,%edx
> > > ...
> > >
> > > Cc: Namhyung Kim <namhyung@kernel.org>
> > > Cc: Jiri Olsa <jolsa@redhat.com>
> > > Cc: Martin Liska <mliska@suse.cz>
> > > Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
> > > Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> > > ---
> > > tools/perf/util/annotate.c | 6 ++++--
> > > tools/perf/util/annotate.h | 2 +-
> > > 2 files changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > > index 11af5f0..a37032b 100644
> > > --- a/tools/perf/util/annotate.c
> > > +++ b/tools/perf/util/annotate.c
> > > @@ -1665,7 +1665,7 @@ 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;
> > > + u64 offset, nr_samples;
> > > double percent_max = 0.0;
> > >
> > > src_line->nr_pcnt = nr_pcnt;
> > > @@ -1674,12 +1674,14 @@ 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];
> > > if (h->sum)
> > > - percent = 100.0 * h->addr[i] / h->sum;
> > > + percent = 100.0 * nr_samples / h->sum;
> > >
> > > if (percent > percent_max)
> > > percent_max = percent;
> > > src_line->samples[k].percent = percent;
> > > + src_line->samples[k].nr = nr_samples;
> > > }
> > >
> > > if (percent_max <= 0.5)
> > > diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> > > index 09776b5..948aa8e 100644
> > > --- a/tools/perf/util/annotate.h
> > > +++ b/tools/perf/util/annotate.h
> > > @@ -98,7 +98,7 @@ struct cyc_hist {
> > > struct source_line_samples {
> > > double percent;
> > > double percent_sum;
> > > - double nr;
> > > + u64 nr;
> > > };
> > >
> > > struct source_line {
> > > --
> > > 2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples
2017-03-28 15:28 ` Arnaldo Carvalho de Melo
@ 2017-03-28 16:15 ` Taeung Song
2017-03-29 8:53 ` Taeung Song
0 siblings, 1 reply; 8+ messages in thread
From: Taeung Song @ 2017-03-28 16:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
Peter Zijlstra, Wang Nan, Masami Hiramatsu, Jiri Olsa,
Martin Liska
On 03/29/2017 12:28 AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 28, 2017 at 11:21:33PM +0900, Taeung Song escreveu:
>>
>>
>> On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
>>>> The option 'show-total-period' works fine without a option '-l'.
>>>> But if running 'perf annotate --stdio -l --show-total-period',
>>>> you can see a problem showing only zero '0' for number of samples.
>>>
>>> If you do that at that point, don't you have to change the TUI not to do
>>> it again?
>>>
>>> - Arnaldo
>>>
>>
>> Yes, there isn't the same problem when using 't' on TUI of 'perf annotate'.
>> (As you know, 't' is show-total-period option)
>
> Yeah, there is no problem on the TUI, but since you do it in a function
> taht the TUI uses, it may not be needed to do this in
No, TUI don't use the function I modified in this patch at all.
(i.e. symbol__get_source_line())
Only symbol__tty_annotate() use symbol__get_source_line().
(As you know, annotate TUI use symbol__tui_annotate() )
> tools/perf/util/annotate.c _and_ in tools/perf/ui/browsers/annotate.c,
> right?
>
> - Arnaldo
Are there something I missed or misunderstand ?
Thanks,
Taeung
>
>> In 'annotate' TUI, we can use the 't' functionality without the problem.
>> Because of the code (util/annotate.c:911 and ui/browsers/annotate.c:412) as
>> below.
>>
>> 881 double disasm__calc_percent(struct annotation *notes, int evidx, s64
>> offset,
>> 882 s64 end, const char **path, u64
>> *nr_samples)
>> 883 {
>> ...
>> 910 if (h->sum) {
>> 911 *nr_samples = hits;
>>
>> And,
>>
>> 378 static void annotate_browser__calc_percent(struct annotate_browser
>> *browser,
>> 379 struct perf_evsel *evsel)
>> 380 {
>> ...
>> 407 bpos->samples[i].percent =
>> disasm__calc_percent(notes,
>> 408 evsel->idx + i,
>> 409 pos->offset,
>> 410 next ? next->offset :
>> len,
>> 411 &path, &nr_samples);
>> 412 bpos->samples[i].nr = nr_samples;
>>
>>
>> And 'perf annotate --stdio --show-total-period' hasn't the problem, too.
>> Because of the same code (util/annotate.c:911).
>>
>>
>> But if with '-l', i.e. 'perf annotate --stdio -l --show-total-period',
>> the problem happen. Because 'nr' of source_line_samples aren't ordinarily
>> set
>> in symbol__get_source_line() (util/annotate.c:1634~..)
>> ,so fix it as this patch
>>
>> Are there something I missed or misunderstand ?
>>
>>
>> Thanks,
>> Taeung
>>
>>
>>>> Before:
>>>> $ perf annotate --stdio -l --show-total-period
>>>> ...
>>>> 0 : 400816: push %rbp
>>>> 0 : 400817: mov %rsp,%rbp
>>>> 0 : 40081a: mov %edi,-0x24(%rbp)
>>>> 0 : 40081d: mov %rsi,-0x30(%rbp)
>>>> 0 : 400821: mov -0x24(%rbp),%eax
>>>> 0 : 400824: mov -0x30(%rbp),%rdx
>>>> 0 : 400828: mov (%rdx),%esi
>>>> 0 : 40082a: mov $0x0,%edx
>>>> ...
>>>>
>>>> The reason is it was missed to set number of samples
>>>> of source_line_samples, so set it ordinarily.
>>>>
>>>> After:
>>>> $ perf annotate --stdio -l --show-total-period
>>>> ...
>>>> 3 : 400816: push %rbp
>>>> 4 : 400817: mov %rsp,%rbp
>>>> 0 : 40081a: mov %edi,-0x24(%rbp)
>>>> 0 : 40081d: mov %rsi,-0x30(%rbp)
>>>> 1 : 400821: mov -0x24(%rbp),%eax
>>>> 2 : 400824: mov -0x30(%rbp),%rdx
>>>> 0 : 400828: mov (%rdx),%esi
>>>> 1 : 40082a: mov $0x0,%edx
>>>> ...
>>>>
>>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>>> Cc: Jiri Olsa <jolsa@redhat.com>
>>>> Cc: Martin Liska <mliska@suse.cz>
>>>> Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
>>>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>>>> ---
>>>> tools/perf/util/annotate.c | 6 ++++--
>>>> tools/perf/util/annotate.h | 2 +-
>>>> 2 files changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>>>> index 11af5f0..a37032b 100644
>>>> --- a/tools/perf/util/annotate.c
>>>> +++ b/tools/perf/util/annotate.c
>>>> @@ -1665,7 +1665,7 @@ 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;
>>>> + u64 offset, nr_samples;
>>>> double percent_max = 0.0;
>>>>
>>>> src_line->nr_pcnt = nr_pcnt;
>>>> @@ -1674,12 +1674,14 @@ 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];
>>>> if (h->sum)
>>>> - percent = 100.0 * h->addr[i] / h->sum;
>>>> + percent = 100.0 * nr_samples / h->sum;
>>>>
>>>> if (percent > percent_max)
>>>> percent_max = percent;
>>>> src_line->samples[k].percent = percent;
>>>> + src_line->samples[k].nr = nr_samples;
>>>> }
>>>>
>>>> if (percent_max <= 0.5)
>>>> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
>>>> index 09776b5..948aa8e 100644
>>>> --- a/tools/perf/util/annotate.h
>>>> +++ b/tools/perf/util/annotate.h
>>>> @@ -98,7 +98,7 @@ struct cyc_hist {
>>>> struct source_line_samples {
>>>> double percent;
>>>> double percent_sum;
>>>> - double nr;
>>>> + u64 nr;
>>>> };
>>>>
>>>> struct source_line {
>>>> --
>>>> 2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples
2017-03-28 16:15 ` Taeung Song
@ 2017-03-29 8:53 ` Taeung Song
0 siblings, 0 replies; 8+ messages in thread
From: Taeung Song @ 2017-03-29 8:53 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
Peter Zijlstra, Wang Nan, Masami Hiramatsu, Jiri Olsa,
Martin Liska
Arnaldo,
I think we had a little communication problem.
So would you mind if I finally sum up this situation ?
Currently there are the two calculation setting for 'number of samples'.
(At util/annotate.c, not TUI code ui/browsers/annotate.c)
A) disasm__calc_percent() -> util/annotate.c:881~
B) symbol__get_source_line() -> util/annotate.c:1634~
If testing them as the three cases,
1) $ perf annotate --stdio -l --show-total-period
2) $ perf annotate --stdio --show-total-period
3) $ perf annotate # using 'k' on TUI
The result is,
1) NG
2) OK
3) OK
The problem of '1)' is to just show '0' zero values for number of samples.
Because 'B)' has a bug missing 'nr' of source_line_samples.
So I fix it.
the case '1)' uses 'B)' and 'A)'
the case '2)' uses only 'A)'
the case '3)' uses only 'A)'
I read and analyzed all about 'number of samples' of perf-anntoate.
with the cset 0c4a5bcea460 you gave me.
This is my conclusion. What do you think about it ?
Thanks,
Taeung
NOTE: 'A)'(i.e. disasm__calc_percent()) has two alternative logic
when using '-l' option or not as below.
888 if (src_line) {
...
903 } else {
...
914 }
On 03/29/2017 01:15 AM, Taeung Song wrote:
>
>
> On 03/29/2017 12:28 AM, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Mar 28, 2017 at 11:21:33PM +0900, Taeung Song escreveu:
>>>
>>>
>>> On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote:
>>>> Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu:
>>>>> The option 'show-total-period' works fine without a option '-l'.
>>>>> But if running 'perf annotate --stdio -l --show-total-period',
>>>>> you can see a problem showing only zero '0' for number of samples.
>>>>
>>>> If you do that at that point, don't you have to change the TUI not
>>>> to do
>>>> it again?
>>>>
>>>> - Arnaldo
>>>>
>>>
>>> Yes, there isn't the same problem when using 't' on TUI of 'perf
>>> annotate'.
>>> (As you know, 't' is show-total-period option)
>>
>> Yeah, there is no problem on the TUI, but since you do it in a function
>> taht the TUI uses, it may not be needed to do this in
>
> No, TUI don't use the function I modified in this patch at all.
> (i.e. symbol__get_source_line())
>
> Only symbol__tty_annotate() use symbol__get_source_line().
>
> (As you know, annotate TUI use symbol__tui_annotate() )
>
>> tools/perf/util/annotate.c _and_ in tools/perf/ui/browsers/annotate.c,
>> right?
>>
>> - Arnaldo
>
>
> Are there something I missed or misunderstand ?
>
> Thanks,
> Taeung
>
SNIP
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:perf/core] perf annotate: Fix missing number of samples for source_line_samples
2017-03-28 12:12 [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples Taeung Song
2017-03-28 13:48 ` Arnaldo Carvalho de Melo
@ 2017-04-05 5:52 ` tip-bot for Taeung Song
1 sibling, 0 replies; 8+ messages in thread
From: tip-bot for Taeung Song @ 2017-04-05 5:52 UTC (permalink / raw)
To: linux-tip-commits
Cc: namhyung, acme, mhiramat, tglx, mingo, linux-kernel,
treeze.taeung, jolsa, mliska, peterz, wangnan0, hpa
Commit-ID: 99094a5e941fe88d95cbd594e6a41bee24003ecb
Gitweb: http://git.kernel.org/tip/99094a5e941fe88d95cbd594e6a41bee24003ecb
Author: Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Tue, 28 Mar 2017 21:12:05 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 4 Apr 2017 21:08:00 -0300
perf annotate: Fix missing number of samples for source_line_samples
The option 'show-total-period' works fine without a option '-l'. But if
running 'perf annotate --stdio -l --show-total-period', you can see a
problem showing only zero '0' for number of samples.
Before:
$ perf annotate --stdio -l --show-total-period
...
0 : 400816: push %rbp
0 : 400817: mov %rsp,%rbp
0 : 40081a: mov %edi,-0x24(%rbp)
0 : 40081d: mov %rsi,-0x30(%rbp)
0 : 400821: mov -0x24(%rbp),%eax
0 : 400824: mov -0x30(%rbp),%rdx
0 : 400828: mov (%rdx),%esi
0 : 40082a: mov $0x0,%edx
...
The reason is it was missed to set number of samples of
source_line_samples, so set it ordinarily.
After:
$ perf annotate --stdio -l --show-total-period
...
3 : 400816: push %rbp
4 : 400817: mov %rsp,%rbp
0 : 40081a: mov %edi,-0x24(%rbp)
0 : 40081d: mov %rsi,-0x30(%rbp)
1 : 400821: mov -0x24(%rbp),%eax
2 : 400824: mov -0x30(%rbp),%rdx
0 : 400828: mov (%rdx),%esi
1 : 40082a: mov $0x0,%edx
...
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Martin Liska <mliska@suse.cz>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Fixes: 0c4a5bcea460 ("perf annotate: Display total number of samples with --show-total-period")
Link: http://lkml.kernel.org/r/1490703125-13643-1-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/annotate.c | 6 ++++--
tools/perf/util/annotate.h | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 11af5f0..a37032b 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1665,7 +1665,7 @@ 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;
+ u64 offset, nr_samples;
double percent_max = 0.0;
src_line->nr_pcnt = nr_pcnt;
@@ -1674,12 +1674,14 @@ 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];
if (h->sum)
- percent = 100.0 * h->addr[i] / h->sum;
+ percent = 100.0 * nr_samples / h->sum;
if (percent > percent_max)
percent_max = percent;
src_line->samples[k].percent = percent;
+ src_line->samples[k].nr = nr_samples;
}
if (percent_max <= 0.5)
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 09776b5..948aa8e 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -98,7 +98,7 @@ struct cyc_hist {
struct source_line_samples {
double percent;
double percent_sum;
- double nr;
+ u64 nr;
};
struct source_line {
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-04-05 5:56 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-28 12:12 [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples Taeung Song
2017-03-28 13:48 ` Arnaldo Carvalho de Melo
2017-03-28 14:21 ` Taeung Song
2017-03-28 14:26 ` Taeung Song
2017-03-28 15:28 ` Arnaldo Carvalho de Melo
2017-03-28 16:15 ` Taeung Song
2017-03-29 8:53 ` Taeung Song
2017-04-05 5:52 ` [tip:perf/core] " tip-bot for Taeung Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).