Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1586031 > unrolled thread
| Started by | Taeung Song <treeze.taeung@gmail.com> |
|---|---|
| First post | 2017-02-22 11:10 +0100 |
| Last post | 2017-02-22 17:40 +0100 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 2/4] perf annotate: Align filename:linenr and more correct summary Taeung Song <treeze.taeung@gmail.com> - 2017-02-22 11:10 +0100
Re: [PATCH 2/4] perf annotate: Align filename:linenr and more correct summary Namhyung Kim <namhyung@kernel.org> - 2017-02-22 12:20 +0100
Re: [PATCH 2/4] perf annotate: Align filename:linenr and more correct summary Namhyung Kim <namhyung@kernel.org> - 2017-02-22 12:30 +0100
Re: [PATCH 2/4] perf annotate: Align filename:linenr and more correct summary Taeung Song <treeze.taeung@gmail.com> - 2017-02-22 17:40 +0100
| From | Taeung Song <treeze.taeung@gmail.com> |
|---|---|
| Date | 2017-02-22 11:10 +0100 |
| Subject | [PATCH 2/4] perf annotate: Align filename:linenr and more correct summary |
| Message-ID | <tdC1X-8u2-9@gated-at.bofh.it> |
In the stdio interface, currently 'filename:linenr' infos
are confusedly printed in the intervals of assembly code.
So fix it.
The cause was a 0.5% filter of if statement. After fixed,
additionally summary of overhead per srcline is more correct.
Before:
# perf annotate --stdio -l
Sorted summary for file /home/taeung/workspace/perf-test/test
----------------------------------------------
36.57 test.c:38
28.72 test.c:37
...
Percent | Source code & Disassembly of test ...
...
0.21 : 400816: push %rbp
test.c:26 1.86 : 400817: mov %rsp,%rbp
0.21 : 40081a: mov %edi,-0x24(%rbp)
0.21 : 40081d: mov %rsi,-0x30(%rbp)
After:
# perf annotate --stdio -l
Sorted summary for file /home/taeung/workspace/perf-test/test
----------------------------------------------
37.40 test.c:38
29.34 test.c:37
...
Percent | Source code & Disassembly of test ...
...
test.c:26
0.21 : 400816: push %rbp
1.86 : 400817: mov %rsp,%rbp
0.21 : 40081a: mov %edi,-0x24(%rbp)
0.21 : 40081d: mov %rsi,-0x30(%rbp)
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
tools/perf/util/annotate.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index bc54e41..9d0aa50 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1002,7 +1002,6 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
int max_lines, struct disasm_line *queue)
{
static const char *prev_line;
- static const char *prev_color;
if (dl->offset != -1) {
const char *path = NULL;
@@ -1059,17 +1058,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
color = get_percent_color(max_percent);
- /*
- * Also color the filename and line if needed, with
- * the same color than the percentage. Don't print it
- * twice for close colored addr with the same filename:line
- */
if (path) {
- if (!prev_line || strcmp(prev_line, path)
- || color != prev_color) {
- color_fprintf(stdout, color, " %s", path);
+ if (!prev_line || strcmp(prev_line, path)) {
+ fprintf(stdout, " %s\n", path);
prev_line = path;
- prev_color = color;
}
}
@@ -1650,14 +1642,9 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
percent_max = src_line->samples[k].percent;
}
- if (percent_max <= 0.5)
- goto next;
-
offset = start + i;
src_line->path = get_srcline(map->dso, offset, NULL, false);
insert_source_line(&tmp_root, src_line);
-
- next:
src_line = (void *)src_line + sizeof_src_line;
}
--
2.7.4
[toc] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2017-02-22 12:20 +0100 |
| Message-ID | <tdD7J-R8-25@gated-at.bofh.it> |
| In reply to | #1586031 |
On Wed, Feb 22, 2017 at 7:08 PM, Taeung Song <treeze.taeung@gmail.com> wrote:
> In the stdio interface, currently 'filename:linenr' infos
> are confusedly printed in the intervals of assembly code.
> So fix it.
>
> The cause was a 0.5% filter of if statement. After fixed,
> additionally summary of overhead per srcline is more correct.
This patch does two things and fails to explain why.
Thanks,
Namhyung
>
> Before:
>
> # perf annotate --stdio -l
>
> Sorted summary for file /home/taeung/workspace/perf-test/test
> ----------------------------------------------
>
> 36.57 test.c:38
> 28.72 test.c:37
>
> ...
>
> Percent | Source code & Disassembly of test ...
>
> ...
>
> 0.21 : 400816: push %rbp
> test.c:26 1.86 : 400817: mov %rsp,%rbp
> 0.21 : 40081a: mov %edi,-0x24(%rbp)
> 0.21 : 40081d: mov %rsi,-0x30(%rbp)
>
> After:
>
> # perf annotate --stdio -l
>
> Sorted summary for file /home/taeung/workspace/perf-test/test
> ----------------------------------------------
>
> 37.40 test.c:38
> 29.34 test.c:37
>
> ...
>
> Percent | Source code & Disassembly of test ...
>
> ...
>
> test.c:26
> 0.21 : 400816: push %rbp
> 1.86 : 400817: mov %rsp,%rbp
> 0.21 : 40081a: mov %edi,-0x24(%rbp)
> 0.21 : 40081d: mov %rsi,-0x30(%rbp)
>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
> tools/perf/util/annotate.c | 17 ++---------------
> 1 file changed, 2 insertions(+), 15 deletions(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index bc54e41..9d0aa50 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1002,7 +1002,6 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
> int max_lines, struct disasm_line *queue)
> {
> static const char *prev_line;
> - static const char *prev_color;
>
> if (dl->offset != -1) {
> const char *path = NULL;
> @@ -1059,17 +1058,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>
> color = get_percent_color(max_percent);
>
> - /*
> - * Also color the filename and line if needed, with
> - * the same color than the percentage. Don't print it
> - * twice for close colored addr with the same filename:line
> - */
> if (path) {
> - if (!prev_line || strcmp(prev_line, path)
> - || color != prev_color) {
> - color_fprintf(stdout, color, " %s", path);
> + if (!prev_line || strcmp(prev_line, path)) {
> + fprintf(stdout, " %s\n", path);
> prev_line = path;
> - prev_color = color;
> }
> }
>
> @@ -1650,14 +1642,9 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
> percent_max = src_line->samples[k].percent;
> }
>
> - if (percent_max <= 0.5)
> - goto next;
> -
> offset = start + i;
> src_line->path = get_srcline(map->dso, offset, NULL, false);
> insert_source_line(&tmp_root, src_line);
> -
> - next:
> src_line = (void *)src_line + sizeof_src_line;
> }
>
> --
> 2.7.4
>
--
Thanks,
Namhyung
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2017-02-22 12:30 +0100 |
| Message-ID | <tdDho-V3-17@gated-at.bofh.it> |
| In reply to | #1586031 |
On Wed, Feb 22, 2017 at 7:08 PM, Taeung Song <treeze.taeung@gmail.com> wrote: > In the stdio interface, currently 'filename:linenr' infos > are confusedly printed in the intervals of assembly code. > So fix it. > > The cause was a 0.5% filter of if statement. After fixed, > additionally summary of overhead per srcline is more correct. > > Before: > > # perf annotate --stdio -l > > Sorted summary for file /home/taeung/workspace/perf-test/test > ---------------------------------------------- > > 36.57 test.c:38 > 28.72 test.c:37 > > ... > > Percent | Source code & Disassembly of test ... > > ... > > 0.21 : 400816: push %rbp > test.c:26 1.86 : 400817: mov %rsp,%rbp > 0.21 : 40081a: mov %edi,-0x24(%rbp) > 0.21 : 40081d: mov %rsi,-0x30(%rbp) > > After: > > # perf annotate --stdio -l > > Sorted summary for file /home/taeung/workspace/perf-test/test > ---------------------------------------------- > > 37.40 test.c:38 > 29.34 test.c:37 > > ... > > Percent | Source code & Disassembly of test ... > > ... > > test.c:26 > 0.21 : 400816: push %rbp > 1.86 : 400817: mov %rsp,%rbp > 0.21 : 40081a: mov %edi,-0x24(%rbp) > 0.21 : 40081d: mov %rsi,-0x30(%rbp) I guess it's just a problem of a missing newline.. Thanks, Namhyung
[toc] | [prev] | [next] | [standalone]
| From | Taeung Song <treeze.taeung@gmail.com> |
|---|---|
| Date | 2017-02-22 17:40 +0100 |
| Subject | Re: [PATCH 2/4] perf annotate: Align filename:linenr and more correct summary |
| Message-ID | <tdI7o-4xK-13@gated-at.bofh.it> |
| In reply to | #1586081 |
On 02/22/2017 08:22 PM, Namhyung Kim wrote:
> On Wed, Feb 22, 2017 at 7:08 PM, Taeung Song <treeze.taeung@gmail.com> wrote:
>> In the stdio interface, currently 'filename:linenr' infos
>> are confusedly printed in the intervals of assembly code.
>> So fix it.
>>
>> The cause was a 0.5% filter of if statement. After fixed,
>> additionally summary of overhead per srcline is more correct.
>>
>> Before:
>>
>> # perf annotate --stdio -l
>>
>> Sorted summary for file /home/taeung/workspace/perf-test/test
>> ----------------------------------------------
>>
>> 36.57 test.c:38
>> 28.72 test.c:37
>>
>> ...
>>
>> Percent | Source code & Disassembly of test ...
>>
>> ...
>>
>> 0.21 : 400816: push %rbp
>> test.c:26 1.86 : 400817: mov %rsp,%rbp
>> 0.21 : 40081a: mov %edi,-0x24(%rbp)
>> 0.21 : 40081d: mov %rsi,-0x30(%rbp)
>>
>> After:
>>
>> # perf annotate --stdio -l
>>
>> Sorted summary for file /home/taeung/workspace/perf-test/test
>> ----------------------------------------------
>>
>> 37.40 test.c:38
>> 29.34 test.c:37
>>
>> ...
>>
>> Percent | Source code & Disassembly of test ...
>>
>> ...
>>
>> test.c:26
>> 0.21 : 400816: push %rbp
>> 1.86 : 400817: mov %rsp,%rbp
>> 0.21 : 40081a: mov %edi,-0x24(%rbp)
>> 0.21 : 40081d: mov %rsi,-0x30(%rbp)
>
> I guess it's just a problem of a missing newline..
>
I think the problem is not only from a missing newline but also
from 0.5 filtering if statement.
For example,
If just appending new line, the output is as below
0.21 : 400816: push %rbp
test.c:26
1.86 : 400817: mov %rsp,%rbp
0.21 : 40081a: mov %edi,-0x24(%rbp)
0.21 : 40081d: mov %rsi,-0x30(%rbp)
The reason of the wrong sorting is that only 400817 is matched with
test.c:26
And the root cause is a if statement filtering smaller values than 0.5.
The if statement prevent other addresses that are less than 0.5
from matching test.c:26
So I eliminated it.
- if (percent_max <= 0.5)
- goto next;
But 400816, 400817, 40081a and 40081d addresses should be matched
with test.c:26. So I think it is better to show as below
test.c:26
0.21 : 400816: push %rbp
1.86 : 400817: mov %rsp,%rbp
0.21 : 40081a: mov %edi,-0x24(%rbp)
0.21 : 40081d: mov %rsi,-0x30(%rbp)
And I think it is better to rewrite this commit title and message..
I'll change this patch as v2 to clearly understand problem and solution.
Thanks,
Teaung
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web