All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 0/5] perf tools tui: Display columns headers
@ 2014-06-19 11:41 Jiri Olsa
  2014-06-19 11:41 ` [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct Jiri Olsa
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Jiri Olsa @ 2014-06-19 11:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arnaldo Carvalho de Melo, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra, 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(-)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct
  2014-06-19 11:41 [PATCHv2 0/5] perf tools tui: Display columns headers Jiri Olsa
@ 2014-06-19 11:41 ` Jiri Olsa
  2014-06-19 15:38   ` Arnaldo Carvalho de Melo
  2014-06-19 11:41 ` [PATCH 2/5] perf tools: Remove ev_name argument from perf_evsel__hists_browse Jiri Olsa
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 21+ messages in thread
From: Jiri Olsa @ 2014-06-19 11:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/5] perf tools: Remove ev_name argument from perf_evsel__hists_browse
  2014-06-19 11:41 [PATCHv2 0/5] perf tools tui: Display columns headers Jiri Olsa
  2014-06-19 11:41 ` [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct Jiri Olsa
@ 2014-06-19 11:41 ` Jiri Olsa
  2014-06-19 15:41   ` Arnaldo Carvalho de Melo
  2014-06-25  5:51   ` [tip:perf/core] perf hists browser: " tip-bot for Jiri Olsa
  2014-06-19 11:41 ` [PATCH 3/5] perf tools: Fix scrollbar refresh row index Jiri Olsa
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 21+ messages in thread
From: Jiri Olsa @ 2014-06-19 11:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 3/5] perf tools: Fix scrollbar refresh row index
  2014-06-19 11:41 [PATCHv2 0/5] perf tools tui: Display columns headers Jiri Olsa
  2014-06-19 11:41 ` [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct Jiri Olsa
  2014-06-19 11:41 ` [PATCH 2/5] perf tools: Remove ev_name argument from perf_evsel__hists_browse Jiri Olsa
@ 2014-06-19 11:41 ` Jiri Olsa
  2014-06-25  5:51   ` [tip:perf/core] perf ui browser: " tip-bot for Jiri Olsa
  2014-06-19 11:41 ` [PATCH 4/5] perf tools tui: Display columns header text on 'H' press Jiri Olsa
  2014-06-19 11:41 ` [PATCH 5/5] perf tools: Add report.show-headers config file option Jiri Olsa
  4 siblings, 1 reply; 21+ messages in thread
From: Jiri Olsa @ 2014-06-19 11:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 4/5] perf tools tui: Display columns header text on 'H' press
  2014-06-19 11:41 [PATCHv2 0/5] perf tools tui: Display columns headers Jiri Olsa
                   ` (2 preceding siblings ...)
  2014-06-19 11:41 ` [PATCH 3/5] perf tools: Fix scrollbar refresh row index Jiri Olsa
@ 2014-06-19 11:41 ` Jiri Olsa
  2014-06-19 11:41 ` [PATCH 5/5] perf tools: Add report.show-headers config file option Jiri Olsa
  4 siblings, 0 replies; 21+ messages in thread
From: Jiri Olsa @ 2014-06-19 11:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 5/5] perf tools: Add report.show-headers config file option
  2014-06-19 11:41 [PATCHv2 0/5] perf tools tui: Display columns headers Jiri Olsa
                   ` (3 preceding siblings ...)
  2014-06-19 11:41 ` [PATCH 4/5] perf tools tui: Display columns header text on 'H' press Jiri Olsa
@ 2014-06-19 11:41 ` Jiri Olsa
  2014-06-19 12:56   ` Namhyung Kim
  4 siblings, 1 reply; 21+ messages in thread
From: Jiri Olsa @ 2014-06-19 11:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/5] perf tools: Add report.show-headers config file option
  2014-06-19 11:41 ` [PATCH 5/5] perf tools: Add report.show-headers config file option Jiri Olsa
@ 2014-06-19 12:56   ` Namhyung Kim
  2014-06-19 13:02     ` Jiri Olsa
  2014-06-19 15:28     ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 21+ messages in thread
From: Namhyung Kim @ 2014-06-19 12:56 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Corey Ashford,
	David Ahern, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra

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;




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/5] perf tools: Add report.show-headers config file option
  2014-06-19 12:56   ` Namhyung Kim
@ 2014-06-19 13:02     ` Jiri Olsa
  2014-06-19 13:09       ` Namhyung Kim
  2014-06-19 15:28     ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 21+ messages in thread
From: Jiri Olsa @ 2014-06-19 13:02 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Jiri Olsa, linux-kernel, Arnaldo Carvalho de Melo, Corey Ashford,
	David Ahern, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/5] perf tools: Add report.show-headers config file option
  2014-06-19 13:02     ` Jiri Olsa
@ 2014-06-19 13:09       ` Namhyung Kim
  2014-06-19 16:51         ` Jiri Olsa
  0 siblings, 1 reply; 21+ messages in thread
From: Namhyung Kim @ 2014-06-19 13:09 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, linux-kernel, Arnaldo Carvalho de Melo, Corey Ashford,
	David Ahern, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra

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



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/5] perf tools: Add report.show-headers config file option
  2014-06-19 12:56   ` Namhyung Kim
  2014-06-19 13:02     ` Jiri Olsa
@ 2014-06-19 15:28     ` Arnaldo Carvalho de Melo
  2014-06-19 16:58       ` Jiri Olsa
  1 sibling, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-06-19 15:28 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Jiri Olsa, linux-kernel, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Paul Mackerras, Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct
  2014-06-19 11:41 ` [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct Jiri Olsa
@ 2014-06-19 15:38   ` Arnaldo Carvalho de Melo
  2014-06-19 16:49     ` Jiri Olsa
  0 siblings, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-06-19 15:38 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Corey Ashford, David Ahern, Frederic Weisbecker,
	Ingo Molnar, Namhyung Kim, Paul Mackerras, Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/5] perf tools: Remove ev_name argument from perf_evsel__hists_browse
  2014-06-19 11:41 ` [PATCH 2/5] perf tools: Remove ev_name argument from perf_evsel__hists_browse Jiri Olsa
@ 2014-06-19 15:41   ` Arnaldo Carvalho de Melo
  2014-06-25  5:51   ` [tip:perf/core] perf hists browser: " tip-bot for Jiri Olsa
  1 sibling, 0 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-06-19 15:41 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Corey Ashford, David Ahern, Frederic Weisbecker,
	Ingo Molnar, Namhyung Kim, Paul Mackerras, Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct
  2014-06-19 15:38   ` Arnaldo Carvalho de Melo
@ 2014-06-19 16:49     ` Jiri Olsa
  2014-06-19 23:33       ` Namhyung Kim
  0 siblings, 1 reply; 21+ messages in thread
From: Jiri Olsa @ 2014-06-19 16:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, linux-kernel, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/5] perf tools: Add report.show-headers config file option
  2014-06-19 13:09       ` Namhyung Kim
@ 2014-06-19 16:51         ` Jiri Olsa
  2014-06-19 23:37           ` Namhyung Kim
  0 siblings, 1 reply; 21+ messages in thread
From: Jiri Olsa @ 2014-06-19 16:51 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Jiri Olsa, linux-kernel, Arnaldo Carvalho de Melo, Corey Ashford,
	David Ahern, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/5] perf tools: Add report.show-headers config file option
  2014-06-19 15:28     ` Arnaldo Carvalho de Melo
@ 2014-06-19 16:58       ` Jiri Olsa
  2014-06-19 23:46         ` Namhyung Kim
  0 siblings, 1 reply; 21+ messages in thread
From: Jiri Olsa @ 2014-06-19 16:58 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Jiri Olsa, linux-kernel, Corey Ashford,
	David Ahern, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct
  2014-06-19 16:49     ` Jiri Olsa
@ 2014-06-19 23:33       ` Namhyung Kim
  0 siblings, 0 replies; 21+ messages in thread
From: Namhyung Kim @ 2014-06-19 23:33 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel, Corey Ashford,
	David Ahern, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/5] perf tools: Add report.show-headers config file option
  2014-06-19 16:51         ` Jiri Olsa
@ 2014-06-19 23:37           ` Namhyung Kim
  0 siblings, 0 replies; 21+ messages in thread
From: Namhyung Kim @ 2014-06-19 23:37 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, linux-kernel, Arnaldo Carvalho de Melo, Corey Ashford,
	David Ahern, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/5] perf tools: Add report.show-headers config file option
  2014-06-19 16:58       ` Jiri Olsa
@ 2014-06-19 23:46         ` Namhyung Kim
  2014-06-20  8:03           ` Jiri Olsa
  0 siblings, 1 reply; 21+ messages in thread
From: Namhyung Kim @ 2014-06-19 23:46 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel, Corey Ashford,
	David Ahern, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 5/5] perf tools: Add report.show-headers config file option
  2014-06-19 23:46         ` Namhyung Kim
@ 2014-06-20  8:03           ` Jiri Olsa
  0 siblings, 0 replies; 21+ messages in thread
From: Jiri Olsa @ 2014-06-20  8:03 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel, Corey Ashford,
	David Ahern, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
	Peter Zijlstra

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [tip:perf/core] perf hists browser: Remove ev_name argument from perf_evsel__hists_browse
  2014-06-19 11:41 ` [PATCH 2/5] perf tools: Remove ev_name argument from perf_evsel__hists_browse Jiri Olsa
  2014-06-19 15:41   ` Arnaldo Carvalho de Melo
@ 2014-06-25  5:51   ` tip-bot for Jiri Olsa
  1 sibling, 0 replies; 21+ messages in thread
From: tip-bot for Jiri Olsa @ 2014-06-25  5:51 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, jolsa, a.p.zijlstra,
	namhyung, fweisbec, dsahern, tglx, cjashfor

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);
 	}
 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [tip:perf/core] perf ui browser: Fix scrollbar refresh row index
  2014-06-19 11:41 ` [PATCH 3/5] perf tools: Fix scrollbar refresh row index Jiri Olsa
@ 2014-06-25  5:51   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 21+ messages in thread
From: tip-bot for Jiri Olsa @ 2014-06-25  5:51 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, hpa, mingo, jolsa, a.p.zijlstra,
	namhyung, fweisbec, dsahern, tglx, cjashfor

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2014-06-25  5:51 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-19 11:41 [PATCHv2 0/5] perf tools tui: Display columns headers Jiri Olsa
2014-06-19 11:41 ` [PATCH 1/5] perf tools: Factor ui_browser ops out of ui_browser struct Jiri Olsa
2014-06-19 15:38   ` Arnaldo Carvalho de Melo
2014-06-19 16:49     ` Jiri Olsa
2014-06-19 23:33       ` Namhyung Kim
2014-06-19 11:41 ` [PATCH 2/5] perf tools: Remove ev_name argument from perf_evsel__hists_browse Jiri Olsa
2014-06-19 15:41   ` Arnaldo Carvalho de Melo
2014-06-25  5:51   ` [tip:perf/core] perf hists browser: " tip-bot for Jiri Olsa
2014-06-19 11:41 ` [PATCH 3/5] perf tools: Fix scrollbar refresh row index Jiri Olsa
2014-06-25  5:51   ` [tip:perf/core] perf ui browser: " tip-bot for Jiri Olsa
2014-06-19 11:41 ` [PATCH 4/5] perf tools tui: Display columns header text on 'H' press Jiri Olsa
2014-06-19 11:41 ` [PATCH 5/5] perf tools: Add report.show-headers config file option Jiri Olsa
2014-06-19 12:56   ` Namhyung Kim
2014-06-19 13:02     ` Jiri Olsa
2014-06-19 13:09       ` Namhyung Kim
2014-06-19 16:51         ` Jiri Olsa
2014-06-19 23:37           ` Namhyung Kim
2014-06-19 15:28     ` Arnaldo Carvalho de Melo
2014-06-19 16:58       ` Jiri Olsa
2014-06-19 23:46         ` Namhyung Kim
2014-06-20  8:03           ` Jiri Olsa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.