Re: [PATCH 00/12] perf annotate: Add support for event group view (v2)

2013-03-12 Thread Arnaldo Carvalho de Melo
Em Tue, Mar 12, 2013 at 12:21:27PM +0900, Namhyung Kim escreveu:
> On Mon, 11 Mar 2013 17:53:16 -0300, Arnaldo Carvalho de Melo wrote:
> > I need to go thru this more thoroughly, as at least one thing caught my
> > attention, in some cases ->nr_pcnt is used and in others
> > evsel->nr_members, do we need both?

> Probably not.  I was thought keeping an array and its length together is
> better.  But it's rather a waste to save the same information to every
> date structure so I move it to the struct annotate_browser->nr_events.
 
> For evsel->nr_members, current code sets it only for grouping is enabled
> so the individual events will have value of 0.  Also it's not pass the
> evsel to every function need it.  But if you think it should really be
> fixed that way I can make the change to pass and use evsel->nr_members.

This can be left for later, my point here is that we should try to avoid
the various UIs from diverging too much, i.e. the code for --gtk,
--stdio and --tui should be as close and using the same abstractions as
possible.
 
> Anyway, below is a fixed version that could survived my testing.

indeed, works, compared the output of --stdio --group with --tui --group
and it matches.

While looking at it I noticed that the stdio output wastes the initial
columns, at some point we may want to make the hist_entry lines match
the TUI ones, so that going back and forth on two xterm tabs shows no
differences.

Anyway, thanks, applied!

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/12] perf annotate: Add support for event group view (v2)

2013-03-12 Thread Arnaldo Carvalho de Melo
Em Tue, Mar 12, 2013 at 12:21:27PM +0900, Namhyung Kim escreveu:
 On Mon, 11 Mar 2013 17:53:16 -0300, Arnaldo Carvalho de Melo wrote:
  I need to go thru this more thoroughly, as at least one thing caught my
  attention, in some cases -nr_pcnt is used and in others
  evsel-nr_members, do we need both?

 Probably not.  I was thought keeping an array and its length together is
 better.  But it's rather a waste to save the same information to every
 date structure so I move it to the struct annotate_browser-nr_events.
 
 For evsel-nr_members, current code sets it only for grouping is enabled
 so the individual events will have value of 0.  Also it's not pass the
 evsel to every function need it.  But if you think it should really be
 fixed that way I can make the change to pass and use evsel-nr_members.

This can be left for later, my point here is that we should try to avoid
the various UIs from diverging too much, i.e. the code for --gtk,
--stdio and --tui should be as close and using the same abstractions as
possible.
 
 Anyway, below is a fixed version that could survived my testing.

indeed, works, compared the output of --stdio --group with --tui --group
and it matches.

While looking at it I noticed that the stdio output wastes the initial
columns, at some point we may want to make the hist_entry lines match
the TUI ones, so that going back and forth on two xterm tabs shows no
differences.

Anyway, thanks, applied!

- Arnaldo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/12] perf annotate: Add support for event group view (v2)

2013-03-11 Thread Namhyung Kim
Hi Arnaldo,

On Mon, 11 Mar 2013 17:53:16 -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 05, 2013 at 02:53:20PM +0900, Namhyung Kim escreveu:
>> This patchset implements event group view on perf annotate.  It's
>> basically a rebased version and major difference to prior version is
>> the GTK annotation browser support.
>
>> Here goes an example:
>
>>  $ perf annotate --group --stdio
>
>>   crtstuff.c:0
>>  8.082.405.29 :387dc0aa50:   mov%rdi,%rdx
>
>
> I applied this series minus the TUI enabling one, as it is not working,
> so right now my perf/core has --group --stdio and --group --gtk working,
> please take a look at --group --tui.

Thanks, I took a look at the TUI code and found some missing pieces and
bugs.  Those code were in the original version but seems to get lost
during the cherry-picking and I tested wrong version. :(

>
> I need to go thru this more thoroughly, as at least one thing caught my
> attention, in some cases ->nr_pcnt is used and in others
> evsel->nr_members, do we need both?

Probably not.  I was thought keeping an array and its length together is
better.  But it's rather a waste to save the same information to every
date structure so I move it to the struct annotate_browser->nr_events.

For evsel->nr_members, current code sets it only for grouping is enabled
so the individual events will have value of 0.  Also it's not pass the
evsel to every function need it.  But if you think it should really be
fixed that way I can make the change to pass and use evsel->nr_members.

Anyway, below is a fixed version that could survived my testing.



>From 9d2948c274c4958ebde866adb28951f44e0595eb Mon Sep 17 00:00:00 2001
From: Namhyung Kim 
Date: Sat, 10 Nov 2012 01:21:02 +0900
Subject: [PATCH] perf annotate browser: Support event group view on TUI

Dynamically allocate browser_disasm_line according to a number of
group members.  This way we can handle multiple events in a general
manner.

Signed-off-by: Namhyung Kim 
---
 tools/perf/ui/browsers/annotate.c | 93 +++
 1 file changed, 75 insertions(+), 18 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c 
b/tools/perf/ui/browsers/annotate.c
index 8b16926dd56e..f56247a03a22 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -17,6 +17,10 @@ struct browser_disasm_line {
u32 idx;
int idx_asm;
int jump_sources;
+   /*
+* actual length of this array is saved on the nr_events field
+* of the struct annotate_browser
+*/
double  percent[1];
 };
 
@@ -34,8 +38,9 @@ struct annotate_browser {
struct ui_browser b;
struct rb_rootentries;
struct rb_node*curr_hot;
-   struct disasm_line*selection;
+   struct disasm_line  *selection;
struct disasm_line  **offsets;
+   int nr_events;
u64 start;
int nr_asm_entries;
int nr_entries;
@@ -95,14 +100,24 @@ static void annotate_browser__write(struct ui_browser 
*browser, void *entry, int
 (!current_entry || (browser->use_navkeypressed &&
 !browser->navkeypressed)));
int width = browser->width, printed;
+   int i, pcnt_width = 7 * ab->nr_events;
+   double percent_max = 0.0;
char bf[256];
 
-   if (dl->offset != -1 && bdl->percent[0] != 0.0) {
-   ui_browser__set_percent_color(browser, bdl->percent[0], 
current_entry);
-   slsmg_printf("%6.2f ", bdl->percent[0]);
+   for (i = 0; i < ab->nr_events; i++) {
+   if (bdl->percent[i] > percent_max)
+   percent_max = bdl->percent[i];
+   }
+
+   if (dl->offset != -1 && percent_max != 0.0) {
+   for (i = 0; i < ab->nr_events; i++) {
+   ui_browser__set_percent_color(browser, bdl->percent[i],
+ current_entry);
+   slsmg_printf("%6.2f ", bdl->percent[i]);
+   }
} else {
ui_browser__set_percent_color(browser, 0, current_entry);
-   slsmg_write_nstring(" ", 7);
+   slsmg_write_nstring(" ", pcnt_width);
}
 
SLsmg_write_char(' ');
@@ -112,12 +127,12 @@ static void annotate_browser__write(struct ui_browser 
*browser, void *entry, int
width += 1;
 
if (!*dl->line)
-   slsmg_write_nstring(" ", width - 7);
+   slsmg_write_nstring(" ", width - pcnt_width);
else if (dl->offset == -1) {
printed = scnprintf(bf, sizeof(bf), "%*s  ",
ab->addr_width, " ");
slsmg_write_nstring(bf, printed);
-   slsmg_write_nstring(dl->line, width - printed - 6);
+   

Re: [PATCH 00/12] perf annotate: Add support for event group view (v2)

2013-03-11 Thread Arnaldo Carvalho de Melo
Em Tue, Mar 05, 2013 at 02:53:20PM +0900, Namhyung Kim escreveu:
> This patchset implements event group view on perf annotate.  It's
> basically a rebased version and major difference to prior version is
> the GTK annotation browser support.

> Here goes an example:

>  $ perf annotate --group --stdio

>   crtstuff.c:0
>  8.082.405.29 :387dc0aa50:   mov%rdi,%rdx


I applied this series minus the TUI enabling one, as it is not working,
so right now my perf/core has --group --stdio and --group --gtk working,
please take a look at --group --tui.

I need to go thru this more thoroughly, as at least one thing caught my
attention, in some cases ->nr_pcnt is used and in others
evsel->nr_members, do we need both?

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/12] perf annotate: Add support for event group view (v2)

2013-03-11 Thread Arnaldo Carvalho de Melo
Em Tue, Mar 05, 2013 at 02:53:20PM +0900, Namhyung Kim escreveu:
 This patchset implements event group view on perf annotate.  It's
 basically a rebased version and major difference to prior version is
 the GTK annotation browser support.

 Here goes an example:

  $ perf annotate --group --stdio

   crtstuff.c:0
  8.082.405.29 :387dc0aa50:   mov%rdi,%rdx


I applied this series minus the TUI enabling one, as it is not working,
so right now my perf/core has --group --stdio and --group --gtk working,
please take a look at --group --tui.

I need to go thru this more thoroughly, as at least one thing caught my
attention, in some cases -nr_pcnt is used and in others
evsel-nr_members, do we need both?

- Arnaldo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/12] perf annotate: Add support for event group view (v2)

2013-03-11 Thread Namhyung Kim
Hi Arnaldo,

On Mon, 11 Mar 2013 17:53:16 -0300, Arnaldo Carvalho de Melo wrote:
 Em Tue, Mar 05, 2013 at 02:53:20PM +0900, Namhyung Kim escreveu:
 This patchset implements event group view on perf annotate.  It's
 basically a rebased version and major difference to prior version is
 the GTK annotation browser support.

 Here goes an example:

  $ perf annotate --group --stdio

   crtstuff.c:0
  8.082.405.29 :387dc0aa50:   mov%rdi,%rdx


 I applied this series minus the TUI enabling one, as it is not working,
 so right now my perf/core has --group --stdio and --group --gtk working,
 please take a look at --group --tui.

Thanks, I took a look at the TUI code and found some missing pieces and
bugs.  Those code were in the original version but seems to get lost
during the cherry-picking and I tested wrong version. :(


 I need to go thru this more thoroughly, as at least one thing caught my
 attention, in some cases -nr_pcnt is used and in others
 evsel-nr_members, do we need both?

Probably not.  I was thought keeping an array and its length together is
better.  But it's rather a waste to save the same information to every
date structure so I move it to the struct annotate_browser-nr_events.

For evsel-nr_members, current code sets it only for grouping is enabled
so the individual events will have value of 0.  Also it's not pass the
evsel to every function need it.  But if you think it should really be
fixed that way I can make the change to pass and use evsel-nr_members.

Anyway, below is a fixed version that could survived my testing.



From 9d2948c274c4958ebde866adb28951f44e0595eb Mon Sep 17 00:00:00 2001
From: Namhyung Kim namhy...@kernel.org
Date: Sat, 10 Nov 2012 01:21:02 +0900
Subject: [PATCH] perf annotate browser: Support event group view on TUI

Dynamically allocate browser_disasm_line according to a number of
group members.  This way we can handle multiple events in a general
manner.

Signed-off-by: Namhyung Kim namhy...@kernel.org
---
 tools/perf/ui/browsers/annotate.c | 93 +++
 1 file changed, 75 insertions(+), 18 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c 
b/tools/perf/ui/browsers/annotate.c
index 8b16926dd56e..f56247a03a22 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -17,6 +17,10 @@ struct browser_disasm_line {
u32 idx;
int idx_asm;
int jump_sources;
+   /*
+* actual length of this array is saved on the nr_events field
+* of the struct annotate_browser
+*/
double  percent[1];
 };
 
@@ -34,8 +38,9 @@ struct annotate_browser {
struct ui_browser b;
struct rb_rootentries;
struct rb_node*curr_hot;
-   struct disasm_line*selection;
+   struct disasm_line  *selection;
struct disasm_line  **offsets;
+   int nr_events;
u64 start;
int nr_asm_entries;
int nr_entries;
@@ -95,14 +100,24 @@ static void annotate_browser__write(struct ui_browser 
*browser, void *entry, int
 (!current_entry || (browser-use_navkeypressed 
 !browser-navkeypressed)));
int width = browser-width, printed;
+   int i, pcnt_width = 7 * ab-nr_events;
+   double percent_max = 0.0;
char bf[256];
 
-   if (dl-offset != -1  bdl-percent[0] != 0.0) {
-   ui_browser__set_percent_color(browser, bdl-percent[0], 
current_entry);
-   slsmg_printf(%6.2f , bdl-percent[0]);
+   for (i = 0; i  ab-nr_events; i++) {
+   if (bdl-percent[i]  percent_max)
+   percent_max = bdl-percent[i];
+   }
+
+   if (dl-offset != -1  percent_max != 0.0) {
+   for (i = 0; i  ab-nr_events; i++) {
+   ui_browser__set_percent_color(browser, bdl-percent[i],
+ current_entry);
+   slsmg_printf(%6.2f , bdl-percent[i]);
+   }
} else {
ui_browser__set_percent_color(browser, 0, current_entry);
-   slsmg_write_nstring( , 7);
+   slsmg_write_nstring( , pcnt_width);
}
 
SLsmg_write_char(' ');
@@ -112,12 +127,12 @@ static void annotate_browser__write(struct ui_browser 
*browser, void *entry, int
width += 1;
 
if (!*dl-line)
-   slsmg_write_nstring( , width - 7);
+   slsmg_write_nstring( , width - pcnt_width);
else if (dl-offset == -1) {
printed = scnprintf(bf, sizeof(bf), %*s  ,
ab-addr_width,  );
slsmg_write_nstring(bf, printed);
-   slsmg_write_nstring(dl-line, width - printed - 6);
+   slsmg_write_nstring(dl-line, width 

[PATCH 00/12] perf annotate: Add support for event group view (v2)

2013-03-04 Thread Namhyung Kim
Hi all,

This patchset implements event group view on perf annotate.  It's
basically a rebased version and major difference to prior version is
the GTK annotation browser support.

Here goes an example:

 $ perf annotate --group --stdio

  Percent |  Source code & Disassembly of libpthread-2.15.so
 

  :
  :
  :
  :  Disassembly of section .text:
  :
  :  00387dc0aa50 
<__pthread_mutex_unlock_usercnt>:
  crtstuff.c:0
 8.082.405.29 :387dc0aa50:   mov%rdi,%rdx
 0.000.000.00 :387dc0aa53:   mov0x10(%rdi),%edi
 0.000.000.00 :387dc0aa56:   mov%edi,%eax
  crtstuff.c:0
 0.000.800.00 :387dc0aa58:   and$0x7f,%eax
 3.032.403.53 :387dc0aa5b:   test   $0x7c,%dil
 0.000.000.00 :387dc0aa5f:   jne387dc0aaa9 
<__pthread_mutex_unlock_use
 0.000.000.00 :387dc0aa61:   test   %eax,%eax
 0.000.000.00 :387dc0aa63:   jne387dc0aa85 
<__pthread_mutex_unlock_use
 0.000.000.00 :387dc0aa65:   and$0x80,%edi
 0.000.000.00 :387dc0aa6b:   test   %esi,%esi
  crtstuff.c:0
 3.035.607.06 :387dc0aa6d:   movl   $0x0,0x8(%rdx)
  crtstuff.c:0
 0.000.000.59 :387dc0aa74:   je 387dc0aa7a 
<__pthread_mutex_unlock_use
 0.000.000.00 :387dc0aa76:   subl   $0x1,0xc(%rdx)
  crtstuff.c:0
 2.025.601.18 :387dc0aa7a:   mov%edi,%esi
 0.000.000.00 :387dc0aa7c:   lock decl (%rdx)
83.84   83.20   82.35 :387dc0aa7f:   jne387dc0aada 
<_L_unlock_586>
 0.000.000.00 :387dc0aa81:   nop
 0.000.000.00 :387dc0aa82:   xor%eax,%eax
 0.000.000.00 :387dc0aa84:   retq   
 ...


You can access it via perf/annotate-group-v2 branch on my tree

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Any comments are welcome, thanks
Namhyung


Namhyung Kim (12):
  perf annotate: Pass evsel instead of evidx on annotation functions
  perf annotate: Add a comment on the symbol__parse_objdump_line()
  perf annotate: Factor out disasm__calc_percent()
  perf annotate: Cleanup disasm__calc_percent()
  perf annotate: Add basic support to event group view
  perf evsel: Introduce perf_evsel__is_group_event() helper
  perf annotate: Factor out struct source_line_percent
  perf annotate: Support event group view for --print-line
  perf annotate browser: Make browser_disasm_line->percent an array
  perf annotate browser: Use disasm__calc_percent()
  perf annotate browser: Support event group view on TUI
  perf annotate/gtk: Support event group view on GTK

 tools/perf/Documentation/perf-annotate.txt |   3 +
 tools/perf/builtin-annotate.c  |  23 ++-
 tools/perf/builtin-report.c|   2 +-
 tools/perf/builtin-top.c   |   2 +-
 tools/perf/ui/browsers/annotate.c  | 139 +--
 tools/perf/ui/browsers/hists.c |   6 +-
 tools/perf/ui/gtk/annotate.c   |  26 ++-
 tools/perf/ui/gtk/hists.c  |   7 +-
 tools/perf/ui/hist.c   |   7 +-
 tools/perf/util/annotate.c | 262 ++---
 tools/perf/util/annotate.h |  49 +++---
 tools/perf/util/evsel.h|  24 +++
 tools/perf/util/hist.h |   5 +-
 13 files changed, 389 insertions(+), 166 deletions(-)

-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 00/12] perf annotate: Add support for event group view (v2)

2013-03-04 Thread Namhyung Kim
Hi all,

This patchset implements event group view on perf annotate.  It's
basically a rebased version and major difference to prior version is
the GTK annotation browser support.

Here goes an example:

 $ perf annotate --group --stdio

  Percent |  Source code  Disassembly of libpthread-2.15.so
 

  :
  :
  :
  :  Disassembly of section .text:
  :
  :  00387dc0aa50 
__pthread_mutex_unlock_usercnt:
  crtstuff.c:0
 8.082.405.29 :387dc0aa50:   mov%rdi,%rdx
 0.000.000.00 :387dc0aa53:   mov0x10(%rdi),%edi
 0.000.000.00 :387dc0aa56:   mov%edi,%eax
  crtstuff.c:0
 0.000.800.00 :387dc0aa58:   and$0x7f,%eax
 3.032.403.53 :387dc0aa5b:   test   $0x7c,%dil
 0.000.000.00 :387dc0aa5f:   jne387dc0aaa9 
__pthread_mutex_unlock_use
 0.000.000.00 :387dc0aa61:   test   %eax,%eax
 0.000.000.00 :387dc0aa63:   jne387dc0aa85 
__pthread_mutex_unlock_use
 0.000.000.00 :387dc0aa65:   and$0x80,%edi
 0.000.000.00 :387dc0aa6b:   test   %esi,%esi
  crtstuff.c:0
 3.035.607.06 :387dc0aa6d:   movl   $0x0,0x8(%rdx)
  crtstuff.c:0
 0.000.000.59 :387dc0aa74:   je 387dc0aa7a 
__pthread_mutex_unlock_use
 0.000.000.00 :387dc0aa76:   subl   $0x1,0xc(%rdx)
  crtstuff.c:0
 2.025.601.18 :387dc0aa7a:   mov%edi,%esi
 0.000.000.00 :387dc0aa7c:   lock decl (%rdx)
83.84   83.20   82.35 :387dc0aa7f:   jne387dc0aada 
_L_unlock_586
 0.000.000.00 :387dc0aa81:   nop
 0.000.000.00 :387dc0aa82:   xor%eax,%eax
 0.000.000.00 :387dc0aa84:   retq   
 ...


You can access it via perf/annotate-group-v2 branch on my tree

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Any comments are welcome, thanks
Namhyung


Namhyung Kim (12):
  perf annotate: Pass evsel instead of evidx on annotation functions
  perf annotate: Add a comment on the symbol__parse_objdump_line()
  perf annotate: Factor out disasm__calc_percent()
  perf annotate: Cleanup disasm__calc_percent()
  perf annotate: Add basic support to event group view
  perf evsel: Introduce perf_evsel__is_group_event() helper
  perf annotate: Factor out struct source_line_percent
  perf annotate: Support event group view for --print-line
  perf annotate browser: Make browser_disasm_line-percent an array
  perf annotate browser: Use disasm__calc_percent()
  perf annotate browser: Support event group view on TUI
  perf annotate/gtk: Support event group view on GTK

 tools/perf/Documentation/perf-annotate.txt |   3 +
 tools/perf/builtin-annotate.c  |  23 ++-
 tools/perf/builtin-report.c|   2 +-
 tools/perf/builtin-top.c   |   2 +-
 tools/perf/ui/browsers/annotate.c  | 139 +--
 tools/perf/ui/browsers/hists.c |   6 +-
 tools/perf/ui/gtk/annotate.c   |  26 ++-
 tools/perf/ui/gtk/hists.c  |   7 +-
 tools/perf/ui/hist.c   |   7 +-
 tools/perf/util/annotate.c | 262 ++---
 tools/perf/util/annotate.h |  49 +++---
 tools/perf/util/evsel.h|  24 +++
 tools/perf/util/hist.h |   5 +-
 13 files changed, 389 insertions(+), 166 deletions(-)

-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/