From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751688AbcIIClI convert rfc822-to-8bit (ORCPT ); Thu, 8 Sep 2016 22:41:08 -0400 Received: from mga01.intel.com ([192.55.52.88]:33425 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750974AbcIIClG (ORCPT ); Thu, 8 Sep 2016 22:41:06 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,303,1470726000"; d="scan'208";a="6390836" From: "Jin, Yao" To: Arnaldo Carvalho de Melo , Andi Kleen CC: Stephane Eranian , Peter Zijlstra , Ingo Molnar , LKML , Jiri Olsa , Linus Torvalds , David Carrillo-Cisneros , Alexander Shishkin , Namhyung Kim , "Liang, Kan" , Anshuman Khandual Subject: RE: [RFC][PATCH 7/7] perf/annotate: Add branch stack / basic block information Thread-Topic: [RFC][PATCH 7/7] perf/annotate: Add branch stack / basic block information Thread-Index: AQHSCfJjPDlr5bzh60uLeXnTVm74O6BvThKAgAEkPnA= Date: Fri, 9 Sep 2016 02:40:56 +0000 Message-ID: <0C6A3B2997299B4EAD81D386F36CD8C2492ED465@shsmsx102.ccr.corp.intel.com> References: <20160708133059.031522978@infradead.org> <20160708134113.718203556@infradead.org> <20160708145555.GB17466@gmail.com> <20160708162733.GJ30909@twins.programming.kicks-ass.net> <20160708163632.GK30927@twins.programming.kicks-ass.net> <20160908161857.GA4801@kernel.org> <20160908165914.GR5871@two.firstfloor.org> <20160908171129.GA32585@kernel.org> In-Reply-To: <20160908171129.GA32585@kernel.org> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, The idea of my patch is a little bit different. It targets to associate the branch coverage percentage and branch mispredict rate with the source code then user can see them directly in perf annotate source code / assembly view. The screenshot to show the branch%. https://github.com/yaoj/perf/blob/master/2.png The screenshot to show the mispredict rate. https://github.com/yaoj/perf/blob/master/3.png The --stdio mode is tested working well now and will do for --tui mode in next. The internal review comments require the patch to keep the existing "Percent" column unchanged and add new columns "Branch%" and "Mispred%" if LBRs are recorded in perf.data. For example, following 3 columns will be shown at default if LBRs are in perf.data when executing perf annotate --stdio. Percent | Branch% | Mispred% | With this comment, the patch needs to be changed. Please let me know what do you think for this patch and if I should go ahead. Thanks Jin Yao -----Original Message----- From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org] Sent: Friday, September 9, 2016 1:11 AM To: Andi Kleen Cc: Stephane Eranian ; Peter Zijlstra ; Ingo Molnar ; LKML ; Jiri Olsa ; Linus Torvalds ; David Carrillo-Cisneros ; Alexander Shishkin ; Namhyung Kim ; Liang, Kan ; Anshuman Khandual ; Jin, Yao Subject: Re: [RFC][PATCH 7/7] perf/annotate: Add branch stack / basic block information Em Thu, Sep 08, 2016 at 09:59:15AM -0700, Andi Kleen escreveu: > On Thu, Sep 08, 2016 at 09:43:53AM -0700, Stephane Eranian wrote: > > Hi, > > > > On Thu, Sep 8, 2016 at 9:18 AM, Arnaldo Carvalho de Melo > > wrote: > > > > > > Em Fri, Jul 08, 2016 at 06:36:32PM +0200, Peter Zijlstra escreveu: > > > > On Fri, Jul 08, 2016 at 06:27:33PM +0200, Peter Zijlstra wrote: > > > > > > > > > I've been thinking of filtering all targets and branches that > > > > > are smaller than 0.1% in order to avoid this, but so far I've > > > > > just been ignoring these things. > > > > > > > > Like so... seems to 'work'. > > > > > > So I merged this one with 7/7 and this is the result, screenshot > > > to capture the colors: > > > > > > http://vger.kernel.org/~acme/perf/annotate_basic_blocks.png > > > > > > Please let me know if I should go ahead and push with the combined > > > patch, that is now at: > > > > > > https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit > > > /?h=perf/annotate_basic_blocks&id=baf41a43fa439ac534d21e41882a7858 > > > d5cee1e5 > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git > > > perf/annotate_basic_blocks > > > > > > Is that ok? > > > > > I like the idea and yes, branch stack can be used for this, but I > > have a hard time understanding the colored output. > > What is the explanation for the color changes? > > How do I interpret the percentages in the comments of the assembly: > > -54.50% (p: 42%) > > Why not have dedicated columns before the assembly with proper column headers? > > Yes columns with headers are better. Jin Yao has been looking at this > and already has some patches. For --tui as well? - Arnaldo