perf tools tui: Display columns headers

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

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Corey Ashford <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 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(-)

tools/perf/ui/browser.c +22 -15

index 3ccf6e1..e6f96be 100644
@@ -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;
 		}

tools/perf/ui/browser.h +17 -11

index 03d4d62..5887ca7 100644
@@ -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);

tools/perf/ui/browsers/annotate.c +11 -8

index f0697a3..bc4bcfa 100644
@@ -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,
 		},

tools/perf/ui/browsers/header.c +5 -3

index 89c16b9..69f1929 100644
@@ -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);

tools/perf/ui/browsers/hists.c +8 -6

index 04a229a..7cb6309 100644
@@ -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,
 		},

tools/perf/ui/browsers/map.c +5 -3

index b11639f..6b09cd6 100644
@@ -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,
 	};

tools/perf/ui/browsers/scripts.c +1 -1

index 402d2bd..bbbc442 100644
@@ -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,

tools/perf/ui/tui/util.c +5 -3

index bf890f7..8ac0465 100644
@@ -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);

Signature · 1 line

1.8.3.1

Em Thu, Jun 19, 2014 at 01:41:12PM +0200, Jiri Olsa escreveu:

Quoted 2 lines

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?

Quoted 370 lines

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Corey Ashford <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 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(-)

tools/perf/ui/browser.c +22 -15

index 3ccf6e1..e6f96be 100644
@@ -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;
 		}

tools/perf/ui/browser.h +17 -11

index 03d4d62..5887ca7 100644
@@ -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);

tools/perf/ui/browsers/annotate.c +11 -8

index f0697a3..bc4bcfa 100644
@@ -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,
 		},

tools/perf/ui/browsers/header.c +5 -3

index 89c16b9..69f1929 100644
@@ -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);

tools/perf/ui/browsers/hists.c +8 -6

index 04a229a..7cb6309 100644
@@ -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,
 		},

tools/perf/ui/browsers/map.c +5 -3

index b11639f..6b09cd6 100644
@@ -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,
 	};

tools/perf/ui/browsers/scripts.c +1 -1

index 402d2bd..bbbc442 100644
@@ -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,

tools/perf/ui/tui/util.c +5 -3

index bf890f7..8ac0465 100644
@@ -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);

Signature · 1 line

1.8.3.1

Quoted 7 lines · Arnaldo Carvalho de Melo

Em Thu, Jun 19, 2014 at 01:41:12PM +0200, Jiri Olsa escreveu:

Quoted 2 lines

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.

Quoted 5 lines

Quoted 5 lines

tools/perf/ui/browser.c +5 -0

+static bool ui_browser__filter(struct ui_browser *browser, void *entry)
+{
+	return browser->ops.filter ? browser->ops.filter(browser, entry) :
+				     false;
+}

Quoted 8 lines

tools/perf/ui/browser.c void ui_browser__gotorc(struct ui_browser *browser, int y, int x) · line 60

Quoted 8 lines

tools/perf/ui/browser.c +2 -1

+
 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))

Quoted 3 lines

Quoted 3 lines

tools/perf/ui/browser.c +0 -0

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

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 <[email protected]>

Cc Corey Ashford <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/ui/browsers/hists.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

tools/perf/ui/browsers/hists.c +11 -14

index 7cb6309..6aeed29 100644
@@ -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);
 	}
 

Signature · 1 line

1.8.3.1

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

tools/perf/util/symbol.c +0 -0

 };
 
 static enum dso_binary_type binary_type_symtab[] = {

tools/perf/util/symbol.h +1 -0

index 615c752..8f1854a 100644
@@ -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;

Hi Jiri,

2014-06-19 (목), 13:41 +0200, Jiri Olsa:

Quoted 3 lines

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

Quoted 5 lines

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]

tools/perf/util/symbol.c struct symbol_conf symbol_conf = { · line 38

Quoted 5 lines

tools/perf/util/symbol.c +1 -0

@@ -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)?

Quoted 8 lines

[SNIP]

tools/perf/util/symbol.c struct symbol_conf symbol_conf = { · line 38

Quoted 5 lines

tools/perf/util/symbol.c +1 -0

@@ -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

2014-06-19 (목), 15:02 +0200, Jiri Olsa:

Quoted 10 lines · Namhyung Kim

Quoted 8 lines

[SNIP]

tools/perf/util/symbol.c struct symbol_conf symbol_conf = { · line 38

Quoted 5 lines

tools/perf/util/symbol.c +1 -0

@@ -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)?

Quoted 2 lines

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?

Quoted 11 lines · Namhyung Kim

2014-06-19 (목), 15:02 +0200, Jiri Olsa:

Quoted 10 lines · Namhyung Kim

Quoted 8 lines

[SNIP]

tools/perf/util/symbol.c struct symbol_conf symbol_conf = { · line 38

Quoted 5 lines

tools/perf/util/symbol.c +1 -0

@@ -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)?

Quoted 5 lines

Quoted 2 lines

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

2014-06-19 (목), 13:41 +0200, Jiri Olsa:

Quoted 3 lines

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

Quoted 5 lines

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"?

Em Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim escreveu:

Quoted 4 lines

2014-06-19 (목), 13:41 +0200, Jiri Olsa:

Quoted 3 lines

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

Quoted 8 lines

Quoted 5 lines

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.

Quoted 17 lines · Arnaldo Carvalho de Melo

Em Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim escreveu:

Quoted 4 lines

2014-06-19 (목), 13:41 +0200, Jiri Olsa:

Quoted 3 lines

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

Quoted 8 lines

Quoted 5 lines

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.

Quoted 3 lines

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

Quoted 22 lines · Jiri Olsa

Quoted 20 lines · Arnaldo Carvalho de Melo

Em Thu, Jun 19, 2014 at 09:56:44PM +0900, Namhyung Kim escreveu:

Quoted 4 lines

2014-06-19 (목), 13:41 +0200, Jiri Olsa:

Quoted 3 lines

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

Quoted 8 lines

Quoted 5 lines

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.

Quoted 4 lines

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. :-/