From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: "Martin Liška" <mliska@suse.cz>
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 14:12:20 -0300 [thread overview]
Message-ID: <YH259OqyzU7P6f7d@kernel.org> (raw)
In-Reply-To: <bdccc648-41e4-2719-cfbb-a9f44ff3b489@suse.cz>
Em Mon, Apr 19, 2021 at 09:39:37AM +0200, Martin Liška escreveu:
> @Arnaldo: May I please ping this?
Applied the refreshed version,
- Arnaldo
> 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;
> >>
> >
>
--
- Arnaldo
prev parent reply other threads:[~2021-04-19 17:12 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
2021-04-19 17:12 ` Arnaldo Carvalho de Melo [this message]
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=YH259OqyzU7P6f7d@kernel.org \
--to=acme@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mliska@suse.cz \
/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).