[PATCH 1/2] perf newt: Use newtGetScreenSize

[PATCH 1/2] perf newt: Use newtGetScreenSize

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

For consistency, use the newt API more fully.

Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/newt.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index 2a4308a..f6ec6f5 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -52,7 +52,7 @@ static size_t hist_entry__append_browser(struct hist_entry *self,
 static void hist_entry__annotate_browser(struct hist_entry *self)
 {
 	FILE *fp;
-	struct winsize ws;
+	int cols, rows;
 	newtComponent form, listbox;
 	struct newtExitStruct es;
 	char *str;
@@ -71,8 +71,8 @@ static void hist_entry__annotate_browser(struct hist_entry *self)
 		goto out_free_str;
 
 	newtPushHelpLine("Press ESC to exit");
-	get_term_dimensions(&ws);
-	listbox = newtListbox(0, 0, ws.ws_row - 5, NEWT_FLAG_SCROLL);
+	newtGetScreenSize(&cols, &rows);
+	listbox = newtListbox(0, 0, rows - 5, NEWT_FLAG_SCROLL);
 
 	while (!feof(fp)) {
 		if (getline(&line, &line_len, fp) < 0 || !line_len)
@@ -87,13 +87,13 @@ static void hist_entry__annotate_browser(struct hist_entry *self)
 	fclose(fp);
 	free(line);
 
-	max_usable_width = ws.ws_col - 22;
+	max_usable_width = cols - 22;
 	if (max_line_len > max_usable_width)
 		max_line_len = max_usable_width;
 
 	newtListboxSetWidth(listbox, max_line_len);
 
-	newtCenteredWindow(max_line_len + 2, ws.ws_row - 5, self->sym->name);
+	newtCenteredWindow(max_line_len + 2, rows - 5, self->sym->name);
 	form = newt_form__new();
 	newtFormAddComponents(form, listbox, NULL);
 
@@ -112,7 +112,7 @@ void perf_session__browse_hists(struct rb_root *hists, u64 session_total,
 	struct rb_node *nd;
 	unsigned int width;
 	char *col_width = symbol_conf.col_width_list_str;
-	struct winsize ws;
+	int rows;
 	size_t max_len = 0;
 	char str[1024];
 	newtComponent form, listbox;
@@ -122,13 +122,13 @@ void perf_session__browse_hists(struct rb_root *hists, u64 session_total,
 	newtDrawRootText(0, 0, str);
 	newtPushHelpLine(helpline);
 
-	get_term_dimensions(&ws);
+	newtGetScreenSize(NULL, &rows);
 
 	form = newt_form__new();
 
-	listbox = newtListbox(1, 1, ws.ws_row - 2, (NEWT_FLAG_SCROLL |
-						    NEWT_FLAG_BORDER |
-						    NEWT_FLAG_RETURNEXIT));
+	listbox = newtListbox(1, 1, rows - 2, (NEWT_FLAG_SCROLL |
+					       NEWT_FLAG_BORDER |
+					       NEWT_FLAG_RETURNEXIT));
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		if (se->elide)
-- 
1.6.2.5

[PATCH 2/2] perf hist: Don't fprintf the callgraph unconditionally

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

  [root@doppio ~]# perf report -i newt.data | head -10
  # Samples: 11999679868
  #
  # Overhead  Command                  Shared Object  Symbol
  # ........  .......  .............................  ......
  #
      63.61%     perf  libslang.so.2.1.4              [.] SLsmg_write_chars
       6.30%     perf  perf                           [.] symbols__find
       2.19%     perf  libnewt.so.0.52.10             [.] newtListboxAppendEntry
       2.08%     perf  libslang.so.2.1.4              [.] SLsmg_write_chars@plt
       1.99%     perf  libc-2.10.2.so                 [.] _IO_vfprintf_internal
  [root@doppio ~]#

Not good, the newt form for report works, but slang has to eat the cost of the
additional callgraph lines everytime it prints a line, and the callgraph
doesn't appear on the screen, so move the callgraph printing to a separate
function and don't use it in newt.c.

Newt tree widgets are being investigated to properly support callgraphs, but
till that gets merged, lets remove this huge overhead and show at least the
symbol overheads for a callgraph rich perf.data with good performance.

Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c |   30 +++++++++++++++++-------------
 1 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index d43be34..1a4e837 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -532,23 +532,23 @@ size_t hist_entry__fprintf(struct hist_entry *self,
 		ret += se->print(fp, self, se->width ? *se->width : 0);
 	}
 
-	ret += fprintf(fp, "\n");
-
-	if (symbol_conf.use_callchain) {
-		int left_margin = 0;
+	return ret + fprintf(fp, "\n");
+}
 
-		if (sort__first_dimension == SORT_COMM) {
-			se = list_first_entry(&hist_entry__sort_list, typeof(*se),
-						list);
-			left_margin = se->width ? *se->width : 0;
-			left_margin -= thread__comm_len(self->thread);
-		}
+static size_t hist_entry__fprintf_callchain(struct hist_entry *self, FILE *fp,
+					    u64 session_total)
+{
+	int left_margin = 0;
 
-		ret += hist_entry_callchain__fprintf(fp, self, session_total,
-						     left_margin);
+	if (sort__first_dimension == SORT_COMM) {
+		struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
+							 typeof(*se), list);
+		left_margin = se->width ? *se->width : 0;
+		left_margin -= thread__comm_len(self->thread);
 	}
 
-	return ret;
+	return hist_entry_callchain__fprintf(fp, self, session_total,
+					     left_margin);
 }
 
 size_t perf_session__fprintf_hists(struct rb_root *hists,
@@ -655,6 +655,10 @@ print_entries:
 		}
 		ret += hist_entry__fprintf(h, pair, show_displacement,
 					   displacement, fp, session_total);
+
+		if (symbol_conf.use_callchain)
+			ret += hist_entry__fprintf_callchain(h, fp, session_total);
+
 		if (h->map == NULL && verbose > 1) {
 			__map_groups__fprintf_maps(&h->thread->mg,
 						   MAP__FUNCTION, fp);
-- 
1.6.2.5

Re: [PATCH 2/2] perf hist: Don't fprintf the callgraph unconditionally

[Frederic Weisbecker]

On Fri, Mar 12, 2010 at 12:46:48PM -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
>   [root@doppio ~]# perf report -i newt.data | head -10
>   # Samples: 11999679868
>   #
>   # Overhead  Command                  Shared Object  Symbol
>   # ........  .......  .............................  ......
>   #
>       63.61%     perf  libslang.so.2.1.4              [.] SLsmg_write_chars
>        6.30%     perf  perf                           [.] symbols__find
>        2.19%     perf  libnewt.so.0.52.10             [.] newtListboxAppendEntry
>        2.08%     perf  libslang.so.2.1.4              [.] SLsmg_write_chars@plt
>        1.99%     perf  libc-2.10.2.so                 [.] _IO_vfprintf_internal
>   [root@doppio ~]#
> 
> Not good, the newt form for report works, but slang has to eat the cost of the
> additional callgraph lines everytime it prints a line, and the callgraph
> doesn't appear on the screen, so move the callgraph printing to a separate
> function and don't use it in newt.c.
> 
> Newt tree widgets are being investigated to properly support callgraphs, but
> till that gets merged, lets remove this huge overhead and show at least the
> symbol overheads for a callgraph rich perf.data with good performance.
> 
> Cc: Frédéric Weisbecker <fweisbec@gmail.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>


Hmm, so for now the callchain won't work anymore until
it gets properly reimplemented right?

Not that much a problem as a necessary step if it really makes
easier for the newt integration.

Re: [PATCH 2/2] perf hist: Don't fprintf the callgraph unconditionally

[Arnaldo Carvalho de Melo]

Em Sun, Mar 14, 2010 at 10:47:34AM +0100, Frederic Weisbecker escreveu:
> Hmm, so for now the callchain won't work anymore until
> it gets properly reimplemented right?

It doesn't work in the newt interface only, it remais working if you do:

perf report | less
perf report > output.txt
rpm -e newt-devel ; make -C tools/perf install

:-)
 
> Not that much a problem as a necessary step if it really makes
> easier for the newt integration.

This patch doesn't change the situation before it, i.e. callgraphs _in
the newt interface_ weren't working.

- Arnaldo

Re: [PATCH 2/2] perf hist: Don't fprintf the callgraph unconditionally

[Frederic Weisbecker]

On Sun, Mar 14, 2010 at 11:21:52AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, Mar 14, 2010 at 10:47:34AM +0100, Frederic Weisbecker escreveu:
> > Hmm, so for now the callchain won't work anymore until
> > it gets properly reimplemented right?
> 
> It doesn't work in the newt interface only, it remais working if you do:
> 
> perf report | less
> perf report > output.txt
> rpm -e newt-devel ; make -C tools/perf install
> 
> :-)


Aah!


  
> > Not that much a problem as a necessary step if it really makes
> > easier for the newt integration.
> 
> This patch doesn't change the situation before it, i.e. callgraphs _in
> the newt interface_ weren't working.


Ok :-)

Re: [PATCH 2/2] perf hist: Don't fprintf the callgraph unconditionally

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:

> Em Sun, Mar 14, 2010 at 10:47:34AM +0100, Frederic Weisbecker escreveu:
> > Hmm, so for now the callchain won't work anymore until
> > it gets properly reimplemented right?
> 
> It doesn't work in the newt interface only, it remais working if you do:
> 
> perf report | less
> perf report > output.txt
> rpm -e newt-devel ; make -C tools/perf install
> 
> :-)

We'll also need a .perfconfig variable for people who prefer flat ascii.

	Ingo