[PATCHv2 0/5] perf tools tui: Display columns headers

[PATCHv2 0/5] perf tools tui: Display columns headers

[Jiri Olsa]

hi,
adding the way to display columns headers in perf TUI on
'H' press.

v2 changes:
  - fixed resize/popup issues (Namhyung)
  - display headers by default (Namhyung)
  - add 'show-headers' config file option to setup
    the headers displaying

Patches 1 and 2 are not necessary for the functionality,
they are just byproducts of another early way I tried,
but I think they could go in.

Also reachable in here:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  perf/core_headers

thanks,
jirka


Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
Jiri Olsa (5):
      perf tools: Factor ui_browser ops out of ui_browser struct
      perf tools: Remove ev_name argument from perf_evsel__hists_browse
      perf tools: Fix scrollbar refresh row index
      perf tools tui: Display columns header text on 'H' press
      perf tools: Add report.show-headers config file option

 tools/perf/builtin-report.c       |   4 ++++
 tools/perf/ui/browser.c           |  39 ++++++++++++++++++-------------
 tools/perf/ui/browser.h           |  28 +++++++++++++---------
 tools/perf/ui/browsers/annotate.c |  19 ++++++++-------
 tools/perf/ui/browsers/header.c   |   8 ++++---
 tools/perf/ui/browsers/hists.c    | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------
 tools/perf/ui/browsers/map.c      |   8 ++++---
 tools/perf/ui/browsers/scripts.c  |   2 +-
 tools/perf/ui/tui/util.c          |   8 ++++---
 tools/perf/util/symbol.c          |   1 +
 tools/perf/util/symbol.h          |   1 +
 11 files changed, 173 insertions(+), 68 deletions(-)

[PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct

[Jiri Olsa]

Separating ops out of 'struct ui_browser' into
'struct ui_browser_ops'.

Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/browser.c           | 37 ++++++++++++++++++++++---------------
 tools/perf/ui/browser.h           | 28 +++++++++++++++++-----------
 tools/perf/ui/browsers/annotate.c | 19 +++++++++++--------
 tools/perf/ui/browsers/header.c   |  8 +++++---
 tools/perf/ui/browsers/hists.c    | 14 ++++++++------
 tools/perf/ui/browsers/map.c      |  8 +++++---
 tools/perf/ui/browsers/scripts.c  |  2 +-
 tools/perf/ui/tui/util.c          |  8 +++++---
 8 files changed, 74 insertions(+), 50 deletions(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index 3ccf6e1..e6f96be 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -46,12 +46,18 @@ void ui_browser__gotorc(struct ui_browser *browser, int y, int x)
 	SLsmg_gotorc(browser->y + y, browser->x + x);
 }
 
+static bool ui_browser__filter(struct ui_browser *browser, void *entry)
+{
+	return browser->ops.filter ? browser->ops.filter(browser, entry) :
+				     false;
+}
+
 static struct list_head *
 ui_browser__list_head_filter_entries(struct ui_browser *browser,
 				     struct list_head *pos)
 {
 	do {
-		if (!browser->filter || !browser->filter(browser, pos))
+		if (!ui_browser__filter(browser, pos))
 			return pos;
 		pos = pos->next;
 	} while (pos != browser->entries);
@@ -64,7 +70,7 @@ ui_browser__list_head_filter_prev_entries(struct ui_browser *browser,
 					  struct list_head *pos)
 {
 	do {
-		if (!browser->filter || !browser->filter(browser, pos))
+		if (!ui_browser__filter(browser, pos))
 			return pos;
 		pos = pos->prev;
 	} while (pos != browser->entries);
@@ -149,7 +155,7 @@ unsigned int ui_browser__rb_tree_refresh(struct ui_browser *browser)
 
 	while (nd != NULL) {
 		ui_browser__gotorc(browser, row, 0);
-		browser->write(browser, nd, row);
+		browser->ops.write(browser, nd, row);
 		if (++row == browser->height)
 			break;
 		nd = rb_next(nd);
@@ -227,7 +233,7 @@ bool ui_browser__dialog_yesno(struct ui_browser *browser, const char *text)
 void ui_browser__reset_index(struct ui_browser *browser)
 {
 	browser->index = browser->top_idx = 0;
-	browser->seek(browser, 0, SEEK_SET);
+	browser->ops.seek(browser, 0, SEEK_SET);
 }
 
 void __ui_browser__show_title(struct ui_browser *browser, const char *title)
@@ -302,7 +308,7 @@ static int __ui_browser__refresh(struct ui_browser *browser)
 	int row;
 	int width = browser->width;
 
-	row = browser->refresh(browser);
+	row = browser->ops.refresh(browser);
 	ui_browser__set_color(browser, HE_COLORSET_NORMAL);
 
 	if (!browser->use_navkeypressed || browser->navkeypressed)
@@ -346,11 +352,12 @@ void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries)
 	}
 
 	browser->top = NULL;
-	browser->seek(browser, browser->top_idx, SEEK_SET);
+	browser->ops.seek(browser, browser->top_idx, SEEK_SET);
 }
 
 int ui_browser__run(struct ui_browser *browser, int delay_secs)
 {
+	struct ui_browser_ops *ops = &browser->ops;
 	int err, key;
 
 	while (1) {
@@ -391,7 +398,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
 			++browser->index;
 			if (browser->index == browser->top_idx + browser->height) {
 				++browser->top_idx;
-				browser->seek(browser, +1, SEEK_CUR);
+				ops->seek(browser, +1, SEEK_CUR);
 			}
 			break;
 		case K_UP:
@@ -400,7 +407,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
 			--browser->index;
 			if (browser->index < browser->top_idx) {
 				--browser->top_idx;
-				browser->seek(browser, -1, SEEK_CUR);
+				ops->seek(browser, -1, SEEK_CUR);
 			}
 			break;
 		case K_PGDN:
@@ -413,7 +420,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
 				offset = browser->nr_entries - 1 - browser->index;
 			browser->index += offset;
 			browser->top_idx += offset;
-			browser->seek(browser, +offset, SEEK_CUR);
+			ops->seek(browser, +offset, SEEK_CUR);
 			break;
 		case K_PGUP:
 			if (browser->top_idx == 0)
@@ -426,7 +433,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
 
 			browser->index -= offset;
 			browser->top_idx -= offset;
-			browser->seek(browser, -offset, SEEK_CUR);
+			ops->seek(browser, -offset, SEEK_CUR);
 			break;
 		case K_HOME:
 			ui_browser__reset_index(browser);
@@ -438,7 +445,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
 
 			browser->index = browser->nr_entries - 1;
 			browser->top_idx = browser->index - offset;
-			browser->seek(browser, -offset, SEEK_END);
+			ops->seek(browser, -offset, SEEK_END);
 			break;
 		default:
 			return key;
@@ -459,9 +466,9 @@ unsigned int ui_browser__list_head_refresh(struct ui_browser *browser)
 	pos = browser->top;
 
 	list_for_each_from(pos, head) {
-		if (!browser->filter || !browser->filter(browser, pos)) {
+		if (!ui_browser__filter(browser, pos)) {
 			ui_browser__gotorc(browser, row, 0);
-			browser->write(browser, pos, row);
+			browser->ops.write(browser, pos, row);
 			if (++row == browser->height)
 				break;
 		}
@@ -584,9 +591,9 @@ unsigned int ui_browser__argv_refresh(struct ui_browser *browser)
 
 	pos = (char **)browser->top;
 	while (idx < browser->nr_entries) {
-		if (!browser->filter || !browser->filter(browser, *pos)) {
+		if (!ui_browser__filter(browser, *pos)) {
 			ui_browser__gotorc(browser, row, 0);
-			browser->write(browser, pos, row);
+			browser->ops.write(browser, pos, row);
 			if (++row == browser->height)
 				break;
 		}
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index 03d4d62..5887ca7 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -11,21 +11,27 @@
 #define HE_COLORSET_ADDR	55
 #define HE_COLORSET_ROOT	56
 
-struct ui_browser {
-	u64	      index, top_idx;
-	void	      *top, *entries;
-	u16	      y, x, width, height;
-	int	      current_color;
-	void	      *priv;
-	const char    *title;
-	char	      *helpline;
+struct ui_browser;
+
+struct ui_browser_ops {
 	unsigned int  (*refresh)(struct ui_browser *browser);
 	void	      (*write)(struct ui_browser *browser, void *entry, int row);
 	void	      (*seek)(struct ui_browser *browser, off_t offset, int whence);
 	bool	      (*filter)(struct ui_browser *browser, void *entry);
-	u32	      nr_entries;
-	bool	      navkeypressed;
-	bool	      use_navkeypressed;
+};
+
+struct ui_browser {
+	u64			index, top_idx;
+	void			*top, *entries;
+	u16			y, x, width, height;
+	int			current_color;
+	void			*priv;
+	const char		*title;
+	char			*helpline;
+	u32			nr_entries;
+	bool			navkeypressed;
+	bool			use_navkeypressed;
+	struct ui_browser_ops	ops;
 };
 
 int  ui_browser__set_color(struct ui_browser *browser, int color);
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index f0697a3..bc4bcfa 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -388,8 +388,9 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
 	struct disasm_line *dl;
 	struct browser_disasm_line *bdl;
 	off_t offset = browser->b.index - browser->b.top_idx;
+	struct ui_browser_ops *ops = &browser->b.ops;
 
-	browser->b.seek(&browser->b, offset, SEEK_CUR);
+	ops->seek(&browser->b, offset, SEEK_CUR);
 	dl = list_entry(browser->b.top, struct disasm_line, node);
 	bdl = disasm_line__browser(dl);
 
@@ -399,13 +400,13 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
 
 		browser->b.nr_entries = browser->nr_entries;
 		annotate_browser__opts.hide_src_code = false;
-		browser->b.seek(&browser->b, -offset, SEEK_CUR);
+		ops->seek(&browser->b, -offset, SEEK_CUR);
 		browser->b.top_idx = bdl->idx - offset;
 		browser->b.index = bdl->idx;
 	} else {
 		if (bdl->idx_asm < 0) {
 			ui_helpline__puts("Only available for assembly lines.");
-			browser->b.seek(&browser->b, -offset, SEEK_CUR);
+			ops->seek(&browser->b, -offset, SEEK_CUR);
 			return false;
 		}
 
@@ -414,7 +415,7 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
 
 		browser->b.nr_entries = browser->nr_asm_entries;
 		annotate_browser__opts.hide_src_code = true;
-		browser->b.seek(&browser->b, -offset, SEEK_CUR);
+		ops->seek(&browser->b, -offset, SEEK_CUR);
 		browser->b.top_idx = bdl->idx_asm - offset;
 		browser->b.index = bdl->idx_asm;
 	}
@@ -882,10 +883,12 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
 	};
 	struct annotate_browser browser = {
 		.b = {
-			.refresh = annotate_browser__refresh,
-			.seek	 = ui_browser__list_head_seek,
-			.write	 = annotate_browser__write,
-			.filter  = disasm_line__filter,
+			.ops = {
+				.refresh = annotate_browser__refresh,
+				.seek	 = ui_browser__list_head_seek,
+				.write	 = annotate_browser__write,
+				.filter  = disasm_line__filter,
+			},
 			.priv	 = &ms,
 			.use_navkeypressed = true,
 		},
diff --git a/tools/perf/ui/browsers/header.c b/tools/perf/ui/browsers/header.c
index 89c16b9..69f1929 100644
--- a/tools/perf/ui/browsers/header.c
+++ b/tools/perf/ui/browsers/header.c
@@ -81,10 +81,12 @@ static int ui__list_menu(int argc, char * const argv[])
 {
 	struct ui_browser menu = {
 		.entries    = (void *)argv,
-		.refresh    = ui_browser__argv_refresh,
-		.seek	    = ui_browser__argv_seek,
-		.write	    = ui_browser__argv_write,
 		.nr_entries = argc,
+		.ops = {
+			.refresh    = ui_browser__argv_refresh,
+			.seek	    = ui_browser__argv_seek,
+			.write	    = ui_browser__argv_write,
+		},
 	};
 
 	return list_menu__run(&menu);
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 04a229a..7cb6309 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1190,8 +1190,8 @@ static struct hist_browser *hist_browser__new(struct hists *hists)
 
 	if (browser) {
 		browser->hists = hists;
-		browser->b.refresh = hist_browser__refresh;
-		browser->b.seek = ui_browser__hists_seek;
+		browser->b.ops.refresh = hist_browser__refresh;
+		browser->b.ops.seek = ui_browser__hists_seek;
 		browser->b.use_navkeypressed = true;
 	}
 
@@ -1947,11 +1947,13 @@ static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
 	struct perf_evsel *pos;
 	struct perf_evsel_menu menu = {
 		.b = {
+			.ops = {
+				.refresh    = ui_browser__list_head_refresh,
+				.seek	    = ui_browser__list_head_seek,
+				.write	    = perf_evsel_menu__write,
+				.filter	    = filter_group_entries,
+			},
 			.entries    = &evlist->entries,
-			.refresh    = ui_browser__list_head_refresh,
-			.seek	    = ui_browser__list_head_seek,
-			.write	    = perf_evsel_menu__write,
-			.filter	    = filter_group_entries,
 			.nr_entries = nr_entries,
 			.priv	    = evlist,
 		},
diff --git a/tools/perf/ui/browsers/map.c b/tools/perf/ui/browsers/map.c
index b11639f..6b09cd6 100644
--- a/tools/perf/ui/browsers/map.c
+++ b/tools/perf/ui/browsers/map.c
@@ -103,9 +103,11 @@ int map__browse(struct map *map)
 	struct map_browser mb = {
 		.b = {
 			.entries = &map->dso->symbols[map->type],
-			.refresh = ui_browser__rb_tree_refresh,
-			.seek	 = ui_browser__rb_tree_seek,
-			.write	 = map_browser__write,
+			.ops = {
+				.refresh = ui_browser__rb_tree_refresh,
+				.seek	 = ui_browser__rb_tree_seek,
+				.write	 = map_browser__write,
+			},
 		},
 		.map = map,
 	};
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 402d2bd..bbbc442 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -116,7 +116,7 @@ int script_browse(const char *script_opt)
 	struct script_line *sline;
 
 	struct perf_script_browser script = {
-		.b = {
+		.b.ops = {
 			.refresh    = ui_browser__list_head_refresh,
 			.seek	    = ui_browser__list_head_seek,
 			.write	    = script_browser__write,
diff --git a/tools/perf/ui/tui/util.c b/tools/perf/ui/tui/util.c
index bf890f7..8ac0465 100644
--- a/tools/perf/ui/tui/util.c
+++ b/tools/perf/ui/tui/util.c
@@ -60,10 +60,12 @@ int ui__popup_menu(int argc, char * const argv[])
 {
 	struct ui_browser menu = {
 		.entries    = (void *)argv,
-		.refresh    = ui_browser__argv_refresh,
-		.seek	    = ui_browser__argv_seek,
-		.write	    = ui_browser__argv_write,
 		.nr_entries = argc,
+		.ops        = {
+			.refresh    = ui_browser__argv_refresh,
+			.seek	    = ui_browser__argv_seek,
+			.write	    = ui_browser__argv_write,
+		},
 	};
 
 	return popup_menu__run(&menu);
-- 
1.8.3.1

[PATCH 2/5] perf tools: Remove ev_name argument from perf_evsel__hists_browse

[Jiri Olsa]

Removing ev_name argument from perf_evsel__hists_browse
function, because it's not needed. We can get the name
out of the 'struct perf_evsel' which is passed as
argument as well.

Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 7cb6309..6aeed29 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -33,8 +33,7 @@ struct hist_browser {
 
 extern void hist_browser__init_hpp(void);
 
-static int hists__browser_title(struct hists *hists, char *bf, size_t size,
-				const char *ev_name);
+static int hists__browser_title(struct hists *hists, char *bf, size_t size);
 static void hist_browser__update_nr_entries(struct hist_browser *hb);
 
 static struct rb_node *hists__filter_entries(struct rb_node *nd,
@@ -346,7 +345,7 @@ static void ui_browser__warn_lost_events(struct ui_browser *browser)
 		"Or reduce the sampling frequency.");
 }
 
-static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
+static int hist_browser__run(struct hist_browser *browser,
 			     struct hist_browser_timer *hbt)
 {
 	int key;
@@ -357,7 +356,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 	browser->b.nr_entries = hist_browser__nr_entries(browser);
 
 	hist_browser__refresh_dimensions(browser);
-	hists__browser_title(browser->hists, title, sizeof(title), ev_name);
+	hists__browser_title(browser->hists, title, sizeof(title));
 
 	if (ui_browser__show(&browser->b, title,
 			     "Press '?' for help on key bindings") < 0)
@@ -384,7 +383,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 				ui_browser__warn_lost_events(&browser->b);
 			}
 
-			hists__browser_title(browser->hists, title, sizeof(title), ev_name);
+			hists__browser_title(browser->hists, title, sizeof(title));
 			ui_browser__show_title(&browser->b, title);
 			continue;
 		}
@@ -1213,8 +1212,7 @@ static struct thread *hist_browser__selected_thread(struct hist_browser *browser
 	return browser->he_selection->thread;
 }
 
-static int hists__browser_title(struct hists *hists, char *bf, size_t size,
-				const char *ev_name)
+static int hists__browser_title(struct hists *hists, char *bf, size_t size)
 {
 	char unit;
 	int printed;
@@ -1223,6 +1221,7 @@ static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 	unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
 	u64 nr_events = hists->stats.total_period;
 	struct perf_evsel *evsel = hists_to_evsel(hists);
+	const char *ev_name = perf_evsel__name(evsel);
 	char buf[512];
 	size_t buflen = sizeof(buf);
 
@@ -1390,7 +1389,7 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb)
 }
 
 static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
-				    const char *helpline, const char *ev_name,
+				    const char *helpline,
 				    bool left_exits,
 				    struct hist_browser_timer *hbt,
 				    float min_pcnt,
@@ -1465,7 +1464,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 		nr_options = 0;
 
-		key = hist_browser__run(browser, ev_name, hbt);
+		key = hist_browser__run(browser, hbt);
 
 		if (browser->he_selection != NULL) {
 			thread = hist_browser__selected_thread(browser);
@@ -1843,7 +1842,7 @@ static int perf_evsel_menu__run(struct perf_evsel_menu *menu,
 {
 	struct perf_evlist *evlist = menu->b.priv;
 	struct perf_evsel *pos;
-	const char *ev_name, *title = "Available samples";
+	const char *title = "Available samples";
 	int delay_secs = hbt ? hbt->refresh : 0;
 	int key;
 
@@ -1876,9 +1875,8 @@ browse_hists:
 			 */
 			if (hbt)
 				hbt->timer(hbt->arg);
-			ev_name = perf_evsel__name(pos);
 			key = perf_evsel__hists_browse(pos, nr_events, help,
-						       ev_name, true, hbt,
+						       true, hbt,
 						       menu->min_pcnt,
 						       menu->env);
 			ui_browser__show_title(&menu->b, title);
@@ -1984,10 +1982,9 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 single_entry:
 	if (nr_entries == 1) {
 		struct perf_evsel *first = perf_evlist__first(evlist);
-		const char *ev_name = perf_evsel__name(first);
 
 		return perf_evsel__hists_browse(first, nr_entries, help,
-						ev_name, false, hbt, min_pcnt,
+						false, hbt, min_pcnt,
 						env);
 	}
 
-- 
1.8.3.1

[PATCH 3/5] perf tools: Fix scrollbar refresh row index

[Jiri Olsa]

The ui_browser__gotorc function needs offset from 'y' member,
so the row index has to begin with 0, which happens by accident
in current code, because we display only one header line.

The bug shows when we want to display more than 1 header lines
like columns headers in following patches.

Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/browser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index e6f96be..6792efb 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -285,7 +285,7 @@ static void ui_browser__scrollbar_set(struct ui_browser *browser)
 {
 	int height = browser->height, h = 0, pct = 0,
 	    col = browser->width,
-	    row = browser->y - 1;
+	    row = 0;
 
 	if (browser->nr_entries > 1) {
 		pct = ((browser->index * (browser->height - 1)) /
-- 
1.8.3.1

[PATCH 4/5] perf tools tui: Display columns header text on 'H' press

[Jiri Olsa]

Displaying columns header text whenever 'H' is pressed,
and hiding it on on another press. Displaying headers
by default.

Note I removed the original width setup pcode code in
hist_browser__refresh_dimensions function, because it
was never used and overwritten by ui_browser setup.
Also all the TUI output expect width ot be the current
terminal width.

Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 80 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 6aeed29..b923f61 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -26,6 +26,7 @@ struct hist_browser {
 	struct map_symbol   *selection;
 	int		     print_seq;
 	bool		     show_dso;
+	bool		     show_headers;
 	float		     min_pcnt;
 	u64		     nr_non_filtered_entries;
 	u64		     nr_callchain_rows;
@@ -58,9 +59,13 @@ static u32 hist_browser__nr_entries(struct hist_browser *hb)
 
 static void hist_browser__refresh_dimensions(struct hist_browser *browser)
 {
-	/* 3 == +/- toggle symbol before actual hist_entry rendering */
-	browser->b.width = 3 + (hists__sort_list_width(browser->hists) +
-			     sizeof("[k]"));
+	u16 header = browser->show_headers ? 1 : 0;
+
+	ui_browser__refresh_dimensions(&browser->b);
+
+	/* shrink view size if there are headers displayed */
+	browser->b.height = SLtt_Screen_Rows - 2 - header;
+	browser->b.y      = 1 + header;
 }
 
 static void hist_browser__reset(struct hist_browser *browser)
@@ -409,6 +414,9 @@ static int hist_browser__run(struct hist_browser *browser,
 			/* Expand the whole world. */
 			hist_browser__set_folding(browser, true);
 			break;
+		case 'H':
+			browser->show_headers = !browser->show_headers;
+			continue;
 		case K_ENTER:
 			if (hist_browser__toggle_fold(browser))
 				break;
@@ -787,6 +795,65 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	return printed;
 }
 
+static int advance_hpp_check(struct perf_hpp *hpp, int inc)
+{
+	advance_hpp(hpp, inc);
+	return hpp->size <= 0;
+}
+
+static int hists__scnprintf_headers(char *buf, size_t size, struct hists *hists)
+{
+	struct perf_hpp dummy_hpp = {
+		.buf    = buf,
+		.size   = size,
+	};
+	struct perf_hpp_fmt *fmt;
+	size_t ret = 0;
+
+	if (symbol_conf.use_callchain) {
+		ret = scnprintf(buf, size, "  ");
+		if (advance_hpp_check(&dummy_hpp, ret))
+			return ret;
+	}
+
+	perf_hpp__for_each_format(fmt) {
+		if (perf_hpp__should_skip(fmt))
+			continue;
+
+		/* We need to add the length of the columns header. */
+		perf_hpp__reset_width(fmt, hists);
+
+		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
+		if (advance_hpp_check(&dummy_hpp, ret))
+			break;
+
+		ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "  ");
+		if (advance_hpp_check(&dummy_hpp, ret))
+			break;
+	}
+
+	return ret;
+}
+
+static void ui_browser__show_headers(struct ui_browser *b, char *headers)
+{
+	SLsmg_gotorc(1, 0);
+	ui_browser__set_color(b, HE_COLORSET_ROOT);
+	slsmg_write_nstring(headers, b->width + 1);
+}
+
+static void hist_browser__show_headers(struct hist_browser *hb)
+{
+	static char buf[1024], *headers;
+
+	if (!headers) {
+		hists__scnprintf_headers(buf, sizeof(buf), hb->hists);
+		headers = buf;
+	}
+
+	ui_browser__show_headers(&hb->b, headers);
+}
+
 static void ui_browser__hists_init_top(struct ui_browser *browser)
 {
 	if (browser->top == NULL) {
@@ -803,6 +870,11 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 	struct rb_node *nd;
 	struct hist_browser *hb = container_of(browser, struct hist_browser, b);
 
+	hist_browser__refresh_dimensions(hb);
+
+	if (hb->show_headers)
+		hist_browser__show_headers(hb);
+
 	ui_browser__hists_init_top(browser);
 
 	for (nd = browser->top; nd; nd = rb_next(nd)) {
@@ -1192,6 +1264,7 @@ static struct hist_browser *hist_browser__new(struct hists *hists)
 		browser->b.ops.refresh = hist_browser__refresh;
 		browser->b.ops.seek = ui_browser__hists_seek;
 		browser->b.use_navkeypressed = true;
+		browser->show_headers = true;
 	}
 
 	return browser;
@@ -1421,6 +1494,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	"d             Zoom into current DSO\n"				\
 	"E             Expand all callchains\n"				\
 	"F             Toggle percentage of filtered entries\n"		\
+	"H             Display column headers\n"			\
 
 	/* help messages are sorted by lexical order of the hotkey */
 	const char report_help[] = HIST_BROWSER_HELP_COMMON
-- 
1.8.3.1

[PATCH 5/5] perf tools: Add report.show-headers config file option

[Jiri Olsa]

Adding report.show-headers config file option to setup
the appearance of the columns headers.

Currently columns headers are displayed by default, following
lines in ~/.perfconfig file will disable that:

  [report]
        show-headers = true

Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-report.c    | 4 ++++
 tools/perf/ui/browsers/hists.c | 6 +++++-
 tools/perf/util/symbol.c       | 1 +
 tools/perf/util/symbol.h       | 1 +
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 21d830b..cbc68c6 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -76,6 +76,10 @@ static int report__config(const char *var, const char *value, void *cb)
 		symbol_conf.cumulate_callchain = perf_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "report.show-headers")) {
+		symbol_conf.show_headers = perf_config_bool(var, value);
+		return 0;
+	}
 
 	return perf_default_config(var, value, cb);
 }
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index b923f61..a878284 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1260,11 +1260,15 @@ static struct hist_browser *hist_browser__new(struct hists *hists)
 	struct hist_browser *browser = zalloc(sizeof(*browser));
 
 	if (browser) {
+		/* If no config option is set, keep the default 'true'. */
+		bool show_headers = symbol_conf.show_headers == -1 ?
+				    true : symbol_conf.show_headers;
+
 		browser->hists = hists;
 		browser->b.ops.refresh = hist_browser__refresh;
 		browser->b.ops.seek = ui_browser__hists_seek;
 		browser->b.use_navkeypressed = true;
-		browser->show_headers = true;
+		browser->show_headers = show_headers;
 	}
 
 	return browser;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 7b9096f..45a319b 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -35,6 +35,7 @@ struct symbol_conf symbol_conf = {
 	.demangle		= true,
 	.cumulate_callchain	= true,
 	.symfs			= "",
+	.show_headers		= -1,
 };
 
 static enum dso_binary_type binary_type_symtab[] = {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 615c752..8f1854a 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -139,6 +139,7 @@ struct symbol_conf {
 			*sym_from_list,
 			*sym_to_list;
 	const char	*symfs;
+	int		show_headers;
 };
 
 extern struct symbol_conf symbol_conf;
-- 
1.8.3.1

Re: [PATCH 5/5] perf tools: Add report.show-headers config file option

[Namhyung Kim]

Hi Jiri,

2014-06-19 (목), 13:41 +0200, Jiri Olsa:
> Adding report.show-headers config file option to setup
> the appearance of the columns headers.
> 
> Currently columns headers are displayed by default, following
> lines in ~/.perfconfig file will disable that:
> 
>   [report]
>         show-headers = true

This also applies to perf top, right?  And it's TUI-specific.  So how
about calling it something like "tui.show-headers"?


[SNIP]
> @@ -35,6 +35,7 @@ struct symbol_conf symbol_conf = {
>  	.demangle		= true,
>  	.cumulate_callchain	= true,
>  	.symfs			= "",
> +	.show_headers		= -1,

Hmm.. why not just making it boolean and set it to true (like others)?

Thanks,
Namhyung


>  };
>  
>  static enum dso_binary_type binary_type_symtab[] = {
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 615c752..8f1854a 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -139,6 +139,7 @@ struct symbol_conf {
>  			*sym_from_list,
>  			*sym_to_list;
>  	const char	*symfs;
> +	int		show_headers;
>  };
>  
>  extern struct symbol_conf symbol_conf;

Re: [PATCH 5/5] perf tools: Add report.show-headers config file option

[Jiri Olsa]

On Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> 2014-06-19 (목), 13:41 +0200, Jiri Olsa:
> > Adding report.show-headers config file option to setup
> > the appearance of the columns headers.
> > 
> > Currently columns headers are displayed by default, following
> > lines in ~/.perfconfig file will disable that:
> > 
> >   [report]
> >         show-headers = true
> 
> This also applies to perf top, right?  And it's TUI-specific.  So how
> about calling it something like "tui.show-headers"?

ook

> 
> 
> [SNIP]
> > @@ -35,6 +35,7 @@ struct symbol_conf symbol_conf = {
> >  	.demangle		= true,
> >  	.cumulate_callchain	= true,
> >  	.symfs			= "",
> > +	.show_headers		= -1,
> 
> Hmm.. why not just making it boolean and set it to true (like others)?

I need 'unset' value, otherwise I dont know if the config
option was unset by user or not touched at all

jirka

Re: [PATCH 5/5] perf tools: Add report.show-headers config file option

[Namhyung Kim]

2014-06-19 (목), 15:02 +0200, Jiri Olsa:
> On Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim wrote:
> > [SNIP]
> > > @@ -35,6 +35,7 @@ struct symbol_conf symbol_conf = {
> > >  	.demangle		= true,
> > >  	.cumulate_callchain	= true,
> > >  	.symfs			= "",
> > > +	.show_headers		= -1,
> > 
> > Hmm.. why not just making it boolean and set it to true (like others)?
> 
> I need 'unset' value, otherwise I dont know if the config
> option was unset by user or not touched at all

I don't understand why you need it..  isn't it enough to check the
config option before using the value?

Thanks,
Namhyung

Re: [PATCH 5/5] perf tools: Add report.show-headers config file option

[Arnaldo Carvalho de Melo]

Em Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim escreveu:
> 2014-06-19 (목), 13:41 +0200, Jiri Olsa:
> > Adding report.show-headers config file option to setup
> > the appearance of the columns headers.
> > 
> > Currently columns headers are displayed by default, following
> > lines in ~/.perfconfig file will disable that:
> > 
> >   [report]
> >         show-headers = true
> 
> This also applies to perf top, right?  And it's TUI-specific.  So how
> about calling it something like "tui.show-headers"?

Why should it be TUI specific? You either show the headers or you don't,
in all UIs.

And also in all tools, I think, so I suggest putting it in some global,
for all tools, section in the config file, i.e. something like
all.show-headers.
 
- Arnaldo

Re: [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct

[Arnaldo Carvalho de Melo]

Em Thu, Jun 19, 2014 at 01:41:12PM +0200, Jiri Olsa escreveu:
> Separating ops out of 'struct ui_browser' into
> 'struct ui_browser_ops'.

You stated what you did, and that helps in understanding what this patch
is about, now we only need to have a paragraph on _why_ this is needed
:-)

So, what is the intent of this patch? Why do we need it?
 
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/ui/browser.c           | 37 ++++++++++++++++++++++---------------
>  tools/perf/ui/browser.h           | 28 +++++++++++++++++-----------
>  tools/perf/ui/browsers/annotate.c | 19 +++++++++++--------
>  tools/perf/ui/browsers/header.c   |  8 +++++---
>  tools/perf/ui/browsers/hists.c    | 14 ++++++++------
>  tools/perf/ui/browsers/map.c      |  8 +++++---
>  tools/perf/ui/browsers/scripts.c  |  2 +-
>  tools/perf/ui/tui/util.c          |  8 +++++---
>  8 files changed, 74 insertions(+), 50 deletions(-)
> 
> diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
> index 3ccf6e1..e6f96be 100644
> --- a/tools/perf/ui/browser.c
> +++ b/tools/perf/ui/browser.c
> @@ -46,12 +46,18 @@ void ui_browser__gotorc(struct ui_browser *browser, int y, int x)
>  	SLsmg_gotorc(browser->y + y, browser->x + x);
>  }
>  
> +static bool ui_browser__filter(struct ui_browser *browser, void *entry)
> +{
> +	return browser->ops.filter ? browser->ops.filter(browser, entry) :
> +				     false;
> +}
> +
>  static struct list_head *
>  ui_browser__list_head_filter_entries(struct ui_browser *browser,
>  				     struct list_head *pos)
>  {
>  	do {
> -		if (!browser->filter || !browser->filter(browser, pos))
> +		if (!ui_browser__filter(browser, pos))
>  			return pos;
>  		pos = pos->next;
>  	} while (pos != browser->entries);
> @@ -64,7 +70,7 @@ ui_browser__list_head_filter_prev_entries(struct ui_browser *browser,
>  					  struct list_head *pos)
>  {
>  	do {
> -		if (!browser->filter || !browser->filter(browser, pos))
> +		if (!ui_browser__filter(browser, pos))
>  			return pos;
>  		pos = pos->prev;
>  	} while (pos != browser->entries);
> @@ -149,7 +155,7 @@ unsigned int ui_browser__rb_tree_refresh(struct ui_browser *browser)
>  
>  	while (nd != NULL) {
>  		ui_browser__gotorc(browser, row, 0);
> -		browser->write(browser, nd, row);
> +		browser->ops.write(browser, nd, row);
>  		if (++row == browser->height)
>  			break;
>  		nd = rb_next(nd);
> @@ -227,7 +233,7 @@ bool ui_browser__dialog_yesno(struct ui_browser *browser, const char *text)
>  void ui_browser__reset_index(struct ui_browser *browser)
>  {
>  	browser->index = browser->top_idx = 0;
> -	browser->seek(browser, 0, SEEK_SET);
> +	browser->ops.seek(browser, 0, SEEK_SET);
>  }
>  
>  void __ui_browser__show_title(struct ui_browser *browser, const char *title)
> @@ -302,7 +308,7 @@ static int __ui_browser__refresh(struct ui_browser *browser)
>  	int row;
>  	int width = browser->width;
>  
> -	row = browser->refresh(browser);
> +	row = browser->ops.refresh(browser);
>  	ui_browser__set_color(browser, HE_COLORSET_NORMAL);
>  
>  	if (!browser->use_navkeypressed || browser->navkeypressed)
> @@ -346,11 +352,12 @@ void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries)
>  	}
>  
>  	browser->top = NULL;
> -	browser->seek(browser, browser->top_idx, SEEK_SET);
> +	browser->ops.seek(browser, browser->top_idx, SEEK_SET);
>  }
>  
>  int ui_browser__run(struct ui_browser *browser, int delay_secs)
>  {
> +	struct ui_browser_ops *ops = &browser->ops;
>  	int err, key;
>  
>  	while (1) {
> @@ -391,7 +398,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
>  			++browser->index;
>  			if (browser->index == browser->top_idx + browser->height) {
>  				++browser->top_idx;
> -				browser->seek(browser, +1, SEEK_CUR);
> +				ops->seek(browser, +1, SEEK_CUR);
>  			}
>  			break;
>  		case K_UP:
> @@ -400,7 +407,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
>  			--browser->index;
>  			if (browser->index < browser->top_idx) {
>  				--browser->top_idx;
> -				browser->seek(browser, -1, SEEK_CUR);
> +				ops->seek(browser, -1, SEEK_CUR);
>  			}
>  			break;
>  		case K_PGDN:
> @@ -413,7 +420,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
>  				offset = browser->nr_entries - 1 - browser->index;
>  			browser->index += offset;
>  			browser->top_idx += offset;
> -			browser->seek(browser, +offset, SEEK_CUR);
> +			ops->seek(browser, +offset, SEEK_CUR);
>  			break;
>  		case K_PGUP:
>  			if (browser->top_idx == 0)
> @@ -426,7 +433,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
>  
>  			browser->index -= offset;
>  			browser->top_idx -= offset;
> -			browser->seek(browser, -offset, SEEK_CUR);
> +			ops->seek(browser, -offset, SEEK_CUR);
>  			break;
>  		case K_HOME:
>  			ui_browser__reset_index(browser);
> @@ -438,7 +445,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
>  
>  			browser->index = browser->nr_entries - 1;
>  			browser->top_idx = browser->index - offset;
> -			browser->seek(browser, -offset, SEEK_END);
> +			ops->seek(browser, -offset, SEEK_END);
>  			break;
>  		default:
>  			return key;
> @@ -459,9 +466,9 @@ unsigned int ui_browser__list_head_refresh(struct ui_browser *browser)
>  	pos = browser->top;
>  
>  	list_for_each_from(pos, head) {
> -		if (!browser->filter || !browser->filter(browser, pos)) {
> +		if (!ui_browser__filter(browser, pos)) {
>  			ui_browser__gotorc(browser, row, 0);
> -			browser->write(browser, pos, row);
> +			browser->ops.write(browser, pos, row);
>  			if (++row == browser->height)
>  				break;
>  		}
> @@ -584,9 +591,9 @@ unsigned int ui_browser__argv_refresh(struct ui_browser *browser)
>  
>  	pos = (char **)browser->top;
>  	while (idx < browser->nr_entries) {
> -		if (!browser->filter || !browser->filter(browser, *pos)) {
> +		if (!ui_browser__filter(browser, *pos)) {
>  			ui_browser__gotorc(browser, row, 0);
> -			browser->write(browser, pos, row);
> +			browser->ops.write(browser, pos, row);
>  			if (++row == browser->height)
>  				break;
>  		}
> diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
> index 03d4d62..5887ca7 100644
> --- a/tools/perf/ui/browser.h
> +++ b/tools/perf/ui/browser.h
> @@ -11,21 +11,27 @@
>  #define HE_COLORSET_ADDR	55
>  #define HE_COLORSET_ROOT	56
>  
> -struct ui_browser {
> -	u64	      index, top_idx;
> -	void	      *top, *entries;
> -	u16	      y, x, width, height;
> -	int	      current_color;
> -	void	      *priv;
> -	const char    *title;
> -	char	      *helpline;
> +struct ui_browser;
> +
> +struct ui_browser_ops {
>  	unsigned int  (*refresh)(struct ui_browser *browser);
>  	void	      (*write)(struct ui_browser *browser, void *entry, int row);
>  	void	      (*seek)(struct ui_browser *browser, off_t offset, int whence);
>  	bool	      (*filter)(struct ui_browser *browser, void *entry);
> -	u32	      nr_entries;
> -	bool	      navkeypressed;
> -	bool	      use_navkeypressed;
> +};
> +
> +struct ui_browser {
> +	u64			index, top_idx;
> +	void			*top, *entries;
> +	u16			y, x, width, height;
> +	int			current_color;
> +	void			*priv;
> +	const char		*title;
> +	char			*helpline;
> +	u32			nr_entries;
> +	bool			navkeypressed;
> +	bool			use_navkeypressed;
> +	struct ui_browser_ops	ops;
>  };
>  
>  int  ui_browser__set_color(struct ui_browser *browser, int color);
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index f0697a3..bc4bcfa 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -388,8 +388,9 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
>  	struct disasm_line *dl;
>  	struct browser_disasm_line *bdl;
>  	off_t offset = browser->b.index - browser->b.top_idx;
> +	struct ui_browser_ops *ops = &browser->b.ops;
>  
> -	browser->b.seek(&browser->b, offset, SEEK_CUR);
> +	ops->seek(&browser->b, offset, SEEK_CUR);
>  	dl = list_entry(browser->b.top, struct disasm_line, node);
>  	bdl = disasm_line__browser(dl);
>  
> @@ -399,13 +400,13 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
>  
>  		browser->b.nr_entries = browser->nr_entries;
>  		annotate_browser__opts.hide_src_code = false;
> -		browser->b.seek(&browser->b, -offset, SEEK_CUR);
> +		ops->seek(&browser->b, -offset, SEEK_CUR);
>  		browser->b.top_idx = bdl->idx - offset;
>  		browser->b.index = bdl->idx;
>  	} else {
>  		if (bdl->idx_asm < 0) {
>  			ui_helpline__puts("Only available for assembly lines.");
> -			browser->b.seek(&browser->b, -offset, SEEK_CUR);
> +			ops->seek(&browser->b, -offset, SEEK_CUR);
>  			return false;
>  		}
>  
> @@ -414,7 +415,7 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
>  
>  		browser->b.nr_entries = browser->nr_asm_entries;
>  		annotate_browser__opts.hide_src_code = true;
> -		browser->b.seek(&browser->b, -offset, SEEK_CUR);
> +		ops->seek(&browser->b, -offset, SEEK_CUR);
>  		browser->b.top_idx = bdl->idx_asm - offset;
>  		browser->b.index = bdl->idx_asm;
>  	}
> @@ -882,10 +883,12 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
>  	};
>  	struct annotate_browser browser = {
>  		.b = {
> -			.refresh = annotate_browser__refresh,
> -			.seek	 = ui_browser__list_head_seek,
> -			.write	 = annotate_browser__write,
> -			.filter  = disasm_line__filter,
> +			.ops = {
> +				.refresh = annotate_browser__refresh,
> +				.seek	 = ui_browser__list_head_seek,
> +				.write	 = annotate_browser__write,
> +				.filter  = disasm_line__filter,
> +			},
>  			.priv	 = &ms,
>  			.use_navkeypressed = true,
>  		},
> diff --git a/tools/perf/ui/browsers/header.c b/tools/perf/ui/browsers/header.c
> index 89c16b9..69f1929 100644
> --- a/tools/perf/ui/browsers/header.c
> +++ b/tools/perf/ui/browsers/header.c
> @@ -81,10 +81,12 @@ static int ui__list_menu(int argc, char * const argv[])
>  {
>  	struct ui_browser menu = {
>  		.entries    = (void *)argv,
> -		.refresh    = ui_browser__argv_refresh,
> -		.seek	    = ui_browser__argv_seek,
> -		.write	    = ui_browser__argv_write,
>  		.nr_entries = argc,
> +		.ops = {
> +			.refresh    = ui_browser__argv_refresh,
> +			.seek	    = ui_browser__argv_seek,
> +			.write	    = ui_browser__argv_write,
> +		},
>  	};
>  
>  	return list_menu__run(&menu);
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 04a229a..7cb6309 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -1190,8 +1190,8 @@ static struct hist_browser *hist_browser__new(struct hists *hists)
>  
>  	if (browser) {
>  		browser->hists = hists;
> -		browser->b.refresh = hist_browser__refresh;
> -		browser->b.seek = ui_browser__hists_seek;
> +		browser->b.ops.refresh = hist_browser__refresh;
> +		browser->b.ops.seek = ui_browser__hists_seek;
>  		browser->b.use_navkeypressed = true;
>  	}
>  
> @@ -1947,11 +1947,13 @@ static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
>  	struct perf_evsel *pos;
>  	struct perf_evsel_menu menu = {
>  		.b = {
> +			.ops = {
> +				.refresh    = ui_browser__list_head_refresh,
> +				.seek	    = ui_browser__list_head_seek,
> +				.write	    = perf_evsel_menu__write,
> +				.filter	    = filter_group_entries,
> +			},
>  			.entries    = &evlist->entries,
> -			.refresh    = ui_browser__list_head_refresh,
> -			.seek	    = ui_browser__list_head_seek,
> -			.write	    = perf_evsel_menu__write,
> -			.filter	    = filter_group_entries,
>  			.nr_entries = nr_entries,
>  			.priv	    = evlist,
>  		},
> diff --git a/tools/perf/ui/browsers/map.c b/tools/perf/ui/browsers/map.c
> index b11639f..6b09cd6 100644
> --- a/tools/perf/ui/browsers/map.c
> +++ b/tools/perf/ui/browsers/map.c
> @@ -103,9 +103,11 @@ int map__browse(struct map *map)
>  	struct map_browser mb = {
>  		.b = {
>  			.entries = &map->dso->symbols[map->type],
> -			.refresh = ui_browser__rb_tree_refresh,
> -			.seek	 = ui_browser__rb_tree_seek,
> -			.write	 = map_browser__write,
> +			.ops = {
> +				.refresh = ui_browser__rb_tree_refresh,
> +				.seek	 = ui_browser__rb_tree_seek,
> +				.write	 = map_browser__write,
> +			},
>  		},
>  		.map = map,
>  	};
> diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> index 402d2bd..bbbc442 100644
> --- a/tools/perf/ui/browsers/scripts.c
> +++ b/tools/perf/ui/browsers/scripts.c
> @@ -116,7 +116,7 @@ int script_browse(const char *script_opt)
>  	struct script_line *sline;
>  
>  	struct perf_script_browser script = {
> -		.b = {
> +		.b.ops = {
>  			.refresh    = ui_browser__list_head_refresh,
>  			.seek	    = ui_browser__list_head_seek,
>  			.write	    = script_browser__write,
> diff --git a/tools/perf/ui/tui/util.c b/tools/perf/ui/tui/util.c
> index bf890f7..8ac0465 100644
> --- a/tools/perf/ui/tui/util.c
> +++ b/tools/perf/ui/tui/util.c
> @@ -60,10 +60,12 @@ int ui__popup_menu(int argc, char * const argv[])
>  {
>  	struct ui_browser menu = {
>  		.entries    = (void *)argv,
> -		.refresh    = ui_browser__argv_refresh,
> -		.seek	    = ui_browser__argv_seek,
> -		.write	    = ui_browser__argv_write,
>  		.nr_entries = argc,
> +		.ops        = {
> +			.refresh    = ui_browser__argv_refresh,
> +			.seek	    = ui_browser__argv_seek,
> +			.write	    = ui_browser__argv_write,
> +		},
>  	};
>  
>  	return popup_menu__run(&menu);
> -- 
> 1.8.3.1

Re: [PATCH 2/5] perf tools: Remove ev_name argument from perf_evsel__hists_browse

[Arnaldo Carvalho de Melo]

Em Thu, Jun 19, 2014 at 01:41:13PM +0200, Jiri Olsa escreveu:
> Removing ev_name argument from perf_evsel__hists_browse
> function, because it's not needed. We can get the name
> out of the 'struct perf_evsel' which is passed as
> argument as well.

See? What you did, why you did it, thanks! Applied, will push together
with a batch today.

- Arnaldo
 
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/ui/browsers/hists.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 7cb6309..6aeed29 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -33,8 +33,7 @@ struct hist_browser {
>  
>  extern void hist_browser__init_hpp(void);
>  
> -static int hists__browser_title(struct hists *hists, char *bf, size_t size,
> -				const char *ev_name);
> +static int hists__browser_title(struct hists *hists, char *bf, size_t size);
>  static void hist_browser__update_nr_entries(struct hist_browser *hb);
>  
>  static struct rb_node *hists__filter_entries(struct rb_node *nd,
> @@ -346,7 +345,7 @@ static void ui_browser__warn_lost_events(struct ui_browser *browser)
>  		"Or reduce the sampling frequency.");
>  }
>  
> -static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
> +static int hist_browser__run(struct hist_browser *browser,
>  			     struct hist_browser_timer *hbt)
>  {
>  	int key;
> @@ -357,7 +356,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
>  	browser->b.nr_entries = hist_browser__nr_entries(browser);
>  
>  	hist_browser__refresh_dimensions(browser);
> -	hists__browser_title(browser->hists, title, sizeof(title), ev_name);
> +	hists__browser_title(browser->hists, title, sizeof(title));
>  
>  	if (ui_browser__show(&browser->b, title,
>  			     "Press '?' for help on key bindings") < 0)
> @@ -384,7 +383,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
>  				ui_browser__warn_lost_events(&browser->b);
>  			}
>  
> -			hists__browser_title(browser->hists, title, sizeof(title), ev_name);
> +			hists__browser_title(browser->hists, title, sizeof(title));
>  			ui_browser__show_title(&browser->b, title);
>  			continue;
>  		}
> @@ -1213,8 +1212,7 @@ static struct thread *hist_browser__selected_thread(struct hist_browser *browser
>  	return browser->he_selection->thread;
>  }
>  
> -static int hists__browser_title(struct hists *hists, char *bf, size_t size,
> -				const char *ev_name)
> +static int hists__browser_title(struct hists *hists, char *bf, size_t size)
>  {
>  	char unit;
>  	int printed;
> @@ -1223,6 +1221,7 @@ static int hists__browser_title(struct hists *hists, char *bf, size_t size,
>  	unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
>  	u64 nr_events = hists->stats.total_period;
>  	struct perf_evsel *evsel = hists_to_evsel(hists);
> +	const char *ev_name = perf_evsel__name(evsel);
>  	char buf[512];
>  	size_t buflen = sizeof(buf);
>  
> @@ -1390,7 +1389,7 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb)
>  }
>  
>  static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
> -				    const char *helpline, const char *ev_name,
> +				    const char *helpline,
>  				    bool left_exits,
>  				    struct hist_browser_timer *hbt,
>  				    float min_pcnt,
> @@ -1465,7 +1464,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
>  
>  		nr_options = 0;
>  
> -		key = hist_browser__run(browser, ev_name, hbt);
> +		key = hist_browser__run(browser, hbt);
>  
>  		if (browser->he_selection != NULL) {
>  			thread = hist_browser__selected_thread(browser);
> @@ -1843,7 +1842,7 @@ static int perf_evsel_menu__run(struct perf_evsel_menu *menu,
>  {
>  	struct perf_evlist *evlist = menu->b.priv;
>  	struct perf_evsel *pos;
> -	const char *ev_name, *title = "Available samples";
> +	const char *title = "Available samples";
>  	int delay_secs = hbt ? hbt->refresh : 0;
>  	int key;
>  
> @@ -1876,9 +1875,8 @@ browse_hists:
>  			 */
>  			if (hbt)
>  				hbt->timer(hbt->arg);
> -			ev_name = perf_evsel__name(pos);
>  			key = perf_evsel__hists_browse(pos, nr_events, help,
> -						       ev_name, true, hbt,
> +						       true, hbt,
>  						       menu->min_pcnt,
>  						       menu->env);
>  			ui_browser__show_title(&menu->b, title);
> @@ -1984,10 +1982,9 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
>  single_entry:
>  	if (nr_entries == 1) {
>  		struct perf_evsel *first = perf_evlist__first(evlist);
> -		const char *ev_name = perf_evsel__name(first);
>  
>  		return perf_evsel__hists_browse(first, nr_entries, help,
> -						ev_name, false, hbt, min_pcnt,
> +						false, hbt, min_pcnt,
>  						env);
>  	}
>  
> -- 
> 1.8.3.1

Re: [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct

[Jiri Olsa]

On Thu, Jun 19, 2014 at 12:38:51PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 19, 2014 at 01:41:12PM +0200, Jiri Olsa escreveu:
> > Separating ops out of 'struct ui_browser' into
> > 'struct ui_browser_ops'.
> 
> You stated what you did, and that helps in understanding what this patch
> is about, now we only need to have a paragraph on _why_ this is needed
> :-)

well.. it's not ;-) but we always separated ops from the
structs IIRC

I introduced another callback in the early stage of this change,
but replaced it later.. this remained ;-)

no need to take it.. as I wrote in patch 0:

Patches 1 and 2 are not necessary for the functionality,
they are just byproducts of another early way I tried,
but I think they could go in.

jirka

> 
> So, what is the intent of this patch? Why do we need it?
>  
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> > Cc: David Ahern <dsahern@gmail.com>
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/perf/ui/browser.c           | 37 ++++++++++++++++++++++---------------
> >  tools/perf/ui/browser.h           | 28 +++++++++++++++++-----------
> >  tools/perf/ui/browsers/annotate.c | 19 +++++++++++--------
> >  tools/perf/ui/browsers/header.c   |  8 +++++---
> >  tools/perf/ui/browsers/hists.c    | 14 ++++++++------
> >  tools/perf/ui/browsers/map.c      |  8 +++++---
> >  tools/perf/ui/browsers/scripts.c  |  2 +-
> >  tools/perf/ui/tui/util.c          |  8 +++++---
> >  8 files changed, 74 insertions(+), 50 deletions(-)
> > 
> > diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
> > index 3ccf6e1..e6f96be 100644
> > --- a/tools/perf/ui/browser.c
> > +++ b/tools/perf/ui/browser.c
> > @@ -46,12 +46,18 @@ void ui_browser__gotorc(struct ui_browser *browser, int y, int x)
> >  	SLsmg_gotorc(browser->y + y, browser->x + x);
> >  }
> >  
> > +static bool ui_browser__filter(struct ui_browser *browser, void *entry)
> > +{
> > +	return browser->ops.filter ? browser->ops.filter(browser, entry) :
> > +				     false;
> > +}
> > +
> >  static struct list_head *
> >  ui_browser__list_head_filter_entries(struct ui_browser *browser,
> >  				     struct list_head *pos)
> >  {
> >  	do {
> > -		if (!browser->filter || !browser->filter(browser, pos))
> > +		if (!ui_browser__filter(browser, pos))
> >  			return pos;
> >  		pos = pos->next;
> >  	} while (pos != browser->entries);
> > @@ -64,7 +70,7 @@ ui_browser__list_head_filter_prev_entries(struct ui_browser *browser,
> >  					  struct list_head *pos)
> >  {
> >  	do {
> > -		if (!browser->filter || !browser->filter(browser, pos))
> > +		if (!ui_browser__filter(browser, pos))
> >  			return pos;
> >  		pos = pos->prev;
> >  	} while (pos != browser->entries);
> > @@ -149,7 +155,7 @@ unsigned int ui_browser__rb_tree_refresh(struct ui_browser *browser)
> >  
> >  	while (nd != NULL) {
> >  		ui_browser__gotorc(browser, row, 0);
> > -		browser->write(browser, nd, row);
> > +		browser->ops.write(browser, nd, row);
> >  		if (++row == browser->height)
> >  			break;
> >  		nd = rb_next(nd);
> > @@ -227,7 +233,7 @@ bool ui_browser__dialog_yesno(struct ui_browser *browser, const char *text)
> >  void ui_browser__reset_index(struct ui_browser *browser)
> >  {
> >  	browser->index = browser->top_idx = 0;
> > -	browser->seek(browser, 0, SEEK_SET);
> > +	browser->ops.seek(browser, 0, SEEK_SET);
> >  }
> >  
> >  void __ui_browser__show_title(struct ui_browser *browser, const char *title)
> > @@ -302,7 +308,7 @@ static int __ui_browser__refresh(struct ui_browser *browser)
> >  	int row;
> >  	int width = browser->width;
> >  
> > -	row = browser->refresh(browser);
> > +	row = browser->ops.refresh(browser);
> >  	ui_browser__set_color(browser, HE_COLORSET_NORMAL);
> >  
> >  	if (!browser->use_navkeypressed || browser->navkeypressed)
> > @@ -346,11 +352,12 @@ void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries)
> >  	}
> >  
> >  	browser->top = NULL;
> > -	browser->seek(browser, browser->top_idx, SEEK_SET);
> > +	browser->ops.seek(browser, browser->top_idx, SEEK_SET);
> >  }
> >  
> >  int ui_browser__run(struct ui_browser *browser, int delay_secs)
> >  {
> > +	struct ui_browser_ops *ops = &browser->ops;
> >  	int err, key;
> >  
> >  	while (1) {
> > @@ -391,7 +398,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
> >  			++browser->index;
> >  			if (browser->index == browser->top_idx + browser->height) {
> >  				++browser->top_idx;
> > -				browser->seek(browser, +1, SEEK_CUR);
> > +				ops->seek(browser, +1, SEEK_CUR);
> >  			}
> >  			break;
> >  		case K_UP:
> > @@ -400,7 +407,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
> >  			--browser->index;
> >  			if (browser->index < browser->top_idx) {
> >  				--browser->top_idx;
> > -				browser->seek(browser, -1, SEEK_CUR);
> > +				ops->seek(browser, -1, SEEK_CUR);
> >  			}
> >  			break;
> >  		case K_PGDN:
> > @@ -413,7 +420,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
> >  				offset = browser->nr_entries - 1 - browser->index;
> >  			browser->index += offset;
> >  			browser->top_idx += offset;
> > -			browser->seek(browser, +offset, SEEK_CUR);
> > +			ops->seek(browser, +offset, SEEK_CUR);
> >  			break;
> >  		case K_PGUP:
> >  			if (browser->top_idx == 0)
> > @@ -426,7 +433,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
> >  
> >  			browser->index -= offset;
> >  			browser->top_idx -= offset;
> > -			browser->seek(browser, -offset, SEEK_CUR);
> > +			ops->seek(browser, -offset, SEEK_CUR);
> >  			break;
> >  		case K_HOME:
> >  			ui_browser__reset_index(browser);
> > @@ -438,7 +445,7 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)
> >  
> >  			browser->index = browser->nr_entries - 1;
> >  			browser->top_idx = browser->index - offset;
> > -			browser->seek(browser, -offset, SEEK_END);
> > +			ops->seek(browser, -offset, SEEK_END);
> >  			break;
> >  		default:
> >  			return key;
> > @@ -459,9 +466,9 @@ unsigned int ui_browser__list_head_refresh(struct ui_browser *browser)
> >  	pos = browser->top;
> >  
> >  	list_for_each_from(pos, head) {
> > -		if (!browser->filter || !browser->filter(browser, pos)) {
> > +		if (!ui_browser__filter(browser, pos)) {
> >  			ui_browser__gotorc(browser, row, 0);
> > -			browser->write(browser, pos, row);
> > +			browser->ops.write(browser, pos, row);
> >  			if (++row == browser->height)
> >  				break;
> >  		}
> > @@ -584,9 +591,9 @@ unsigned int ui_browser__argv_refresh(struct ui_browser *browser)
> >  
> >  	pos = (char **)browser->top;
> >  	while (idx < browser->nr_entries) {
> > -		if (!browser->filter || !browser->filter(browser, *pos)) {
> > +		if (!ui_browser__filter(browser, *pos)) {
> >  			ui_browser__gotorc(browser, row, 0);
> > -			browser->write(browser, pos, row);
> > +			browser->ops.write(browser, pos, row);
> >  			if (++row == browser->height)
> >  				break;
> >  		}
> > diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
> > index 03d4d62..5887ca7 100644
> > --- a/tools/perf/ui/browser.h
> > +++ b/tools/perf/ui/browser.h
> > @@ -11,21 +11,27 @@
> >  #define HE_COLORSET_ADDR	55
> >  #define HE_COLORSET_ROOT	56
> >  
> > -struct ui_browser {
> > -	u64	      index, top_idx;
> > -	void	      *top, *entries;
> > -	u16	      y, x, width, height;
> > -	int	      current_color;
> > -	void	      *priv;
> > -	const char    *title;
> > -	char	      *helpline;
> > +struct ui_browser;
> > +
> > +struct ui_browser_ops {
> >  	unsigned int  (*refresh)(struct ui_browser *browser);
> >  	void	      (*write)(struct ui_browser *browser, void *entry, int row);
> >  	void	      (*seek)(struct ui_browser *browser, off_t offset, int whence);
> >  	bool	      (*filter)(struct ui_browser *browser, void *entry);
> > -	u32	      nr_entries;
> > -	bool	      navkeypressed;
> > -	bool	      use_navkeypressed;
> > +};
> > +
> > +struct ui_browser {
> > +	u64			index, top_idx;
> > +	void			*top, *entries;
> > +	u16			y, x, width, height;
> > +	int			current_color;
> > +	void			*priv;
> > +	const char		*title;
> > +	char			*helpline;
> > +	u32			nr_entries;
> > +	bool			navkeypressed;
> > +	bool			use_navkeypressed;
> > +	struct ui_browser_ops	ops;
> >  };
> >  
> >  int  ui_browser__set_color(struct ui_browser *browser, int color);
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index f0697a3..bc4bcfa 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -388,8 +388,9 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> >  	struct disasm_line *dl;
> >  	struct browser_disasm_line *bdl;
> >  	off_t offset = browser->b.index - browser->b.top_idx;
> > +	struct ui_browser_ops *ops = &browser->b.ops;
> >  
> > -	browser->b.seek(&browser->b, offset, SEEK_CUR);
> > +	ops->seek(&browser->b, offset, SEEK_CUR);
> >  	dl = list_entry(browser->b.top, struct disasm_line, node);
> >  	bdl = disasm_line__browser(dl);
> >  
> > @@ -399,13 +400,13 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> >  
> >  		browser->b.nr_entries = browser->nr_entries;
> >  		annotate_browser__opts.hide_src_code = false;
> > -		browser->b.seek(&browser->b, -offset, SEEK_CUR);
> > +		ops->seek(&browser->b, -offset, SEEK_CUR);
> >  		browser->b.top_idx = bdl->idx - offset;
> >  		browser->b.index = bdl->idx;
> >  	} else {
> >  		if (bdl->idx_asm < 0) {
> >  			ui_helpline__puts("Only available for assembly lines.");
> > -			browser->b.seek(&browser->b, -offset, SEEK_CUR);
> > +			ops->seek(&browser->b, -offset, SEEK_CUR);
> >  			return false;
> >  		}
> >  
> > @@ -414,7 +415,7 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
> >  
> >  		browser->b.nr_entries = browser->nr_asm_entries;
> >  		annotate_browser__opts.hide_src_code = true;
> > -		browser->b.seek(&browser->b, -offset, SEEK_CUR);
> > +		ops->seek(&browser->b, -offset, SEEK_CUR);
> >  		browser->b.top_idx = bdl->idx_asm - offset;
> >  		browser->b.index = bdl->idx_asm;
> >  	}
> > @@ -882,10 +883,12 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
> >  	};
> >  	struct annotate_browser browser = {
> >  		.b = {
> > -			.refresh = annotate_browser__refresh,
> > -			.seek	 = ui_browser__list_head_seek,
> > -			.write	 = annotate_browser__write,
> > -			.filter  = disasm_line__filter,
> > +			.ops = {
> > +				.refresh = annotate_browser__refresh,
> > +				.seek	 = ui_browser__list_head_seek,
> > +				.write	 = annotate_browser__write,
> > +				.filter  = disasm_line__filter,
> > +			},
> >  			.priv	 = &ms,
> >  			.use_navkeypressed = true,
> >  		},
> > diff --git a/tools/perf/ui/browsers/header.c b/tools/perf/ui/browsers/header.c
> > index 89c16b9..69f1929 100644
> > --- a/tools/perf/ui/browsers/header.c
> > +++ b/tools/perf/ui/browsers/header.c
> > @@ -81,10 +81,12 @@ static int ui__list_menu(int argc, char * const argv[])
> >  {
> >  	struct ui_browser menu = {
> >  		.entries    = (void *)argv,
> > -		.refresh    = ui_browser__argv_refresh,
> > -		.seek	    = ui_browser__argv_seek,
> > -		.write	    = ui_browser__argv_write,
> >  		.nr_entries = argc,
> > +		.ops = {
> > +			.refresh    = ui_browser__argv_refresh,
> > +			.seek	    = ui_browser__argv_seek,
> > +			.write	    = ui_browser__argv_write,
> > +		},
> >  	};
> >  
> >  	return list_menu__run(&menu);
> > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > index 04a229a..7cb6309 100644
> > --- a/tools/perf/ui/browsers/hists.c
> > +++ b/tools/perf/ui/browsers/hists.c
> > @@ -1190,8 +1190,8 @@ static struct hist_browser *hist_browser__new(struct hists *hists)
> >  
> >  	if (browser) {
> >  		browser->hists = hists;
> > -		browser->b.refresh = hist_browser__refresh;
> > -		browser->b.seek = ui_browser__hists_seek;
> > +		browser->b.ops.refresh = hist_browser__refresh;
> > +		browser->b.ops.seek = ui_browser__hists_seek;
> >  		browser->b.use_navkeypressed = true;
> >  	}
> >  
> > @@ -1947,11 +1947,13 @@ static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
> >  	struct perf_evsel *pos;
> >  	struct perf_evsel_menu menu = {
> >  		.b = {
> > +			.ops = {
> > +				.refresh    = ui_browser__list_head_refresh,
> > +				.seek	    = ui_browser__list_head_seek,
> > +				.write	    = perf_evsel_menu__write,
> > +				.filter	    = filter_group_entries,
> > +			},
> >  			.entries    = &evlist->entries,
> > -			.refresh    = ui_browser__list_head_refresh,
> > -			.seek	    = ui_browser__list_head_seek,
> > -			.write	    = perf_evsel_menu__write,
> > -			.filter	    = filter_group_entries,
> >  			.nr_entries = nr_entries,
> >  			.priv	    = evlist,
> >  		},
> > diff --git a/tools/perf/ui/browsers/map.c b/tools/perf/ui/browsers/map.c
> > index b11639f..6b09cd6 100644
> > --- a/tools/perf/ui/browsers/map.c
> > +++ b/tools/perf/ui/browsers/map.c
> > @@ -103,9 +103,11 @@ int map__browse(struct map *map)
> >  	struct map_browser mb = {
> >  		.b = {
> >  			.entries = &map->dso->symbols[map->type],
> > -			.refresh = ui_browser__rb_tree_refresh,
> > -			.seek	 = ui_browser__rb_tree_seek,
> > -			.write	 = map_browser__write,
> > +			.ops = {
> > +				.refresh = ui_browser__rb_tree_refresh,
> > +				.seek	 = ui_browser__rb_tree_seek,
> > +				.write	 = map_browser__write,
> > +			},
> >  		},
> >  		.map = map,
> >  	};
> > diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
> > index 402d2bd..bbbc442 100644
> > --- a/tools/perf/ui/browsers/scripts.c
> > +++ b/tools/perf/ui/browsers/scripts.c
> > @@ -116,7 +116,7 @@ int script_browse(const char *script_opt)
> >  	struct script_line *sline;
> >  
> >  	struct perf_script_browser script = {
> > -		.b = {
> > +		.b.ops = {
> >  			.refresh    = ui_browser__list_head_refresh,
> >  			.seek	    = ui_browser__list_head_seek,
> >  			.write	    = script_browser__write,
> > diff --git a/tools/perf/ui/tui/util.c b/tools/perf/ui/tui/util.c
> > index bf890f7..8ac0465 100644
> > --- a/tools/perf/ui/tui/util.c
> > +++ b/tools/perf/ui/tui/util.c
> > @@ -60,10 +60,12 @@ int ui__popup_menu(int argc, char * const argv[])
> >  {
> >  	struct ui_browser menu = {
> >  		.entries    = (void *)argv,
> > -		.refresh    = ui_browser__argv_refresh,
> > -		.seek	    = ui_browser__argv_seek,
> > -		.write	    = ui_browser__argv_write,
> >  		.nr_entries = argc,
> > +		.ops        = {
> > +			.refresh    = ui_browser__argv_refresh,
> > +			.seek	    = ui_browser__argv_seek,
> > +			.write	    = ui_browser__argv_write,
> > +		},
> >  	};
> >  
> >  	return popup_menu__run(&menu);
> > -- 
> > 1.8.3.1

Re: [PATCH 5/5] perf tools: Add report.show-headers config file option

[Jiri Olsa]

On Thu, Jun 19, 2014 at 10:09:59PM +0900, Namhyung Kim wrote:
> 2014-06-19 (목), 15:02 +0200, Jiri Olsa:
> > On Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim wrote:
> > > [SNIP]
> > > > @@ -35,6 +35,7 @@ struct symbol_conf symbol_conf = {
> > > >  	.demangle		= true,
> > > >  	.cumulate_callchain	= true,
> > > >  	.symfs			= "",
> > > > +	.show_headers		= -1,
> > > 
> > > Hmm.. why not just making it boolean and set it to true (like others)?
> > 
> > I need 'unset' value, otherwise I dont know if the config
> > option was unset by user or not touched at all
> 
> I don't understand why you need it..  isn't it enough to check the
> config option before using the value?

if config file option is not specified the show_headers
value will be 'false' ... which will cause headers not
to be displayed.. but they should be by default

jirka

Re: [PATCH 5/5] perf tools: Add report.show-headers config file option

[Jiri Olsa]

On Thu, Jun 19, 2014 at 12:28:43PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim escreveu:
> > 2014-06-19 (목), 13:41 +0200, Jiri Olsa:
> > > Adding report.show-headers config file option to setup
> > > the appearance of the columns headers.
> > > 
> > > Currently columns headers are displayed by default, following
> > > lines in ~/.perfconfig file will disable that:
> > > 
> > >   [report]
> > >         show-headers = true
> > 
> > This also applies to perf top, right?  And it's TUI-specific.  So how
> > about calling it something like "tui.show-headers"?
> 
> Why should it be TUI specific? You either show the headers or you don't,
> in all UIs.
> 
> And also in all tools, I think, so I suggest putting it in some global,
> for all tools, section in the config file, i.e. something like
> all.show-headers.

seems like so far we have 'core.' and 'hist.' as non-command specific
config sections (note '.core' does not hold any options yet)

I can add another 'all.' section

jirka

Re: [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct

[Namhyung Kim]

Hi Jiri,

On Thu, 19 Jun 2014 18:49:21 +0200, Jiri Olsa wrote:
> On Thu, Jun 19, 2014 at 12:38:51PM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Thu, Jun 19, 2014 at 01:41:12PM +0200, Jiri Olsa escreveu:
>> > Separating ops out of 'struct ui_browser' into
>> > 'struct ui_browser_ops'.
>> 
>> You stated what you did, and that helps in understanding what this patch
>> is about, now we only need to have a paragraph on _why_ this is needed
>> :-)
>
> well.. it's not ;-) but we always separated ops from the
> structs IIRC
>
> I introduced another callback in the early stage of this change,
> but replaced it later.. this remained ;-)
>
> no need to take it.. as I wrote in patch 0:
>
> Patches 1 and 2 are not necessary for the functionality,
> they are just byproducts of another early way I tried,
> but I think they could go in.

Hmm.. I think it should make the ops const, otherwise it makes only a
little sense.


[SNIP]
>> > +static bool ui_browser__filter(struct ui_browser *browser, void *entry)
>> > +{
>> > +	return browser->ops.filter ? browser->ops.filter(browser, entry) :
>> > +				     false;
>> > +}
>> > +
>> >  static struct list_head *
>> >  ui_browser__list_head_filter_entries(struct ui_browser *browser,
>> >  				     struct list_head *pos)
>> >  {
>> >  	do {
>> > -		if (!browser->filter || !browser->filter(browser, pos))
>> > +		if (!ui_browser__filter(browser, pos))

Looks like this can be a separate cleanup.

Thanks,
Namhyung


>> >  			return pos;
>> >  		pos = pos->next;
>> >  	} while (pos != browser->entries);

Re: [PATCH 5/5] perf tools: Add report.show-headers config file option

[Namhyung Kim]

On Thu, 19 Jun 2014 18:51:13 +0200, Jiri Olsa wrote:
> On Thu, Jun 19, 2014 at 10:09:59PM +0900, Namhyung Kim wrote:
>> 2014-06-19 (목), 15:02 +0200, Jiri Olsa:
>> > On Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim wrote:
>> > > [SNIP]
>> > > > @@ -35,6 +35,7 @@ struct symbol_conf symbol_conf = {
>> > > >  	.demangle		= true,
>> > > >  	.cumulate_callchain	= true,
>> > > >  	.symfs			= "",
>> > > > +	.show_headers		= -1,
>> > > 
>> > > Hmm.. why not just making it boolean and set it to true (like others)?
>> > 
>> > I need 'unset' value, otherwise I dont know if the config
>> > option was unset by user or not touched at all
>> 
>> I don't understand why you need it..  isn't it enough to check the
>> config option before using the value?
>
> if config file option is not specified the show_headers
> value will be 'false' ... 

Confused.  What makes it to be false if its default value is true and
the config option is not specified?

Thanks,
Namhyung


> which will cause headers not
> to be displayed.. but they should be by default
>
> jirka

Re: [PATCH 5/5] perf tools: Add report.show-headers config file option

[Namhyung Kim]

On Thu, 19 Jun 2014 18:58:15 +0200, Jiri Olsa wrote:
> On Thu, Jun 19, 2014 at 12:28:43PM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim escreveu:
>> > 2014-06-19 (목), 13:41 +0200, Jiri Olsa:
>> > > Adding report.show-headers config file option to setup
>> > > the appearance of the columns headers.
>> > > 
>> > > Currently columns headers are displayed by default, following
>> > > lines in ~/.perfconfig file will disable that:
>> > > 
>> > >   [report]
>> > >         show-headers = true
>> > 
>> > This also applies to perf top, right?  And it's TUI-specific.  So how
>> > about calling it something like "tui.show-headers"?
>> 
>> Why should it be TUI specific? You either show the headers or you don't,
>> in all UIs.
>> 
>> And also in all tools, I think, so I suggest putting it in some global,
>> for all tools, section in the config file, i.e. something like
>> all.show-headers.
>
> seems like so far we have 'core.' and 'hist.' as non-command specific
> config sections (note '.core' does not hold any options yet)
>
> I can add another 'all.' section

I don't think it's a better name - it's too general.  If we really want
to control all UI behavior I'd rather suggest 'ui.' section name.

Oh, I found that it'd somewhat clash with existing --header(-only)
option in perf report. :-/

Thanks,
Namhyung

Re: [PATCH 5/5] perf tools: Add report.show-headers config file option

[Jiri Olsa]

On Fri, Jun 20, 2014 at 08:46:58AM +0900, Namhyung Kim wrote:
> On Thu, 19 Jun 2014 18:58:15 +0200, Jiri Olsa wrote:
> > On Thu, Jun 19, 2014 at 12:28:43PM -0300, Arnaldo Carvalho de Melo wrote:
> >> Em Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim escreveu:
> >> > 2014-06-19 (목), 13:41 +0200, Jiri Olsa:
> >> > > Adding report.show-headers config file option to setup
> >> > > the appearance of the columns headers.
> >> > > 
> >> > > Currently columns headers are displayed by default, following
> >> > > lines in ~/.perfconfig file will disable that:
> >> > > 
> >> > >   [report]
> >> > >         show-headers = true
> >> > 
> >> > This also applies to perf top, right?  And it's TUI-specific.  So how
> >> > about calling it something like "tui.show-headers"?
> >> 
> >> Why should it be TUI specific? You either show the headers or you don't,
> >> in all UIs.
> >> 
> >> And also in all tools, I think, so I suggest putting it in some global,
> >> for all tools, section in the config file, i.e. something like
> >> all.show-headers.
> >
> > seems like so far we have 'core.' and 'hist.' as non-command specific
> > config sections (note '.core' does not hold any options yet)
> >
> > I can add another 'all.' section
> 
> I don't think it's a better name - it's too general.  If we really want
> to control all UI behavior I'd rather suggest 'ui.' section name.

I'm fine with 'ui'

> 
> Oh, I found that it'd somewhat clash with existing --header(-only)
> option in perf report. :-/

right, so how about:
   show-columns-headers
   show-columns-headings
   show-headings

jirka

[tip:perf/core] perf hists browser: Remove ev_name argument from perf_evsel__hists_browse

[tip-bot for Jiri Olsa]

Commit-ID:  dd00d486ddb7f181cf9487f6aceb1066bc6b0b6a
Gitweb:     http://git.kernel.org/tip/dd00d486ddb7f181cf9487f6aceb1066bc6b0b6a
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 19 Jun 2014 13:41:13 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 19 Jun 2014 16:13:14 -0300

perf hists browser: Remove ev_name argument from perf_evsel__hists_browse

Removing ev_name argument from perf_evsel__hists_browse function,
because it's not needed. We can get the name out of the 'struct
perf_evsel' which is passed as argument as well.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1403178076-14072-3-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 52c03fb..1bd35e8 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -32,8 +32,7 @@ struct hist_browser {
 
 extern void hist_browser__init_hpp(void);
 
-static int hists__browser_title(struct hists *hists, char *bf, size_t size,
-				const char *ev_name);
+static int hists__browser_title(struct hists *hists, char *bf, size_t size);
 static void hist_browser__update_nr_entries(struct hist_browser *hb);
 
 static struct rb_node *hists__filter_entries(struct rb_node *nd,
@@ -345,7 +344,7 @@ static void ui_browser__warn_lost_events(struct ui_browser *browser)
 		"Or reduce the sampling frequency.");
 }
 
-static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
+static int hist_browser__run(struct hist_browser *browser,
 			     struct hist_browser_timer *hbt)
 {
 	int key;
@@ -356,7 +355,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 	browser->b.nr_entries = hist_browser__nr_entries(browser);
 
 	hist_browser__refresh_dimensions(browser);
-	hists__browser_title(browser->hists, title, sizeof(title), ev_name);
+	hists__browser_title(browser->hists, title, sizeof(title));
 
 	if (ui_browser__show(&browser->b, title,
 			     "Press '?' for help on key bindings") < 0)
@@ -383,7 +382,7 @@ static int hist_browser__run(struct hist_browser *browser, const char *ev_name,
 				ui_browser__warn_lost_events(&browser->b);
 			}
 
-			hists__browser_title(browser->hists, title, sizeof(title), ev_name);
+			hists__browser_title(browser->hists, title, sizeof(title));
 			ui_browser__show_title(&browser->b, title);
 			continue;
 		}
@@ -1212,8 +1211,7 @@ static struct thread *hist_browser__selected_thread(struct hist_browser *browser
 	return browser->he_selection->thread;
 }
 
-static int hists__browser_title(struct hists *hists, char *bf, size_t size,
-				const char *ev_name)
+static int hists__browser_title(struct hists *hists, char *bf, size_t size)
 {
 	char unit;
 	int printed;
@@ -1222,6 +1220,7 @@ static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 	unsigned long nr_samples = hists->stats.nr_events[PERF_RECORD_SAMPLE];
 	u64 nr_events = hists->stats.total_period;
 	struct perf_evsel *evsel = hists_to_evsel(hists);
+	const char *ev_name = perf_evsel__name(evsel);
 	char buf[512];
 	size_t buflen = sizeof(buf);
 
@@ -1389,7 +1388,7 @@ static void hist_browser__update_nr_entries(struct hist_browser *hb)
 }
 
 static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
-				    const char *helpline, const char *ev_name,
+				    const char *helpline,
 				    bool left_exits,
 				    struct hist_browser_timer *hbt,
 				    float min_pcnt,
@@ -1464,7 +1463,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 		nr_options = 0;
 
-		key = hist_browser__run(browser, ev_name, hbt);
+		key = hist_browser__run(browser, hbt);
 
 		if (browser->he_selection != NULL) {
 			thread = hist_browser__selected_thread(browser);
@@ -1832,7 +1831,7 @@ static int perf_evsel_menu__run(struct perf_evsel_menu *menu,
 {
 	struct perf_evlist *evlist = menu->b.priv;
 	struct perf_evsel *pos;
-	const char *ev_name, *title = "Available samples";
+	const char *title = "Available samples";
 	int delay_secs = hbt ? hbt->refresh : 0;
 	int key;
 
@@ -1865,9 +1864,8 @@ browse_hists:
 			 */
 			if (hbt)
 				hbt->timer(hbt->arg);
-			ev_name = perf_evsel__name(pos);
 			key = perf_evsel__hists_browse(pos, nr_events, help,
-						       ev_name, true, hbt,
+						       true, hbt,
 						       menu->min_pcnt,
 						       menu->env);
 			ui_browser__show_title(&menu->b, title);
@@ -1971,10 +1969,9 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 single_entry:
 	if (nr_entries == 1) {
 		struct perf_evsel *first = perf_evlist__first(evlist);
-		const char *ev_name = perf_evsel__name(first);
 
 		return perf_evsel__hists_browse(first, nr_entries, help,
-						ev_name, false, hbt, min_pcnt,
+						false, hbt, min_pcnt,
 						env);
 	}
 

[tip:perf/core] perf ui browser: Fix scrollbar refresh row index

[tip-bot for Jiri Olsa]

Commit-ID:  89632972e2c56356d1e227aac151cf4e7c2f30d6
Gitweb:     http://git.kernel.org/tip/89632972e2c56356d1e227aac151cf4e7c2f30d6
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Thu, 19 Jun 2014 13:41:14 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 19 Jun 2014 16:13:14 -0300

perf ui browser: Fix scrollbar refresh row index

The ui_browser__gotorc function needs offset from 'y' member, so the row
index has to begin with 0, which happens by accident in current code,
because we display only one header line.

The bug shows when we want to display more than 1 header lines like
columns headers in following patches.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1403178076-14072-4-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index 3ccf6e1..9d2294e 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -279,7 +279,7 @@ static void ui_browser__scrollbar_set(struct ui_browser *browser)
 {
 	int height = browser->height, h = 0, pct = 0,
 	    col = browser->width,
-	    row = browser->y - 1;
+	    row = 0;
 
 	if (browser->nr_entries > 1) {
 		pct = ((browser->index * (browser->height - 1)) /