All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Andi Kleen <andi@firstfloor.org>
Cc: linux-kernel@vger.kernel.org, namhyung@kernel.org,
	acme@kernel.org, Andi Kleen <ak@linux.intel.com>
Subject: Re: [PATCH 5/8] perf, tools: Support source line numbers in annotate
Date: Mon, 20 Oct 2014 20:06:05 +0200	[thread overview]
Message-ID: <20141020180605.GC9913@krava.brq.redhat.com> (raw)
In-Reply-To: <1411774636-6870-6-git-send-email-andi@firstfloor.org>

On Fri, Sep 26, 2014 at 04:37:13PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> With srcline key/sort'ing it's useful to have line numbers
> in the annotate window. This patch implements this.
> 
> Use objdump -l to request the line numbers and
> save them in the line structure. Then the browser
> displays them for source lines.
> 
> The line numbers are not displayed by default, but can be
> toggled on with 'k'
> 
> There is one unfortunate problem with this setup. For
> lines not containing source and which are outside functions
> objdump -l reports line numbers off by a few: it always reports
> the first line number in the next function even for lines
> that are outside the function.
> I haven't found a nice way to detect/correct this. Probably objdump
> has to be fixed.
> See https://sourceware.org/bugzilla/show_bug.cgi?id=16433

seems like nice feature, anyway could you format the
line number into separated column?

currently without lines:


       │             /* Receive them all */                                                                                               ▒
       │             for (i = 0; i < ctx->num_packets; i++) {                                                                             ▒
  0.02 │       mov    (%r14),%eax                                                                                                         ▒
  0.02 │       xor    %r13d,%r13d                                                                                                         ▒
       │                     char data[DATASIZE];                                                                                         ▒
       │                     int ret, done = 0;                                                                                           ▒
       │                                                                                                                                  ▒
       │     again:                                                                                                                       ▒
       │                     ret = read(ctx->in_fds[0], data + done, DATASIZE - done);                                                    ▒
       │       mov    $0x64,%r12d                                                                                                         ▒
       │                                                                                                                                  ▒
       │             /* Wait for start... */                                                                                              ▒
       │             ready(ctx->ready_out, ctx->wakefd);                                                                                  ▒


with 'k' pressed:


       │    126           /* Receive them all */                                                                                          ▒
       │    127           for (i = 0; i < ctx->num_packets; i++) {                                                                        ▒
  0.02 │       mov    (%r14),%eax                                                                                                         ▒
  0.02 │       xor    %r13d,%r13d                                                                                                         ▒
       │    127                   char data[DATASIZE];                                                                                    ▒
       │    128                   int ret, done = 0;                                                                                      ▒
       │                                                                                                                                  ▒
       │    130   again:                                                                                                                  ▒
       │    131                   ret = read(ctx->in_fds[0], data + done, DATASIZE - done);                                               ▒
       │       mov    $0x64,%r12d                                                                                                         ▒
       │                                                                                                                                  ▒
       │    123           /* Wait for start... */                                                                                         ▒
       │    124           ready(ctx->ready_out, ctx->wakefd);                                                                             ▒


putting line numbers close to the left line
and making the source line not to be shifted
right by line number width would be great


SNIP

>  		slsmg_write_nstring(" ", width - pcnt_width);
>  	else if (dl->offset == -1) {
> -		printed = scnprintf(bf, sizeof(bf), "%*s  ",
> +		if (dl->line_nr && annotate_browser__opts.show_linenr)
> +			printed = scnprintf(bf, sizeof(bf), "%*s %-5d ",
> +					ab->addr_width, " ", dl->line_nr);
> +		else
> +			printed = scnprintf(bf, sizeof(bf), "%*s  ",
>  				    ab->addr_width, " ");
>  		slsmg_write_nstring(bf, printed);
>  		slsmg_write_nstring(dl->line, width - printed - pcnt_width + 1);
> @@ -733,6 +738,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
>  		"o             Toggle disassembler output/simplified view\n"
>  		"s             Toggle source code view\n"
>  		"/             Search string\n"
> +		"k	       Toggle line numbers\n"

extra tab --------^^^^^^ making the text misaligned in the help box


thanks,
jirka

  reply	other threads:[~2014-10-20 18:06 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-26 23:37 Implement lbr-as-callgraph v10 Andi Kleen
2014-09-26 23:37 ` [PATCH 1/8] perf, tools: Support handling complete branch stacks as histograms Andi Kleen
2014-10-20  7:54   ` Jiri Olsa
2014-10-20 10:10   ` Jiri Olsa
2014-10-22  1:03   ` Namhyung Kim
2014-11-11 23:31     ` Andi Kleen
2014-11-17  6:23       ` Namhyung Kim
2014-11-18  2:08         ` Andi Kleen
2014-09-26 23:37 ` [PATCH 2/8] perf, tools: Add --branch-history option to report Andi Kleen
2014-10-20  7:54   ` Jiri Olsa
2014-10-20 17:51   ` Jiri Olsa
2014-10-22  1:26   ` Namhyung Kim
2014-11-12  0:05     ` Andi Kleen
2014-11-17  6:31       ` Namhyung Kim
2014-11-18  2:01         ` Andi Kleen
2014-09-26 23:37 ` [PATCH 3/8] perf, tools: Enable printing the srcline in the history Andi Kleen
2014-10-20 17:57   ` Jiri Olsa
2014-09-26 23:37 ` [PATCH 4/8] perf, tools: Only print base source file for srcline Andi Kleen
2014-09-26 23:37 ` [PATCH 5/8] perf, tools: Support source line numbers in annotate Andi Kleen
2014-10-20 18:06   ` Jiri Olsa [this message]
2014-10-20 18:17     ` Jiri Olsa
2014-11-12  0:36     ` Andi Kleen
2014-09-26 23:37 ` [PATCH 6/8] tools, perf: Make get_srcline fall back to sym+offset Andi Kleen
2014-09-26 23:37 ` [PATCH 7/8] tools, perf: Make srcline output address with -v Andi Kleen
2014-10-20 18:20   ` Jiri Olsa
2014-09-26 23:37 ` [PATCH 8/8] tools, perf: Add asprintf replacement Andi Kleen
2014-10-20 18:28   ` Jiri Olsa
2014-10-20 19:13     ` Geert Uytterhoeven
2014-10-20 19:21       ` Jiri Olsa
2014-10-20 19:33         ` Geert Uytterhoeven
  -- strict thread matches above, loose matches on Subject: below --
2014-09-15 23:54 Implement lbr-as-callgraph v9 Andi Kleen
2014-09-15 23:54 ` [PATCH 5/8] perf, tools: Support source line numbers in annotate Andi Kleen
2014-03-11  1:05 Implement lbr-as-callgraph v5 Andi Kleen
2014-03-11  1:05 ` [PATCH 5/8] perf, tools: Support source line numbers in annotate Andi Kleen

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=20141020180605.GC9913@krava.brq.redhat.com \
    --to=jolsa@redhat.com \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=andi@firstfloor.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.