From: "Martin Liška" <mliska@suse.cz>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH] perf annotate: improve --stdio mode
Date: Mon, 19 Apr 2021 09:39:37 +0200 [thread overview]
Message-ID: <bdccc648-41e4-2719-cfbb-a9f44ff3b489@suse.cz> (raw)
In-Reply-To: <4816a983-9d3c-2813-e9eb-1901b524e241@suse.cz>
@Arnaldo: May I please ping this?
Thanks,
Martin
On 4/8/21 12:08 PM, Martin Liška wrote:
> On 4/7/21 10:25 PM, Arnaldo Carvalho de Melo wrote:
>> Em Wed, Apr 07, 2021 at 04:30:46PM -0300, Arnaldo Carvalho de Melo escreveu:
>>> Em Fri, Feb 26, 2021 at 10:24:00AM +0100, Martin Liška escreveu:
>>>> On 2/23/21 8:47 PM, Arnaldo Carvalho de Melo wrote:
>>>> Sure. But I think the current format provides quite broken visual layout:
>>>>
>>>> 0.00 : 405ef1: inc %r15
>>>> 0.01 : 405ef4: vfmadd213sd 0x2b9b3(%rip),%xmm0,%xmm3 # 4318b0 <_IO_stdin_used+0x8b0>
>>>> eff.c:1811 0.67 : 405efd: vfmadd213sd 0x2b9b2(%rip),%xmm0,%xmm3 # 4318b8 <_IO_stdin_used+0x8b8>
>>>> : TA + tmpsd * (TB +
>>>>
>>>> vs.
>>>>
>>>> 0.00 : 405ef1: inc %r15
>>>> 0.01 : 405ef4: vfmadd213sd 0x2b9b3(%rip),%xmm0,%xmm3 # 4318b0 <_IO_stdin_used+0x8b0>
>>>> 0.67 : 405efd: vfmadd213sd 0x2b9b2(%rip),%xmm0,%xmm3 # 4318b8 <_IO_stdin_used+0x8b8> // eff.c:1811
>>>> : 1810 TA + tmpsd * (TB +
>>>>
>>>> I bet also the current users of --stdio mode would benefit from it.
>>>> What do you think?
>>
>>> Agreed, I tried applying but it bitrotted, it seems :-\
>>
>> I refreshed it, please check.
>
> Thanks! I've just tested the patch on top of acme/perf/core and it works as was planned.
> I'm attaching 2 perf annotate snippets (perf annotate --stdio -l --stdio-color=always) before
> and after the revision:
>
> https://splichal.eu/tmp/perf-before.html
> https://splichal.eu/tmp/perf-after.html
>
> I hope it nicely describes that it's an improvement.
>
> Cheers,
> Martin
>
>>
>> - Arnaldo
>>
>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>> index 18eee25b4976bea8..abe1499a91645375 100644
>> --- a/tools/perf/util/annotate.c
>> +++ b/tools/perf/util/annotate.c
>> @@ -1368,7 +1368,6 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
>> {
>> struct disasm_line *dl = container_of(al, struct disasm_line, al);
>> static const char *prev_line;
>> - static const char *prev_color;
>>
>> if (al->offset != -1) {
>> double max_percent = 0.0;
>> @@ -1407,20 +1406,6 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
>>
>> 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 (al->path) {
>> - if (!prev_line || strcmp(prev_line, al->path)
>> - || color != prev_color) {
>> - color_fprintf(stdout, color, " %s", al->path);
>> - prev_line = al->path;
>> - prev_color = color;
>> - }
>> - }
>> -
>> for (i = 0; i < nr_percent; i++) {
>> struct annotation_data *data = &al->data[i];
>> double percent;
>> @@ -1441,6 +1426,19 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
>> printf(" : ");
>>
>> disasm_line__print(dl, start, addr_fmt_width);
>> +
>> + /*
>> + * 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 (al->path) {
>> + if (!prev_line || strcmp(prev_line, al->path)) {
>> + color_fprintf(stdout, color, " // %s", al->path);
>> + prev_line = al->path;
>> + }
>> + }
>> +
>> printf("\n");
>> } else if (max_lines && printed >= max_lines)
>> return 1;
>> @@ -1456,7 +1454,7 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
>> if (!*al->line)
>> printf(" %*s:\n", width, " ");
>> else
>> - printf(" %*s: %*s %s\n", width, " ", addr_fmt_width, " ", al->line);
>> + printf(" %*s: %-*d %s\n", width, " ", addr_fmt_width, al->line_nr, al->line);
>> }
>>
>> return 0;
>>
>
next prev parent reply other threads:[~2021-04-19 7:39 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-21 12:46 [PATCH] perf annotate: improve --stdio mode Martin Liška
2021-02-23 19:47 ` Arnaldo Carvalho de Melo
2021-02-26 9:24 ` Martin Liška
2021-03-19 9:38 ` Martin Liška
2021-04-07 19:28 ` Arnaldo Carvalho de Melo
2021-04-07 19:30 ` Arnaldo Carvalho de Melo
2021-04-07 20:25 ` Arnaldo Carvalho de Melo
2021-04-08 10:08 ` Martin Liška
2021-04-19 7:39 ` Martin Liška [this message]
2021-04-19 17:12 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bdccc648-41e4-2719-cfbb-a9f44ff3b489@suse.cz \
--to=mliska@suse.cz \
--cc=acme@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).