Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754857AbdC2Ixx (ORCPT ); Wed, 29 Mar 2017 04:53:53 -0400 Received: from mail-pg0-f54.google.com ([74.125.83.54]:36778 "EHLO mail-pg0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752168AbdC2Ixu (ORCPT ); Wed, 29 Mar 2017 04:53:50 -0400 Subject: Re: [PATCH v4] perf annotate: Fix missing number of samples for source_line_samples To: Arnaldo Carvalho de Melo References: <1490703125-13643-1-git-send-email-treeze.taeung@gmail.com> <20170328134824.GA17135@kernel.org> <20170328152837.GB17135@kernel.org> Cc: linux-kernel@vger.kernel.org, Jiri Olsa , Namhyung Kim , Ingo Molnar , Peter Zijlstra , Wang Nan , Masami Hiramatsu , Jiri Olsa , Martin Liska From: Taeung Song Message-ID: Date: Wed, 29 Mar 2017 17:53:42 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2576 Lines: 97 Arnaldo, I think we had a little communication problem. So would you mind if I finally sum up this situation ? Currently there are the two calculation setting for 'number of samples'. (At util/annotate.c, not TUI code ui/browsers/annotate.c) A) disasm__calc_percent() -> util/annotate.c:881~ B) symbol__get_source_line() -> util/annotate.c:1634~ If testing them as the three cases, 1) $ perf annotate --stdio -l --show-total-period 2) $ perf annotate --stdio --show-total-period 3) $ perf annotate # using 'k' on TUI The result is, 1) NG 2) OK 3) OK The problem of '1)' is to just show '0' zero values for number of samples. Because 'B)' has a bug missing 'nr' of source_line_samples. So I fix it. the case '1)' uses 'B)' and 'A)' the case '2)' uses only 'A)' the case '3)' uses only 'A)' I read and analyzed all about 'number of samples' of perf-anntoate. with the cset 0c4a5bcea460 you gave me. This is my conclusion. What do you think about it ? Thanks, Taeung NOTE: 'A)'(i.e. disasm__calc_percent()) has two alternative logic when using '-l' option or not as below. 888 if (src_line) { ... 903 } else { ... 914 } On 03/29/2017 01:15 AM, Taeung Song wrote: > > > On 03/29/2017 12:28 AM, Arnaldo Carvalho de Melo wrote: >> Em Tue, Mar 28, 2017 at 11:21:33PM +0900, Taeung Song escreveu: >>> >>> >>> On 03/28/2017 10:48 PM, Arnaldo Carvalho de Melo wrote: >>>> Em Tue, Mar 28, 2017 at 09:12:05PM +0900, Taeung Song escreveu: >>>>> The option 'show-total-period' works fine without a option '-l'. >>>>> But if running 'perf annotate --stdio -l --show-total-period', >>>>> you can see a problem showing only zero '0' for number of samples. >>>> >>>> If you do that at that point, don't you have to change the TUI not >>>> to do >>>> it again? >>>> >>>> - Arnaldo >>>> >>> >>> Yes, there isn't the same problem when using 't' on TUI of 'perf >>> annotate'. >>> (As you know, 't' is show-total-period option) >> >> Yeah, there is no problem on the TUI, but since you do it in a function >> taht the TUI uses, it may not be needed to do this in > > No, TUI don't use the function I modified in this patch at all. > (i.e. symbol__get_source_line()) > > Only symbol__tty_annotate() use symbol__get_source_line(). > > (As you know, annotate TUI use symbol__tui_annotate() ) > >> tools/perf/util/annotate.c _and_ in tools/perf/ui/browsers/annotate.c, >> right? >> >> - Arnaldo > > > Are there something I missed or misunderstand ? > > Thanks, > Taeung > SNIP