mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Taeung Song <treeze.taeung@gmail.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Jiri Olsa <jolsa@kernel.org>, Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Wang Nan <wangnan0@huawei.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Jiri Olsa <jolsa@redhat.com>
Subject: Re: [PATCH 2/4] perf annotate: Align filename:linenr and more correct summary
Date: Thu, 23 Feb 2017 01:31:33 +0900	[thread overview]
Message-ID: <e24c6f80-c4a8-4a81-8bec-b03cd8661e39@gmail.com> (raw)
In-Reply-To: <CAM9d7ciwSXAc7gx84jahMLmyCGU05VuC-=8dEV68N2Dz9Jc_YA@mail.gmail.com>



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

  reply	other threads:[~2017-02-22 16:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-22 10:08 [PATCH 0/4] perf annotate: Fixes for line numbers and Introduce source_code Taeung Song
2017-02-22 10:08 ` [PATCH 1/4] perf annotate: Remove needless regular expression for filename:linenr Taeung Song
2017-02-22 10:47   ` Namhyung Kim
2017-02-22 16:00     ` Taeung Song
2017-02-22 10:08 ` [PATCH 2/4] perf annotate: Align filename:linenr and more correct summary Taeung Song
2017-02-22 11:12   ` Namhyung Kim
2017-02-22 11:22   ` Namhyung Kim
2017-02-22 16:31     ` Taeung Song [this message]
2017-02-22 10:08 ` [PATCH 3/4] perf annotate: Change the method counting line numbers Taeung Song
2017-02-22 10:08 ` [PATCH 4/4] perf annotate: Introduce source_code to collect actual code Taeung Song
2017-02-22 11:27   ` Namhyung Kim
2017-02-22 16:41     ` Taeung Song
2017-02-24  5:57   ` Ravi Bangoria

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=e24c6f80-c4a8-4a81-8bec-b03cd8661e39@gmail.com \
    --to=treeze.taeung@gmail.com \
    --cc=acme@kernel.org \
    --cc=jolsa@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=wangnan0@huawei.com \
    /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

all inboxes | Powered by JetHome®