* [Suggestion/Problems] perf annoate: Some problems related to the source code view and Improvement of it with line numbers
@ 2017-02-15 12:34 Taeung Song
2017-02-15 13:24 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Taeung Song @ 2017-02-15 12:34 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim
Cc: perf group, LKML, Ingo Molnar, Peter Zijlstra
Hi all :)
I found some problems about showing line numbers of perf-annotate.
and I have a suggestion about perf-annotate.
Sure, I have a plan to send the patchset about that.
But I'd like to know other opinions about that before sending the
patchset. :)
If the user do perf-annotate,
# perf record ./a.out
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.043 MB perf.data (799 samples) ]
# perf annotate
(In TUI, if using a 'k' option to see line numbers)
│3 Disassembly of section .text:
│
│5 0000000000400966 <get_cond_maxprice>:
│6 get_cond_maxprice():
│7 };
│
│9 unsigned int limited_wgt;
│
│11 unsigned int get_cond_maxprice(int wgt, struct jewelry
*jewelry)
│12 {
But I think it has some problems and seems a bit deficient..
There are several reasons as below.
1) I think we don't need the line numbers except for source codes
│3 Disassembly of section .text:
│
│5 0000000000400966 <get_cond_maxprice>:
│6 get_cond_maxprice():
I think that '3', '5' and '6' seems like needless.
2) The line numbers aren't correct
(But they are correct if perf-annotate work with --stdio)
If we check the actual source file as below,
we can find many wrong line numbers on perf-annotate.
(e.g. A actual line number of get_cond_amxprice() is 25,
but the line number of the function on perf-annotate is 11.)
...
18 struct jewelry {
19 unsigned int wgt;
20 unsigned int price;
21 };
22
23 unsigned int limited_wgt;
24
25 unsigned int get_cond_maxprice(int wgt, struct jewelry *jewelry)
26 {
27 /* Get maximum price based on a specific weight
28 * following a specific jewelry.
29 */
30 int i;
31 unsigned int nr_cases = wgt/jewelry->wgt;
32 unsigned int maxprice = 0;
...
Sure, the correct line numbers partially exist as well
but there are more wrong line numbers than them.
3) It is hard to read source code + assembly code of perf-annoate.
If the user want to see source code on perf-annotate,
In TUI the user can use 'k' option but the output is mixed with
both source code and assembly code so it's so confusing.. :-\
and the readability is not good.. :(
And the line numbers aren't even wrong..
So, I suggest first showing source code before assembly code
only when the target program has debug info.
If we do, perf-annotate provide good readable source code view
per function
and correct line numbers. :)
For example,
If there is 'get_cond_maxprice()' on a source file as below,
... (with line numbers) ...
25 unsigned int get_cond_maxprice(int wgt, struct jewelry *jewelry)
26 {
27 /* Get maximum price based on a specific weight
28 * following a specific jewelry.
29 */
30 int i;
31 unsigned int nr_cases = wgt/jewelry->wgt;
32 unsigned int maxprice = 0;
33
34 for (i = 1; i <= nr_cases; i++) {
35 unsigned int price, rest_wgt;
36
37 rest_wgt = wgt - (i * jewelry->wgt);
38 price = (i * jewelry->price) +
knapsack_list[rest_wgt].maxprice;
39 if (maxprice < price)
40 maxprice = price;
41 }
42
43 return maxprice;
44 }
...
And if the target program(e.g. a.out) compiled with the source file has
debug info,
we can first show source code before assembly code for good readability
as below.
(If we do, the user don't be confusing due to mixed parts of code any more)
# perf record ./a.out
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.043 MB perf.data (799 samples) ]
# perf annoate (using 'k' option in TUI)
...
│25 unsigned int get_cond_maxprice(int wgt, struct jewelry *jewelry)
2.21 │26 {
│27 /* Get maximum price based on a specific weight
│28 * following a specific jewelry.
│29 */
│30 int i;
4.43 │31 unsigned int nr_cases = wgt/jewelry->wgt;
│32 unsigned int maxprice = 0;
│33
7.49 │34 for (i = 1; i <= nr_cases; i++) {
│35 unsigned int price, rest_wgt;
│36
29.12 │37 rest_wgt = wgt - (i * jewelry->wgt);
15.83 │38 price = (i * jewelry->price) +
knapsack_list[rest_wgt].maxprice;
17.38 │39 if (maxprice < price)
0.34 │40 maxprice = price;
│41 }
│42
1.19 │43 return maxprice;
1.19 │44 }
And if the user want to see not only source code but also assembly code,
we can show as below.
(This output is similar to current annotate view
but it sequentially show full code of the function, not confusing mixed
parts of code
And this output show correct line numbers unlike current annotate view)
│25 unsigned int get_cond_maxprice(int wgt, struct jewelry *jewelry)
│26 {
0.17 │ push %rbp
│ mov %rsp,%rbp
│ sub $0x30,%rsp
0.51 │ → callq mcount@plt
│ mov %edi,-0x24(%rbp)
1.53 │ mov %rsi,-0x30(%rbp)
│27 /* Get maximum price based on a specific weight
│28 * following a specific jewelry.
│29 */
│30 int i;
│31 unsigned int nr_cases = wgt/jewelry->wgt;
│ mov -0x24(%rbp),%eax
│ mov -0x30(%rbp),%rdx
0.34 │ mov (%rdx),%esi
1.36 │ mov $0x0,%edx
2.73 │ div %esi
│ mov %eax,-0xc(%rbp)
│32 unsigned int maxprice = 0;
│ movl $0x0,-0x10(%rbp)
│33
│34 for (i = 1; i <= nr_cases; i++) {
│ movl $0x1,-0x14(%rbp)
│ ↓ jmp <get_cond_maxprice+0x84>
│35 unsigned int price, rest_wgt;
│36
│37 rest_wgt = wgt - (i * jewelry->wgt);
1.53 │ mov -0x24(%rbp),%edx
│ mov -0x30(%rbp),%rax
0.51 │ mov (%rax),%ecx
10.56 │ mov -0x14(%rbp),%eax
1.70 │ imul %ecx,%eax
0.34 │ sub %eax,%edx
1.36 │ mov %edx,%eax
13.12 │ mov %eax,-0x8(%rbp)
│38 price = (i * jewelry->price) +
knapsack_list[rest_wgt].maxprice;
1.87 │ mov -0x30(%rbp),%rax
0.34 │ mov 0x4(%rax),%edx
0.85 │ mov -0x14(%rbp),%eax
10.90 │ imul %eax,%edx
1.87 │ mov knapsack_list,%rax
│ mov -0x8(%rbp),%ecx
0.68 │ shl $0x2,%rcx
11.24 │ add %rcx,%rax
3.58 │ mov (%rax),%eax
2.39 │ add %edx,%eax
2.90 │ mov %eax,-0x4(%rbp)
│39 if (maxprice < price)
7.67 │ mov -0x10(%rbp),%eax
│ cmp -0x4(%rbp),%eax
9.71 │ ↓ jae <get_cond_maxprice+0x80>
│40 maxprice = price;
│ mov -0x4(%rbp),%eax
0.34 │ mov %eax,-0x10(%rbp)
│
3.58 │ addl $0x1,-0x14(%rbp)
1.36 │ mov -0x14(%rbp),%eax
0.34 │ cmp -0xc(%rbp),%eax
2.21 │ ↑ jbe <get_cond_maxprice+0x37>
│41 }
│42
│43 return maxprice;
1.19 │ mov -0x10(%rbp),%eax
│44 }
0.68 │ leaveq
0.51 │ ← retq
Sure, we can show only assembly code as below (this is current feature)
(Of course, the target program hasn't debug info, perf-annotate show
assembly code as below)
# perf annotate
0.17 │ push %rbp
│ mov %rsp,%rbp
│ sub $0x30,%rsp
0.51 │ → callq mcount@plt
│ mov %edi,-0x24(%rbp)
1.53 │ mov %rsi,-0x30(%rbp)
│ mov -0x24(%rbp),%eax
│ mov -0x30(%rbp),%rdx
0.34 │ mov (%rdx),%esi
1.36 │ mov $0x0,%edx
2.73 │ div %esi
│ mov %eax,-0xc(%rbp)
│ movl $0x0,-0x10(%rbp)
│ movl $0x1,-0x14(%rbp)
│ ↓ jmp 84
1.53 │37: mov -0x24(%rbp),%edx
│ mov -0x30(%rbp),%rax
0.51 │ mov (%rax),%ecx
10.56 │ mov -0x14(%rbp),%eax
1.70 │ imul %ecx,%eax
0.34 │ sub %eax,%edx
1.36 │ mov %edx,%eax
13.12 │ mov %eax,-0x8(%rbp)
1.87 │ mov -0x30(%rbp),%rax
0.34 │ mov 0x4(%rax),%edx
0.85 │ mov -0x14(%rbp),%eax
10.90 │ imul %eax,%edx
1.87 │ mov knapsack_list,%rax
│ mov -0x8(%rbp),%ecx
0.68 │ shl $0x2,%rcx
11.24 │ add %rcx,%rax
3.58 │ mov (%rax),%eax
2.39 │ add %edx,%eax
2.90 │ mov %eax,-0x4(%rbp)
7.67 │ mov -0x10(%rbp),%eax
│ cmp -0x4(%rbp),%eax
9.71 │ ↓ jae 80
│ mov -0x4(%rbp),%eax
0.34 │ mov %eax,-0x10(%rbp)
3.58 │80: addl $0x1,-0x14(%rbp)
1.36 │84: mov -0x14(%rbp),%eax
0.34 │ cmp -0xc(%rbp),%eax
2.21 │ ↑ jbe 37
1.19 │ mov -0x10(%rbp),%eax
0.68 │ leaveq
0.51 │ ← retq
What do you think about the suggestion and some problems ?
Just send patchset for this problems and the my suggestion ?
I think the lack comes from the output of objdump command.
perf internally handle source codes and line numbers
by output of objdump with 'd' and 'S' as below.
# cat util/annotate.c
...
1436 "%s %s%s --start-address=0x%016" PRIx64
1437 " --stop-address=0x%016" PRIx64
1438 " -l -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
1439 objdump_path ? objdump_path : "objdump",
1440 disassembler_style ? "-M " : "",
1441 disassembler_style ? disassembler_style : "",
1442 map__rip_2objdump(map, sym->start),
1443 map__rip_2objdump(map, sym->end),
1444 symbol_conf.annotate_asm_raw ? "" : "--no-show-raw",
1445 symbol_conf.annotate_src ? "-S" : "",
1446 symfs_filename, symfs_filename);
...
So if we don't rely on a feature '-S' of objdump and
if we read the actual source file,
I think perf-annotate can provide more readable source code view per
function
and more precise line numbers.
Thanks,
Taeung
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Suggestion/Problems] perf annoate: Some problems related to the source code view and Improvement of it with line numbers
2017-02-15 12:34 [Suggestion/Problems] perf annoate: Some problems related to the source code view and Improvement of it with line numbers Taeung Song
@ 2017-02-15 13:24 ` Arnaldo Carvalho de Melo
2017-02-15 14:24 ` Taeung Song
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-02-15 13:24 UTC (permalink / raw)
To: Taeung Song
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim, perf group,
LKML, Ingo Molnar, Peter Zijlstra
Em Wed, Feb 15, 2017 at 09:34:51PM +0900, Taeung Song escreveu:
> Hi all :)
>
> I found some problems about showing line numbers of perf-annotate.
> and I have a suggestion about perf-annotate.
>
> Sure, I have a plan to send the patchset about that.
> But I'd like to know other opinions about that before sending the patchset.
> :)
Humm, TLDR, please try to find one problem, describe it precisely, show
before and after results (in the changeset message), and send it, rinse,
repeat.
For instance, if the line numbers are different in --stdio and --tui,
that is a problem, state that, show the output (as you did below), put
it in a patch, send it.
Thanks,
- Arnaldo
> If the user do perf-annotate,
>
> # perf record ./a.out
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.043 MB perf.data (799 samples) ]
>
> # perf annotate
>
> (In TUI, if using a 'k' option to see line numbers)
>
> │3 Disassembly of section .text:
> │
> │5 0000000000400966 <get_cond_maxprice>:
> │6 get_cond_maxprice():
> │7 };
> │
> │9 unsigned int limited_wgt;
> │
> │11 unsigned int get_cond_maxprice(int wgt, struct jewelry *jewelry)
> │12 {
>
> But I think it has some problems and seems a bit deficient..
> There are several reasons as below.
>
> 1) I think we don't need the line numbers except for source codes
>
> │3 Disassembly of section .text:
> │
> │5 0000000000400966 <get_cond_maxprice>:
> │6 get_cond_maxprice():
>
> I think that '3', '5' and '6' seems like needless.
>
> 2) The line numbers aren't correct
> (But they are correct if perf-annotate work with --stdio)
>
> If we check the actual source file as below,
> we can find many wrong line numbers on perf-annotate.
>
> (e.g. A actual line number of get_cond_amxprice() is 25,
> but the line number of the function on perf-annotate is 11.)
>
> ...
> 18 struct jewelry {
> 19 unsigned int wgt;
> 20 unsigned int price;
> 21 };
> 22
> 23 unsigned int limited_wgt;
> 24
> 25 unsigned int get_cond_maxprice(int wgt, struct jewelry *jewelry)
> 26 {
> 27 /* Get maximum price based on a specific weight
>
> 28 * following a specific jewelry.
>
> 29 */
> 30 int i;
> 31 unsigned int nr_cases = wgt/jewelry->wgt;
> 32 unsigned int maxprice = 0;
> ...
>
> Sure, the correct line numbers partially exist as well
> but there are more wrong line numbers than them.
>
> 3) It is hard to read source code + assembly code of perf-annoate.
> If the user want to see source code on perf-annotate,
> In TUI the user can use 'k' option but the output is mixed with
> both source code and assembly code so it's so confusing.. :-\
> and the readability is not good.. :(
> And the line numbers aren't even wrong..
>
> So, I suggest first showing source code before assembly code
> only when the target program has debug info.
> If we do, perf-annotate provide good readable source code view per
> function
> and correct line numbers. :)
>
> For example,
> If there is 'get_cond_maxprice()' on a source file as below,
>
> ... (with line numbers) ...
> 25 unsigned int get_cond_maxprice(int wgt, struct jewelry *jewelry)
> 26 {
> 27 /* Get maximum price based on a specific weight
>
> 28 * following a specific jewelry.
>
> 29 */
> 30 int i;
> 31 unsigned int nr_cases = wgt/jewelry->wgt;
> 32 unsigned int maxprice = 0;
> 33
> 34 for (i = 1; i <= nr_cases; i++) {
> 35 unsigned int price, rest_wgt;
> 36
> 37 rest_wgt = wgt - (i * jewelry->wgt);
> 38 price = (i * jewelry->price) +
> knapsack_list[rest_wgt].maxprice;
> 39 if (maxprice < price)
> 40 maxprice = price;
> 41 }
> 42
> 43 return maxprice;
> 44 }
> ...
>
> And if the target program(e.g. a.out) compiled with the source file has
> debug info,
> we can first show source code before assembly code for good readability as
> below.
> (If we do, the user don't be confusing due to mixed parts of code any more)
>
> # perf record ./a.out
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.043 MB perf.data (799 samples) ]
>
> # perf annoate (using 'k' option in TUI)
> ...
> │25 unsigned int get_cond_maxprice(int wgt, struct jewelry *jewelry)
> 2.21 │26 {
> │27 /* Get maximum price based on a specific weight
>
> │28 * following a specific jewelry.
>
> │29 */
> │30 int i;
> 4.43 │31 unsigned int nr_cases = wgt/jewelry->wgt;
> │32 unsigned int maxprice = 0;
> │33
> 7.49 │34 for (i = 1; i <= nr_cases; i++) {
> │35 unsigned int price, rest_wgt;
> │36
> 29.12 │37 rest_wgt = wgt - (i * jewelry->wgt);
> 15.83 │38 price = (i * jewelry->price) +
> knapsack_list[rest_wgt].maxprice;
> 17.38 │39 if (maxprice < price)
> 0.34 │40 maxprice = price;
> │41 }
> │42
> 1.19 │43 return maxprice;
> 1.19 │44 }
>
>
> And if the user want to see not only source code but also assembly code,
> we can show as below.
> (This output is similar to current annotate view
> but it sequentially show full code of the function, not confusing mixed
> parts of code
> And this output show correct line numbers unlike current annotate view)
>
> │25 unsigned int get_cond_maxprice(int wgt, struct jewelry *jewelry)
> │26 {
> 0.17 │ push %rbp
> │ mov %rsp,%rbp
> │ sub $0x30,%rsp
> 0.51 │ → callq mcount@plt
> │ mov %edi,-0x24(%rbp)
> 1.53 │ mov %rsi,-0x30(%rbp)
> │27 /* Get maximum price based on a specific weight
>
> │28 * following a specific jewelry.
>
> │29 */
> │30 int i;
> │31 unsigned int nr_cases = wgt/jewelry->wgt;
> │ mov -0x24(%rbp),%eax
> │ mov -0x30(%rbp),%rdx
> 0.34 │ mov (%rdx),%esi
> 1.36 │ mov $0x0,%edx
> 2.73 │ div %esi
> │ mov %eax,-0xc(%rbp)
> │32 unsigned int maxprice = 0;
> │ movl $0x0,-0x10(%rbp)
> │33
> │34 for (i = 1; i <= nr_cases; i++) {
> │ movl $0x1,-0x14(%rbp)
> │ ↓ jmp <get_cond_maxprice+0x84>
> │35 unsigned int price, rest_wgt;
> │36
> │37 rest_wgt = wgt - (i * jewelry->wgt);
> 1.53 │ mov -0x24(%rbp),%edx
> │ mov -0x30(%rbp),%rax
> 0.51 │ mov (%rax),%ecx
> 10.56 │ mov -0x14(%rbp),%eax
> 1.70 │ imul %ecx,%eax
> 0.34 │ sub %eax,%edx
> 1.36 │ mov %edx,%eax
> 13.12 │ mov %eax,-0x8(%rbp)
> │38 price = (i * jewelry->price) +
> knapsack_list[rest_wgt].maxprice;
> 1.87 │ mov -0x30(%rbp),%rax
> 0.34 │ mov 0x4(%rax),%edx
> 0.85 │ mov -0x14(%rbp),%eax
> 10.90 │ imul %eax,%edx
> 1.87 │ mov knapsack_list,%rax
> │ mov -0x8(%rbp),%ecx
> 0.68 │ shl $0x2,%rcx
> 11.24 │ add %rcx,%rax
> 3.58 │ mov (%rax),%eax
> 2.39 │ add %edx,%eax
> 2.90 │ mov %eax,-0x4(%rbp)
> │39 if (maxprice < price)
> 7.67 │ mov -0x10(%rbp),%eax
> │ cmp -0x4(%rbp),%eax
> 9.71 │ ↓ jae <get_cond_maxprice+0x80>
> │40 maxprice = price;
> │ mov -0x4(%rbp),%eax
> 0.34 │ mov %eax,-0x10(%rbp)
> │
> 3.58 │ addl $0x1,-0x14(%rbp)
> 1.36 │ mov -0x14(%rbp),%eax
> 0.34 │ cmp -0xc(%rbp),%eax
> 2.21 │ ↑ jbe <get_cond_maxprice+0x37>
> │41 }
> │42
> │43 return maxprice;
> 1.19 │ mov -0x10(%rbp),%eax
> │44 }
> 0.68 │ leaveq
> 0.51 │ ← retq
>
>
> Sure, we can show only assembly code as below (this is current feature)
> (Of course, the target program hasn't debug info, perf-annotate show
> assembly code as below)
>
> # perf annotate
>
> 0.17 │ push %rbp
> │ mov %rsp,%rbp
> │ sub $0x30,%rsp
> 0.51 │ → callq mcount@plt
> │ mov %edi,-0x24(%rbp)
> 1.53 │ mov %rsi,-0x30(%rbp)
> │ mov -0x24(%rbp),%eax
> │ mov -0x30(%rbp),%rdx
> 0.34 │ mov (%rdx),%esi
> 1.36 │ mov $0x0,%edx
> 2.73 │ div %esi
> │ mov %eax,-0xc(%rbp)
> │ movl $0x0,-0x10(%rbp)
> │ movl $0x1,-0x14(%rbp)
> │ ↓ jmp 84
> 1.53 │37: mov -0x24(%rbp),%edx
> │ mov -0x30(%rbp),%rax
> 0.51 │ mov (%rax),%ecx
> 10.56 │ mov -0x14(%rbp),%eax
> 1.70 │ imul %ecx,%eax
> 0.34 │ sub %eax,%edx
> 1.36 │ mov %edx,%eax
> 13.12 │ mov %eax,-0x8(%rbp)
> 1.87 │ mov -0x30(%rbp),%rax
> 0.34 │ mov 0x4(%rax),%edx
> 0.85 │ mov -0x14(%rbp),%eax
> 10.90 │ imul %eax,%edx
> 1.87 │ mov knapsack_list,%rax
> │ mov -0x8(%rbp),%ecx
> 0.68 │ shl $0x2,%rcx
> 11.24 │ add %rcx,%rax
> 3.58 │ mov (%rax),%eax
> 2.39 │ add %edx,%eax
> 2.90 │ mov %eax,-0x4(%rbp)
> 7.67 │ mov -0x10(%rbp),%eax
> │ cmp -0x4(%rbp),%eax
> 9.71 │ ↓ jae 80
> │ mov -0x4(%rbp),%eax
> 0.34 │ mov %eax,-0x10(%rbp)
> 3.58 │80: addl $0x1,-0x14(%rbp)
> 1.36 │84: mov -0x14(%rbp),%eax
> 0.34 │ cmp -0xc(%rbp),%eax
> 2.21 │ ↑ jbe 37
> 1.19 │ mov -0x10(%rbp),%eax
> 0.68 │ leaveq
> 0.51 │ ← retq
>
>
> What do you think about the suggestion and some problems ?
> Just send patchset for this problems and the my suggestion ?
>
> I think the lack comes from the output of objdump command.
> perf internally handle source codes and line numbers
> by output of objdump with 'd' and 'S' as below.
>
> # cat util/annotate.c
> ...
> 1436 "%s %s%s --start-address=0x%016" PRIx64
> 1437 " --stop-address=0x%016" PRIx64
> 1438 " -l -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
> 1439 objdump_path ? objdump_path : "objdump",
> 1440 disassembler_style ? "-M " : "",
> 1441 disassembler_style ? disassembler_style : "",
> 1442 map__rip_2objdump(map, sym->start),
> 1443 map__rip_2objdump(map, sym->end),
> 1444 symbol_conf.annotate_asm_raw ? "" : "--no-show-raw",
> 1445 symbol_conf.annotate_src ? "-S" : "",
> 1446 symfs_filename, symfs_filename);
> ...
>
> So if we don't rely on a feature '-S' of objdump and
> if we read the actual source file,
> I think perf-annotate can provide more readable source code view per
> function
> and more precise line numbers.
>
> Thanks,
> Taeung
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Suggestion/Problems] perf annoate: Some problems related to the source code view and Improvement of it with line numbers
2017-02-15 13:24 ` Arnaldo Carvalho de Melo
@ 2017-02-15 14:24 ` Taeung Song
2017-02-17 2:33 ` Taeung Song
0 siblings, 1 reply; 6+ messages in thread
From: Taeung Song @ 2017-02-15 14:24 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, perf group, LKML, Ingo Molnar,
Peter Zijlstra
On 02/15/2017 10:24 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Feb 15, 2017 at 09:34:51PM +0900, Taeung Song escreveu:
>> > Hi all :)
>> >
>> > I found some problems about showing line numbers of perf-annotate.
>> > and I have a suggestion about perf-annotate.
>> >
>> > Sure, I have a plan to send the patchset about that.
>> > But I'd like to know other opinions about that before sending the patchset.
>> > :)
> Humm, TLDR, please try to find one problem, describe it precisely, show
> before and after results (in the changeset message), and send it, rinse,
> repeat.
>
> For instance, if the line numbers are different in --stdio and --tui,
> that is a problem, state that, show the output (as you did below), put
> it in a patch, send it.
>
Okey it was too long.
I'll separate the contents into simple parts
and send a patch with things you said :)
for easy review.
Thanks,
Taeung
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Suggestion/Problems] perf annoate: Some problems related to the source code view and Improvement of it with line numbers
2017-02-15 14:24 ` Taeung Song
@ 2017-02-17 2:33 ` Taeung Song
2017-02-17 13:29 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Taeung Song @ 2017-02-17 2:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, perf group, LKML, Ingo Molnar,
Peter Zijlstra
Hi, Arnaldo :)
Regarding perf annotate:
1) Problem : wrong line numbers on perf-annotate (both stdio and TUI)
2) Problem : wrong sum of overhead(percent) matching source lines
3) Suggestion : new option showing only source code per function
with overhead info (to be more readable :) )
I'll send the patchset for them, maybe it'll a bit take time.
But I won't be long..
Thanks,
Taeung
On 02/15/2017 11:24 PM, Taeung Song wrote:
>
>
> On 02/15/2017 10:24 PM, Arnaldo Carvalho de Melo wrote:
>> Em Wed, Feb 15, 2017 at 09:34:51PM +0900, Taeung Song escreveu:
>>> > Hi all :)
>>> >
>>> > I found some problems about showing line numbers of perf-annotate.
>>> > and I have a suggestion about perf-annotate.
>>> >
>>> > Sure, I have a plan to send the patchset about that.
>>> > But I'd like to know other opinions about that before sending the
>>> patchset.
>>> > :)
>> Humm, TLDR, please try to find one problem, describe it precisely, show
>> before and after results (in the changeset message), and send it, rinse,
>> repeat.
>>
>> For instance, if the line numbers are different in --stdio and --tui,
>> that is a problem, state that, show the output (as you did below), put
>> it in a patch, send it.
>>
>
> Okey it was too long.
>
> I'll separate the contents into simple parts
> and send a patch with things you said :)
> for easy review.
>
> Thanks,
> Taeung
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Suggestion/Problems] perf annoate: Some problems related to the source code view and Improvement of it with line numbers
2017-02-17 2:33 ` Taeung Song
@ 2017-02-17 13:29 ` Arnaldo Carvalho de Melo
2017-02-18 0:15 ` Taeung Song
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-02-17 13:29 UTC (permalink / raw)
To: Taeung Song
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim, perf group,
LKML, Ingo Molnar, Peter Zijlstra
Em Fri, Feb 17, 2017 at 11:33:29AM +0900, Taeung Song escreveu:
> Hi, Arnaldo :)
>
> Regarding perf annotate:
>
> 1) Problem : wrong line numbers on perf-annotate (both stdio and TUI)
> 2) Problem : wrong sum of overhead(percent) matching source lines
> 3) Suggestion : new option showing only source code per function
> with overhead info (to be more readable :) )
>
> I'll send the patchset for them, maybe it'll a bit take time.
> But I won't be long..
Concentrate on the first, send the patch, while waiting for review, work
on the second, and so on :-)
- Arnaldo
> Thanks,
> Taeung
>
> On 02/15/2017 11:24 PM, Taeung Song wrote:
> >
> >
> > On 02/15/2017 10:24 PM, Arnaldo Carvalho de Melo wrote:
> > > Em Wed, Feb 15, 2017 at 09:34:51PM +0900, Taeung Song escreveu:
> > > > > Hi all :)
> > > > >
> > > > > I found some problems about showing line numbers of perf-annotate.
> > > > > and I have a suggestion about perf-annotate.
> > > > >
> > > > > Sure, I have a plan to send the patchset about that.
> > > > > But I'd like to know other opinions about that before sending the
> > > > patchset.
> > > > > :)
> > > Humm, TLDR, please try to find one problem, describe it precisely, show
> > > before and after results (in the changeset message), and send it, rinse,
> > > repeat.
> > >
> > > For instance, if the line numbers are different in --stdio and --tui,
> > > that is a problem, state that, show the output (as you did below), put
> > > it in a patch, send it.
> > >
> >
> > Okey it was too long.
> >
> > I'll separate the contents into simple parts
> > and send a patch with things you said :)
> > for easy review.
> >
> > Thanks,
> > Taeung
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Suggestion/Problems] perf annoate: Some problems related to the source code view and Improvement of it with line numbers
2017-02-17 13:29 ` Arnaldo Carvalho de Melo
@ 2017-02-18 0:15 ` Taeung Song
0 siblings, 0 replies; 6+ messages in thread
From: Taeung Song @ 2017-02-18 0:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, perf group, LKML, Ingo Molnar,
Peter Zijlstra
On 02/17/2017 10:29 PM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 17, 2017 at 11:33:29AM +0900, Taeung Song escreveu:
>> Hi, Arnaldo :)
>>
>> Regarding perf annotate:
>>
>> 1) Problem : wrong line numbers on perf-annotate (both stdio and TUI)
>> 2) Problem : wrong sum of overhead(percent) matching source lines
>> 3) Suggestion : new option showing only source code per function
>> with overhead info (to be more readable :) )
>>
>> I'll send the patchset for them, maybe it'll a bit take time.
>> But I won't be long..
>
> Concentrate on the first, send the patch, while waiting for review, work
> on the second, and so on :-)
>
> - Arnaldo
>
Understood!
I appreciate your concern :)
Thanks,
Taeung
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-18 0:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-15 12:34 [Suggestion/Problems] perf annoate: Some problems related to the source code view and Improvement of it with line numbers Taeung Song
2017-02-15 13:24 ` Arnaldo Carvalho de Melo
2017-02-15 14:24 ` Taeung Song
2017-02-17 2:33 ` Taeung Song
2017-02-17 13:29 ` Arnaldo Carvalho de Melo
2017-02-18 0:15 ` 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).