linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] perf ui/browser: Add browser for perf script
@ 2012-09-07  8:42 Feng Tang
  2012-09-07  8:42 ` [PATCH v2 1/7] perf symbols: Filter samples with unresolved symbol when "--symbols" option is used Feng Tang
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Feng Tang @ 2012-09-07  8:42 UTC (permalink / raw)
  To: acme
  Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel, Feng Tang

Hi Arnaldo and all,

This is a patch set mainly to add a browser for perf script, which
will be integrated into the main hists and annotation browser.

Patch 1-4 are some preparation for adding the script
patch 5 introduce the script browser
patch 6-7 integrate the browser to hists browser and annotation browser

The patches are on top of current perf/core branch of your git tree. 
Please help to review. 

Namhyung,

Many thanks for providing many good comments for the v1 patch sets. In
this v2, I addressed most of those comments, for the suggestion to use
browser for "perf script -l", I'll handle it later, and would post the
v2 first for review.

Changelog:
	Since v1:
	* Add filter for scripts can't be run in script browser
	* Fix some bugs about buffer handling and error check 

Feng
-----------------------
Feng Tang (7):
  perf symbols: Filter samples with unresolved symbol when "--symbols"
    option is used
  perf scripts: Add --symbols option to handle specific symbols
  perf scripts: Add event_analyzing_sample-record/report
  perf scripts: Export a find_scripts() function
  perf ui/browser: Add a browser for perf script
  perf ui/browser: Integrate script browser into annotation browser
  perf ui/browser: Integrate script browser into main hists browser

 tools/perf/Makefile                                |    4 +
 tools/perf/builtin-script.c                        |   58 ++++++
 tools/perf/builtin.h                               |    1 +
 .../python/bin/event_analyzing_sample-record       |    8 +
 .../python/bin/event_analyzing_sample-report       |    3 +
 tools/perf/ui/browsers/annotate.c                  |    6 +
 tools/perf/ui/browsers/hists.c                     |   37 ++++
 tools/perf/ui/browsers/scripts.c                   |  182 ++++++++++++++++++++
 tools/perf/util/event.c                            |    5 +-
 tools/perf/util/hist.h                             |    7 +
 10 files changed, 309 insertions(+), 2 deletions(-)
 create mode 100644 tools/perf/scripts/python/bin/event_analyzing_sample-record
 create mode 100644 tools/perf/scripts/python/bin/event_analyzing_sample-report
 create mode 100644 tools/perf/ui/browsers/scripts.c


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

* [PATCH v2 1/7] perf symbols: Filter samples with unresolved symbol when "--symbols" option is used
  2012-09-07  8:42 [PATCH v2 0/7] perf ui/browser: Add browser for perf script Feng Tang
@ 2012-09-07  8:42 ` Feng Tang
  2012-09-19 15:21   ` [tip:perf/core] " tip-bot for Feng Tang
  2012-09-07  8:42 ` [PATCH v2 2/7] perf scripts: Add --symbols option to handle specific symbols Feng Tang
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Feng Tang @ 2012-09-07  8:42 UTC (permalink / raw)
  To: acme
  Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel, Feng Tang

Report/top commands support to only handle specific symbols with
"--symbols" option, but current code will keep those samples whose
symbol can't be resolved, which should actually be filtered.

If we run following commands:
$perf record -a tree
$perf report --symbols intel_idle -n
the output will be:

Without the patch:
==================
    46.27%        156     sshd  [unknown]
    26.05%         48  swapper  [kernel.kallsyms]
    17.26%         38     tree  libc-2.12.1.so
     7.69%         17     tree  tree
     2.73%          6     tree  ld-2.12.1.so

With the patch:
===============
   100.00%         48  swapper  [kernel.kallsyms]

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 tools/perf/util/event.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 3a0f1a5..08910f0 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -886,8 +886,9 @@ int perf_event__preprocess_sample(const union perf_event *event,
 		al->sym = map__find_symbol(al->map, al->addr, filter);
 	}
 
-	if (symbol_conf.sym_list && al->sym &&
-	    !strlist__has_entry(symbol_conf.sym_list, al->sym->name))
+	if (symbol_conf.sym_list &&
+		(!al->sym || !strlist__has_entry(symbol_conf.sym_list,
+						al->sym->name)))
 		goto out_filtered;
 
 	return 0;
-- 
1.7.1


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

* [PATCH v2 2/7] perf scripts: Add --symbols option to handle specific symbols
  2012-09-07  8:42 [PATCH v2 0/7] perf ui/browser: Add browser for perf script Feng Tang
  2012-09-07  8:42 ` [PATCH v2 1/7] perf symbols: Filter samples with unresolved symbol when "--symbols" option is used Feng Tang
@ 2012-09-07  8:42 ` Feng Tang
  2012-09-19 15:22   ` [tip:perf/core] " tip-bot for Feng Tang
  2012-09-07  8:42 ` [PATCH v2 3/7] perf scripts: Add event_analyzing_sample-record/report Feng Tang
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Feng Tang @ 2012-09-07  8:42 UTC (permalink / raw)
  To: acme
  Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel, Feng Tang

Since perf script no longer only handle the trace points, we can
add the symbol filter option so that scripts can handle specified
samples.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 tools/perf/builtin-script.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 2d6e3b2..7ab86bb 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -14,6 +14,7 @@
 #include "util/util.h"
 #include "util/evlist.h"
 #include "util/evsel.h"
+#include "util/sort.h"
 #include <linux/bitmap.h>
 
 static char const		*script_name;
@@ -1142,6 +1143,8 @@ static const struct option options[] = {
 		     parse_output_fields),
 	OPT_BOOLEAN('a', "all-cpus", &system_wide,
 		     "system-wide collection from all CPUs"),
+	OPT_STRING('S', "symbols", &symbol_conf.sym_list_str, "symbol[,symbol...]",
+		   "only consider these symbols"),
 	OPT_STRING('C', "cpu", &cpu_list, "cpu", "list of cpus to profile"),
 	OPT_STRING('c', "comms", &symbol_conf.comm_list_str, "comm[,comm...]",
 		   "only display events for these comms"),
-- 
1.7.1


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

* [PATCH v2 3/7] perf scripts: Add event_analyzing_sample-record/report
  2012-09-07  8:42 [PATCH v2 0/7] perf ui/browser: Add browser for perf script Feng Tang
  2012-09-07  8:42 ` [PATCH v2 1/7] perf symbols: Filter samples with unresolved symbol when "--symbols" option is used Feng Tang
  2012-09-07  8:42 ` [PATCH v2 2/7] perf scripts: Add --symbols option to handle specific symbols Feng Tang
@ 2012-09-07  8:42 ` Feng Tang
  2012-09-19 15:23   ` [tip:perf/core] perf scripts: Add event_analyzing_sample-record/ report tip-bot for Feng Tang
  2012-09-07  8:42 ` [PATCH v2 4/7] perf scripts: Export a find_scripts() function Feng Tang
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Feng Tang @ 2012-09-07  8:42 UTC (permalink / raw)
  To: acme
  Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel, Feng Tang

So that event_analyzing_sample.py can be shown by "perf script -l"

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 .../python/bin/event_analyzing_sample-record       |    8 ++++++++
 .../python/bin/event_analyzing_sample-report       |    3 +++
 2 files changed, 11 insertions(+), 0 deletions(-)
 create mode 100644 tools/perf/scripts/python/bin/event_analyzing_sample-record
 create mode 100644 tools/perf/scripts/python/bin/event_analyzing_sample-report

diff --git a/tools/perf/scripts/python/bin/event_analyzing_sample-record b/tools/perf/scripts/python/bin/event_analyzing_sample-record
new file mode 100644
index 0000000..5ce652d
--- /dev/null
+++ b/tools/perf/scripts/python/bin/event_analyzing_sample-record
@@ -0,0 +1,8 @@
+#!/bin/bash
+
+#
+# event_analyzing_sample.py can cover all type of perf samples including
+# the tracepoints, so no special record requirements, just record what
+# you want to analyze.
+#
+perf record $@
diff --git a/tools/perf/scripts/python/bin/event_analyzing_sample-report b/tools/perf/scripts/python/bin/event_analyzing_sample-report
new file mode 100644
index 0000000..0941fc9
--- /dev/null
+++ b/tools/perf/scripts/python/bin/event_analyzing_sample-report
@@ -0,0 +1,3 @@
+#!/bin/bash
+# description: analyze all perf samples
+perf script $@ -s "$PERF_EXEC_PATH"/scripts/python/event_analyzing_sample.py
-- 
1.7.1


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

* [PATCH v2 4/7] perf scripts: Export a find_scripts() function
  2012-09-07  8:42 [PATCH v2 0/7] perf ui/browser: Add browser for perf script Feng Tang
                   ` (2 preceding siblings ...)
  2012-09-07  8:42 ` [PATCH v2 3/7] perf scripts: Add event_analyzing_sample-record/report Feng Tang
@ 2012-09-07  8:42 ` Feng Tang
  2012-09-19 15:24   ` [tip:perf/core] " tip-bot for Feng Tang
  2012-09-07  8:42 ` [PATCH v2 5/7] perf ui/browser: Add a browser for perf script Feng Tang
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Feng Tang @ 2012-09-07  8:42 UTC (permalink / raw)
  To: acme
  Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel, Feng Tang

So that other perf commands/browser has a way to dig out the available
scripts info in system, this is a preparation for the script browser.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 tools/perf/builtin-script.c |   55 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/builtin.h        |    1 +
 2 files changed, 56 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 7ab86bb..79f21d5 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1031,6 +1031,61 @@ static int list_available_scripts(const struct option *opt __used,
 	exit(0);
 }
 
+/*
+ * Return -1 if none is found, otherwise the actual scripts number.
+ *
+ * Currently the only user of this function is the script browser, which
+ * will list all statically runnable scripts, select one, execute it and
+ * show the output in a perf browser.
+ */
+int find_scripts(char **scripts_array, char **scripts_path_array)
+{
+	struct dirent *script_next, *lang_next, script_dirent, lang_dirent;
+	char scripts_path[MAXPATHLEN];
+	DIR *scripts_dir, *lang_dir;
+	char lang_path[MAXPATHLEN];
+	char *temp;
+	int i = 0;
+
+	snprintf(scripts_path, MAXPATHLEN, "%s/scripts", perf_exec_path());
+
+	scripts_dir = opendir(scripts_path);
+	if (!scripts_dir)
+		return -1;
+
+	for_each_lang(scripts_path, scripts_dir, lang_dirent, lang_next) {
+		snprintf(lang_path, MAXPATHLEN, "%s/%s", scripts_path,
+			 lang_dirent.d_name);
+#ifdef NO_LIBPERL
+		if (strstr(lang_path, "perl"))
+			continue;
+#endif
+#ifdef NO_LIBPYTHON
+		if (strstr(lang_path, "python"))
+			continue;
+#endif
+
+		lang_dir = opendir(lang_path);
+		if (!lang_dir)
+			continue;
+
+		for_each_script(lang_path, lang_dir, script_dirent, script_next) {
+			/* Skip those real time scripts: xxxtop.p[yl] */
+			if (strstr(script_dirent.d_name, "top."))
+				continue;
+			sprintf(scripts_path_array[i], "%s/%s", lang_path,
+				script_dirent.d_name);
+			temp = strchr(script_dirent.d_name, '.');
+			snprintf(scripts_array[i],
+				(temp - script_dirent.d_name) + 1,
+				"%s", script_dirent.d_name);
+			i++;
+		}
+	}
+
+	return i;
+}
+
 static char *get_script_path(const char *script_root, const char *suffix)
 {
 	struct dirent *script_next, *lang_next, script_dirent, lang_dirent;
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index b382bd5..3ea74ed 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -36,4 +36,5 @@ extern int cmd_kvm(int argc, const char **argv, const char *prefix);
 extern int cmd_test(int argc, const char **argv, const char *prefix);
 extern int cmd_inject(int argc, const char **argv, const char *prefix);
 
+extern int find_scripts(char **scripts_array, char **scripts_path_array);
 #endif
-- 
1.7.1


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

* [PATCH v2 5/7] perf ui/browser: Add a browser for perf script
  2012-09-07  8:42 [PATCH v2 0/7] perf ui/browser: Add browser for perf script Feng Tang
                   ` (3 preceding siblings ...)
  2012-09-07  8:42 ` [PATCH v2 4/7] perf scripts: Export a find_scripts() function Feng Tang
@ 2012-09-07  8:42 ` Feng Tang
  2012-09-07  8:42 ` [PATCH v2 6/7] perf ui/browser: Integrate script browser into annotation browser Feng Tang
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Feng Tang @ 2012-09-07  8:42 UTC (permalink / raw)
  To: acme
  Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel, Feng Tang

Create a script browser, so that user can check all the available
scripts and run them inside the main perf report or annotation
browsers, for all the perf samples or samples belong to one
thread/symbol.

The work flow is, users can use function key to list all the available
scripts in system and chose one, which will be executed with
popen("perf script -s xxx.xx",) and all the output lines are put into
one ui browser, pressing 'q' or left arrow key will make it return
to previous browser.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 tools/perf/Makefile              |    4 +
 tools/perf/ui/browsers/scripts.c |  182 ++++++++++++++++++++++++++++++++++++++
 tools/perf/util/hist.h           |    7 ++
 3 files changed, 193 insertions(+), 0 deletions(-)
 create mode 100644 tools/perf/ui/browsers/scripts.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index cd196bc..941e700 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -579,6 +579,7 @@ else
 		LIB_OBJS += $(OUTPUT)ui/browsers/annotate.o
 		LIB_OBJS += $(OUTPUT)ui/browsers/hists.o
 		LIB_OBJS += $(OUTPUT)ui/browsers/map.o
+		LIB_OBJS += $(OUTPUT)ui/browsers/scripts.o
 		LIB_OBJS += $(OUTPUT)ui/progress.o
 		LIB_OBJS += $(OUTPUT)ui/util.o
 		LIB_OBJS += $(OUTPUT)ui/tui/setup.o
@@ -890,6 +891,9 @@ $(OUTPUT)ui/browsers/hists.o: ui/browsers/hists.c $(OUTPUT)PERF-CFLAGS
 $(OUTPUT)ui/browsers/map.o: ui/browsers/map.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<
 
+$(OUTPUT)ui/browsers/scripts.o: ui/browsers/scripts.c $(OUTPUT)PERF-CFLAGS
+	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DENABLE_SLFUTURE_CONST $<
+
 $(OUTPUT)util/rbtree.o: ../../lib/rbtree.c $(OUTPUT)PERF-CFLAGS
 	$(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $<
 
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
new file mode 100644
index 0000000..28a58fc
--- /dev/null
+++ b/tools/perf/ui/browsers/scripts.c
@@ -0,0 +1,182 @@
+#include <elf.h>
+#include <newt.h>
+#include <inttypes.h>
+#include <sys/ttydefaults.h>
+#include <string.h>
+#include "../../util/sort.h"
+#include "../../util/util.h"
+#include "../../util/hist.h"
+#include "../../util/debug.h"
+#include "../../util/symbol.h"
+#include "../browser.h"
+#include "../helpline.h"
+#include "../libslang.h"
+
+/* 2048 lines should be enough for a script output */
+#define MAX_LINES		2048
+
+/* 160 bytes for one output line */
+#define AVERAGE_LINE_LEN	160
+
+struct script_line {
+	struct list_head node;
+	char line[AVERAGE_LINE_LEN];
+};
+
+struct perf_script_browser {
+	struct ui_browser b;
+	struct list_head entries;
+	const char *script_name;
+	int nr_lines;
+};
+
+#define SCRIPT_NAMELEN	128
+#define SCRIPT_MAX_NO	64
+/*
+ * Usually the full path for a script is:
+ *	/home/username/libexec/perf-core/scripts/python/xxx.py
+ *	/home/username/libexec/perf-core/scripts/perl/xxx.pl
+ * So 256 should be long enough to contain the full path.
+ */
+#define SCRIPT_FULLPATH_LEN	256
+
+/*
+ * When success, will copy the full path of the selected script
+ * into  the buffer pointed by script_name, and return 0.
+ * Return -1 on failure.
+ */
+static int list_scripts(char *script_name)
+{
+	char *buf, *names[SCRIPT_MAX_NO], *paths[SCRIPT_MAX_NO];
+	int i, num, choice, ret = -1;
+
+	/* Preset the script name to SCRIPT_NAMELEN */
+	buf = malloc(SCRIPT_MAX_NO * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN));
+	if (!buf)
+		return ret;
+
+	for (i = 0; i < SCRIPT_MAX_NO; i++) {
+		names[i] = buf + i * (SCRIPT_NAMELEN + SCRIPT_FULLPATH_LEN);
+		paths[i] = names[i] + SCRIPT_NAMELEN;
+	}
+
+	num = find_scripts(names, paths);
+	if (num > 0) {
+		choice = ui__popup_menu(num, names);
+		if (choice < num && choice >= 0) {
+			strcpy(script_name, paths[choice]);
+			ret = 0;
+		}
+	}
+
+	free(buf);
+	return ret;
+}
+
+static void script_browser__write(struct ui_browser *browser,
+				   void *entry, int row)
+{
+	struct script_line *sline = list_entry(entry, struct script_line, node);
+	bool current_entry = ui_browser__is_current_entry(browser, row);
+
+	ui_browser__set_color(browser, current_entry ? HE_COLORSET_SELECTED :
+						       HE_COLORSET_NORMAL);
+
+	slsmg_write_nstring(sline->line, browser->width);
+}
+
+static int script_browser__run(struct perf_script_browser *self)
+{
+	int key;
+
+	if (ui_browser__show(&self->b, self->script_name,
+			     "Press <- or ESC to exit") < 0)
+		return -1;
+
+	while (1) {
+		key = ui_browser__run(&self->b, 0);
+
+		/* We can add some special key handling here if needed */
+		break;
+	}
+
+	ui_browser__hide(&self->b);
+	return key;
+}
+
+
+int script_browse(const char *script_opt)
+{
+	char cmd[SCRIPT_FULLPATH_LEN*2], script_name[SCRIPT_FULLPATH_LEN];
+	char *line = NULL;
+	size_t len = 0;
+	ssize_t retlen;
+	int ret = -1, nr_entries = 0;
+	FILE *fp;
+	void *buf;
+	struct script_line *sline;
+
+	struct perf_script_browser script = {
+		.b = {
+			.refresh    = ui_browser__list_head_refresh,
+			.seek	    = ui_browser__list_head_seek,
+			.write	    = script_browser__write,
+		},
+		.script_name = script_name,
+	};
+
+	INIT_LIST_HEAD(&script.entries);
+
+	/* Save each line of the output in one struct script_line object. */
+	buf = zalloc((sizeof(*sline)) * MAX_LINES);
+	if (!buf)
+		return -1;
+	sline = buf;
+
+	memset(script_name, 0, SCRIPT_FULLPATH_LEN);
+	if (list_scripts(script_name))
+		goto exit;
+
+	if (script_opt)
+		sprintf(cmd, "perf script %s -s %s 2>&1", script_opt, script_name);
+	else
+		sprintf(cmd, "perf script -s %s 2>&1", script_name);
+
+	fp = popen(cmd, "r");
+	if (!fp)
+		goto exit;
+
+	while ((retlen = getline(&line, &len, fp)) != -1) {
+		strncpy(sline->line, line, AVERAGE_LINE_LEN);
+
+		/* If one output line is very large, just cut it short */
+		if (retlen >= AVERAGE_LINE_LEN) {
+			sline->line[AVERAGE_LINE_LEN - 1] = '\0';
+			sline->line[AVERAGE_LINE_LEN - 2] = '\n';
+		}
+		list_add_tail(&sline->node, &script.entries);
+
+		if (script.b.width < retlen)
+			script.b.width = retlen;
+
+		if (nr_entries++ >= MAX_LINES)
+			break;
+		sline++;
+	}
+
+	if (script.b.width > AVERAGE_LINE_LEN)
+		script.b.width = AVERAGE_LINE_LEN;
+
+	if (line)
+		free(line);
+	pclose(fp);
+
+	script.nr_lines = nr_entries;
+	script.b.nr_entries = nr_entries;
+	script.b.entries = &script.entries;
+
+	ret = script_browser__run(&script);
+exit:
+	free(buf);
+	return ret;
+}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2e650ff..418a040 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -136,6 +136,12 @@ static inline int hist_entry__tui_annotate(struct hist_entry *self __used,
 {
 	return 0;
 }
+
+static inline int script_browse(const char *script_opt)
+{
+	return 0;
+}
+
 #define K_LEFT -1
 #define K_RIGHT -2
 #else
@@ -146,6 +152,7 @@ int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 				  void(*timer)(void *arg), void *arg,
 				  int refresh);
+int script_browse(const char *script_opt);
 #endif
 
 #ifdef NO_GTK2_SUPPORT
-- 
1.7.1


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

* [PATCH v2 6/7] perf ui/browser: Integrate script browser into annotation browser
  2012-09-07  8:42 [PATCH v2 0/7] perf ui/browser: Add browser for perf script Feng Tang
                   ` (4 preceding siblings ...)
  2012-09-07  8:42 ` [PATCH v2 5/7] perf ui/browser: Add a browser for perf script Feng Tang
@ 2012-09-07  8:42 ` Feng Tang
  2012-09-17 15:23   ` Arnaldo Carvalho de Melo
  2012-09-07  8:42 ` [PATCH v2 7/7] perf ui/browser: Integrate script browser into main hists browser Feng Tang
  2012-09-10  1:35 ` [PATCH v2 0/7] perf ui/browser: Add browser for perf script Namhyung Kim
  7 siblings, 1 reply; 17+ messages in thread
From: Feng Tang @ 2012-09-07  8:42 UTC (permalink / raw)
  To: acme
  Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel, Feng Tang

Integrate the script browser into annotation, users can press function
key 'r' to list all perf scripts and select one of them to run that
script, the output will be shown in a separate browser.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 tools/perf/ui/browsers/annotate.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 67a2703..13ac54c 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -675,8 +675,14 @@ static int annotate_browser__run(struct annotate_browser *browser, int evidx,
 		"o             Toggle disassembler output/simplified view\n"
 		"s             Toggle source code view\n"
 		"/             Search string\n"
+		"r             Run available scripts\n"
 		"?             Search previous string\n");
 			continue;
+		case 'r':
+			{
+				script_browse(NULL);
+				continue;
+			}
 		case 'H':
 			nd = browser->curr_hot;
 			break;
-- 
1.7.1


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

* [PATCH v2 7/7] perf ui/browser: Integrate script browser into main hists browser
  2012-09-07  8:42 [PATCH v2 0/7] perf ui/browser: Add browser for perf script Feng Tang
                   ` (5 preceding siblings ...)
  2012-09-07  8:42 ` [PATCH v2 6/7] perf ui/browser: Integrate script browser into annotation browser Feng Tang
@ 2012-09-07  8:42 ` Feng Tang
  2012-09-10  1:35 ` [PATCH v2 0/7] perf ui/browser: Add browser for perf script Namhyung Kim
  7 siblings, 0 replies; 17+ messages in thread
From: Feng Tang @ 2012-09-07  8:42 UTC (permalink / raw)
  To: acme
  Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel, Feng Tang

Integrate the script browser into "perf report" framework, users can
use function key 'r' or the drop down menu to list all perf scripts
and select one of them, just like they did for the annotation.

Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 tools/perf/ui/browsers/hists.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 81bd8c2..73cff34 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1089,6 +1089,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	int nr_options = 0;
 	int key = -1;
 	char buf[64];
+	char script_opt[64];
 
 	if (browser == NULL)
 		return -1;
@@ -1099,6 +1100,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 	ui_helpline__push(helpline);
 
+	memset(script_opt, 0, 64);
 	memset(options, 0, sizeof(options));
 
 	while (1) {
@@ -1107,6 +1109,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		int choice = 0,
 		    annotate = -2, zoom_dso = -2, zoom_thread = -2,
 		    annotate_f = -2, annotate_t = -2, browse_map = -2;
+		int scripts_comm = -2, scripts_symbol = -2, scripts_all = -2;
 
 		nr_options = 0;
 
@@ -1159,6 +1162,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 				hist_browser__reset(browser);
 			}
 			continue;
+		case 'r':
+			goto do_scripts;
 		case K_F1:
 		case 'h':
 		case '?':
@@ -1177,6 +1182,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 					"E             Expand all callchains\n"
 					"d             Zoom into current DSO\n"
 					"t             Zoom into current Thread\n"
+					"r             Run available scripts\n"
 					"P             Print histograms to perf.hist.N\n"
 					"V             Verbose (DSO names in callchains, etc)\n"
 					"/             Filter symbol by name");
@@ -1265,6 +1271,25 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		    browser->selection->map != NULL &&
 		    asprintf(&options[nr_options], "Browse map details") > 0)
 			browse_map = nr_options++;
+
+		/* perf script support */
+		if (browser->he_selection) {
+			struct symbol *sym;
+
+			if (asprintf(&options[nr_options], "Run scripts for samples of thread [%s]",
+				browser->he_selection->thread->comm) > 0)
+				scripts_comm = nr_options++;
+
+			sym = browser->he_selection->ms.sym;
+			if (sym && sym->namelen &&
+				asprintf(&options[nr_options], "Run scripts for samples of symbol [%s]",
+						sym->name) > 0)
+				scripts_symbol = nr_options++;
+		}
+
+		if (asprintf(&options[nr_options], "Run scripts for all samples") > 0)
+			scripts_all = nr_options++;
+
 add_exit_option:
 		options[nr_options++] = (char *)"Exit";
 retry_popup_menu:
@@ -1359,6 +1384,18 @@ zoom_out_thread:
 			hists__filter_by_thread(hists);
 			hist_browser__reset(browser);
 		}
+		/* perf scripts support */
+		else if (choice == scripts_all || choice == scripts_comm ||
+				choice == scripts_symbol) {
+do_scripts:
+			if (choice == scripts_comm)
+				sprintf(script_opt, " -c %s ", browser->he_selection->thread->comm);
+
+			if (choice == scripts_symbol)
+				sprintf(script_opt, " -S %s ", browser->he_selection->ms.sym->name);
+
+			script_browse(script_opt);
+		}
 	}
 out_free_stack:
 	pstack__delete(fstack);
-- 
1.7.1


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

* Re: [PATCH v2 0/7] perf ui/browser: Add browser for perf script
  2012-09-07  8:42 [PATCH v2 0/7] perf ui/browser: Add browser for perf script Feng Tang
                   ` (6 preceding siblings ...)
  2012-09-07  8:42 ` [PATCH v2 7/7] perf ui/browser: Integrate script browser into main hists browser Feng Tang
@ 2012-09-10  1:35 ` Namhyung Kim
  7 siblings, 0 replies; 17+ messages in thread
From: Namhyung Kim @ 2012-09-10  1:35 UTC (permalink / raw)
  To: Feng Tang; +Cc: acme, mingo, a.p.zijlstra, andi, dsahern, linux-kernel

Hi Feng,

On Fri,  7 Sep 2012 16:42:22 +0800, Feng Tang wrote:
> Hi Arnaldo and all,
>
> This is a patch set mainly to add a browser for perf script, which
> will be integrated into the main hists and annotation browser.
>
> Patch 1-4 are some preparation for adding the script
> patch 5 introduce the script browser
> patch 6-7 integrate the browser to hists browser and annotation browser
>
> The patches are on top of current perf/core branch of your git tree. 
> Please help to review. 
>
> Namhyung,
>
> Many thanks for providing many good comments for the v1 patch sets. In
> this v2, I addressed most of those comments, for the suggestion to use
> browser for "perf script -l", I'll handle it later, and would post the
> v2 first for review.
>
> Changelog:
> 	Since v1:
> 	* Add filter for scripts can't be run in script browser
> 	* Fix some bugs about buffer handling and error check 

For whole series,

Reviewed-by: Namhyung Kim <namhyung@kernel.org>

Thanks for your work!
Namhyung

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

* Re: [PATCH v2 6/7] perf ui/browser: Integrate script browser into annotation browser
  2012-09-07  8:42 ` [PATCH v2 6/7] perf ui/browser: Integrate script browser into annotation browser Feng Tang
@ 2012-09-17 15:23   ` Arnaldo Carvalho de Melo
  2012-09-18  7:40     ` Feng Tang
  0 siblings, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-17 15:23 UTC (permalink / raw)
  To: Feng Tang; +Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel

Em Fri, Sep 07, 2012 at 04:42:28PM +0800, Feng Tang escreveu:
> Integrate the script browser into annotation, users can press function
> key 'r' to list all perf scripts and select one of them to run that
> script, the output will be shown in a separate browser.

I think this needs a bit more work... i.e. I tried it with your script,
event_analyzing_sample and I kinda get meaningful output when pressing
'r' + that script from the top and hists browser.

But with other scripts, and you filtered the -top suffixed ones, that
require special handling, I get things like "Press Control+C to get a
summary", and when I press that, the browser exits, going back to
annotate or report/top browser.

I.e. this only works if we are processing a perf.data file made
specially for that specific script, right? I.e. the record phase is not
integrated at all, just running specific scripts in specific perf.data
files.

How to allow the user to chose appropriate scripts to run each perf.data
file is an aspect of usability that is missing...

Can you ellaborate on the assumptions you made while working on this, do
they match what I described above as how to use this scripts browser?

So I merged part of this patchset, but will wait till more discussion
happens on the browsing part,

Thanks,

- Arnaldo
 
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> ---
>  tools/perf/ui/browsers/annotate.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 67a2703..13ac54c 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -675,8 +675,14 @@ static int annotate_browser__run(struct annotate_browser *browser, int evidx,
>  		"o             Toggle disassembler output/simplified view\n"
>  		"s             Toggle source code view\n"
>  		"/             Search string\n"
> +		"r             Run available scripts\n"
>  		"?             Search previous string\n");
>  			continue;
> +		case 'r':
> +			{
> +				script_browse(NULL);
> +				continue;
> +			}
>  		case 'H':
>  			nd = browser->curr_hot;
>  			break;
> -- 
> 1.7.1

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

* Re: [PATCH v2 6/7] perf ui/browser: Integrate script browser into annotation browser
  2012-09-17 15:23   ` Arnaldo Carvalho de Melo
@ 2012-09-18  7:40     ` Feng Tang
  2012-09-18 11:30       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 17+ messages in thread
From: Feng Tang @ 2012-09-18  7:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel

Hi Arnaldo,

Thanks for the reviews!

On Mon, 17 Sep 2012 12:23:01 -0300
Arnaldo Carvalho de Melo <acme@redhat.com> wrote:

> Em Fri, Sep 07, 2012 at 04:42:28PM +0800, Feng Tang escreveu:
> > Integrate the script browser into annotation, users can press function
> > key 'r' to list all perf scripts and select one of them to run that
> > script, the output will be shown in a separate browser.
> 
> I think this needs a bit more work... i.e. I tried it with your script,
> event_analyzing_sample and I kinda get meaningful output when pressing
> 'r' + that script from the top and hists browser.

I would answer you question about "elllaborate on the assumptions" first
here, that the main purpose of this script browser is not to replace the
normal "perf script" command, but to:
1. Provide a convenient way for user to directly run scripts while they
are browsing through the hists or annotation browser, and provide some
option for run script with samples from specific thread or symbol
2. Prepare for some advance feature, like inside the annotation browser,
analyze some specific hot line of code with samples containing precise
information, like the register values, store/load latency stuff. But I
have no idea how to implement it now, I guess after Stefan work out the
general perf latency tool, we may be able to revisit it.   

And when I started to code, I assumed user will mainly use this feature
for running scripts for general samples other than the existing scripts
for tracepoints. And yes, some user cases you mentioned are not covered.

> 
> But with other scripts, and you filtered the -top suffixed ones, that
> require special handling, I get things like "Press Control+C to get a
> summary", and when I press that, the browser exits, going back to
> annotate or report/top browser.

That's right, 4 current perf python scripts will print that line in its
"trace_begin" function, as some of the scripts may run for quiet some
time if the perf.data is huge. And what you see in the script browser 
is just some static text of the script output. 

> 
> I.e. this only works if we are processing a perf.data file made
> specially for that specific script, right? I.e. the record phase is not
> integrated at all, just running specific scripts in specific perf.data
> files.

No, the record phase is not added, it just handle the recorded data. And
there is something missed in current implementation, that the script in
browser mode is only run for the "perf.data" even if the perf report/annotate
is run for data with another name.

> 
> How to allow the user to chose appropriate scripts to run each perf.data
> file is an aspect of usability that is missing...

Do we need to run script for another perf.data when we run report/annotation
for one perf.data? or should we add a option for script browser to make it
work in browser mode, this is something Numhyung has mentioned in his review

Thanks,
Feng

> 
> Can you ellaborate on the assumptions you made while working on this, do
> they match what I described above as how to use this scripts browser?
> 
> So I merged part of this patchset, but will wait till more discussion
> happens on the browsing part,
> 
> Thanks,
> 
> - Arnaldo

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

* Re: [PATCH v2 6/7] perf ui/browser: Integrate script browser into annotation browser
  2012-09-18  7:40     ` Feng Tang
@ 2012-09-18 11:30       ` Arnaldo Carvalho de Melo
  2012-09-18 15:56         ` Feng Tang
  0 siblings, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-18 11:30 UTC (permalink / raw)
  To: Feng Tang; +Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel

Em Tue, Sep 18, 2012 at 03:40:10PM +0800, Feng Tang escreveu:
> Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
> > Em Fri, Sep 07, 2012 at 04:42:28PM +0800, Feng Tang escreveu:
> > > Integrate the script browser into annotation, users can press function
> > > key 'r' to list all perf scripts and select one of them to run that
> > > script, the output will be shown in a separate browser.
> > 
> > I think this needs a bit more work... i.e. I tried it with your script,
> > event_analyzing_sample and I kinda get meaningful output when pressing
> > 'r' + that script from the top and hists browser.
> 
> I would answer you question about "elllaborate on the assumptions" first
> here, that the main purpose of this script browser is not to replace the
> normal "perf script" command, but to:
> 1. Provide a convenient way for user to directly run scripts while they

I think this is what confused me, you say "directly run scripts", but
that only means "directly run the report phase of scripts, not the
record phase". Ok, that is clear now.

> are browsing through the hists or annotation browser, and provide some
> option for run script with samples from specific thread or symbol

> 2. Prepare for some advance feature, like inside the annotation browser,
> analyze some specific hot line of code with samples containing precise
> information, like the register values, store/load latency stuff. But I
> have no idea how to implement it now, I guess after Stefan work out the
> general perf latency tool, we may be able to revisit it.   

I'm ok with adding infrastructure for later improvements, but I think we
should try to do it in a way that doesn't confuses users too much,
otherwise they may get annoyed and not try the feature when it becomes
useful, worse, tell others that it is cumbersome, don't bother trying
it.

So we should try to do it in a way that is useful for the constraints
you explained here, more on that later in this message.
 
> And when I started to code, I assumed user will mainly use this feature
> for running scripts for general samples other than the existing scripts
> for tracepoints. And yes, some user cases you mentioned are not covered.

Ok, so we need to filter out some more, like you did for the top script.
 
> > But with other scripts, and you filtered the -top suffixed ones, that
> > require special handling, I get things like "Press Control+C to get a
> > summary", and when I press that, the browser exits, going back to
> > annotate or report/top browser.
> 
> That's right, 4 current perf python scripts will print that line in its
> "trace_begin" function, as some of the scripts may run for quiet some
> time if the perf.data is huge. And what you see in the script browser 
> is just some static text of the script output. 

Ok, so one thing I think we can do is to look at the record script:

[acme@sandy linux]$ cat
tools/perf/scripts/python/bin/syscall-counts-by-pid-record 
#!/bin/bash
perf record -e raw_syscalls:sys_enter $@

and also at the perf.data file:

[root@sandy linux]# perf evlist
cycles

See? we can't offer the syscall-counts-by-pid script to analyse this
perf.data file, one is made for the raw_syscalls:sys_enter tracepoint
while the other has only cpu cycles.

While the script you provided:

[acme@sandy linux]$ cat
tools/perf/scripts/python/bin/event_analyzing_sample-record 
#!/bin/bash

#
# event_analyzing_sample.py can cover all type of perf samples including
# the tracepoints, so no special record requirements, just record what
# you want to analyze.
#
perf record $@
[acme@sandy linux]$ 

-----------------------

You made it general purpose, so yeah, we can offer that script to the
user analysing that perf.data file.

So this is one possible way of matching scripts to perf.data files,
another would be for us to somehow annotate the scripts, in the first
few lines, with markers that would help do this matching, but I'd start
with what we have: parse the line that has the 'perf record' command in
the tools/perf/scripts/python/bin/*-record file, look at the event it
handles and match to what 'perf evlist' reports.

Not necessarily running 'perf evlist', just doing what it does to figure
out the needed information. 

> > I.e. this only works if we are processing a perf.data file made
> > specially for that specific script, right? I.e. the record phase is not
> > integrated at all, just running specific scripts in specific perf.data
> > files.
> 
> No, the record phase is not added, it just handle the recorded data. And
> there is something missed in current implementation, that the script in
> browser mode is only run for the "perf.data" even if the perf report/annotate
> is run for data with another name.

Ok, so that needs to be handled, using the filtering I suggested above.

> > How to allow the user to chose appropriate scripts to run each perf.data
> > file is an aspect of usability that is missing...
> 
> Do we need to run script for another perf.data when we run report/annotation
> for one perf.data? or should we add a option for script browser to make it
> work in browser mode, this is something Numhyung has mentioned in his review

Well, it would be great to be able to press some key in the main hists
browser and then switch to a different perf.data file, the tool could
just look at the current directory, looking at the magic number in the
first few bytes of all files, looking for perf.data files to offer.

That, done in the 'top' tool would make it stop doing continuous
profiling, stopping the thread it uses to read the counters and instead
switch to static profiling, i.e. just reading a perf.data file.

This is something that is a nice new feature and one that would pave the
way for a better top/report/script/annotate integration.

- Arnaldo

> > Can you ellaborate on the assumptions you made while working on this, do
> > they match what I described above as how to use this scripts browser?
> > 
> > So I merged part of this patchset, but will wait till more discussion
> > happens on the browsing part,
> > 
> > Thanks,
> > 
> > - Arnaldo

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

* Re: [PATCH v2 6/7] perf ui/browser: Integrate script browser into annotation browser
  2012-09-18 11:30       ` Arnaldo Carvalho de Melo
@ 2012-09-18 15:56         ` Feng Tang
  0 siblings, 0 replies; 17+ messages in thread
From: Feng Tang @ 2012-09-18 15:56 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: mingo, a.p.zijlstra, andi, namhyung, dsahern, linux-kernel

Hi Arnaldo,

On Tue, 18 Sep 2012 08:30:28 -0300
Arnaldo Carvalho de Melo <acme@redhat.com> wrote:

> Em Tue, Sep 18, 2012 at 03:40:10PM +0800, Feng Tang escreveu:
> > Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
> > > Em Fri, Sep 07, 2012 at 04:42:28PM +0800, Feng Tang escreveu:
> > > > Integrate the script browser into annotation, users can press function
> > > > key 'r' to list all perf scripts and select one of them to run that
> > > > script, the output will be shown in a separate browser.
> > > 
> > > I think this needs a bit more work... i.e. I tried it with your script,
> > > event_analyzing_sample and I kinda get meaningful output when pressing
> > > 'r' + that script from the top and hists browser.
> > 
> > I would answer you question about "elllaborate on the assumptions" first
> > here, that the main purpose of this script browser is not to replace the
> > normal "perf script" command, but to:
> > 1. Provide a convenient way for user to directly run scripts while they
> 
> I think this is what confused me, you say "directly run scripts", but
> that only means "directly run the report phase of scripts, not the
> record phase". Ok, that is clear now.
> 
> > are browsing through the hists or annotation browser, and provide some
> > option for run script with samples from specific thread or symbol
> 
> > 2. Prepare for some advance feature, like inside the annotation browser,
> > analyze some specific hot line of code with samples containing precise
> > information, like the register values, store/load latency stuff. But I
> > have no idea how to implement it now, I guess after Stefan work out the
> > general perf latency tool, we may be able to revisit it.   
> 
> I'm ok with adding infrastructure for later improvements, but I think we
> should try to do it in a way that doesn't confuses users too much,
> otherwise they may get annoyed and not try the feature when it becomes
> useful, worse, tell others that it is cumbersome, don't bother trying
> it.

OK, will try to make it clear in the commit log and comments inside
the code.

> 
> So we should try to do it in a way that is useful for the constraints
> you explained here, more on that later in this message.
>  
> > And when I started to code, I assumed user will mainly use this feature
> > for running scripts for general samples other than the existing scripts
> > for tracepoints. And yes, some user cases you mentioned are not covered.
> 
> Ok, so we need to filter out some more, like you did for the top script.
>  
> > > But with other scripts, and you filtered the -top suffixed ones, that
> > > require special handling, I get things like "Press Control+C to get a
> > > summary", and when I press that, the browser exits, going back to
> > > annotate or report/top browser.
> > 
> > That's right, 4 current perf python scripts will print that line in its
> > "trace_begin" function, as some of the scripts may run for quiet some
> > time if the perf.data is huge. And what you see in the script browser 
> > is just some static text of the script output. 
> 
> Ok, so one thing I think we can do is to look at the record script:
> 
> [acme@sandy linux]$ cat
> tools/perf/scripts/python/bin/syscall-counts-by-pid-record 
> #!/bin/bash
> perf record -e raw_syscalls:sys_enter $@
> 
> and also at the perf.data file:
> 
> [root@sandy linux]# perf evlist
> cycles
> 
> See? we can't offer the syscall-counts-by-pid script to analyse this
> perf.data file, one is made for the raw_syscalls:sys_enter tracepoint
> while the other has only cpu cycles.

Yeah, good idea.

> 
> While the script you provided:
> 
> [acme@sandy linux]$ cat
> tools/perf/scripts/python/bin/event_analyzing_sample-record 
> #!/bin/bash
> 
> #
> # event_analyzing_sample.py can cover all type of perf samples including
> # the tracepoints, so no special record requirements, just record what
> # you want to analyze.
> #
> perf record $@
> [acme@sandy linux]$ 
> 
> -----------------------
> 
> You made it general purpose, so yeah, we can offer that script to the
> user analysing that perf.data file.
> 
> So this is one possible way of matching scripts to perf.data files,
> another would be for us to somehow annotate the scripts, in the first
> few lines, with markers that would help do this matching, but I'd start
> with what we have: parse the line that has the 'perf record' command in
> the tools/perf/scripts/python/bin/*-record file, look at the event it
> handles and match to what 'perf evlist' reports.
> 
> Not necessarily running 'perf evlist', just doing what it does to figure
> out the needed information. 

OK, will try this dynamic way for scripts filtering (comparing to the
static way of filtering xxxtop scripts) 

> 
> > > I.e. this only works if we are processing a perf.data file made
> > > specially for that specific script, right? I.e. the record phase is not
> > > integrated at all, just running specific scripts in specific perf.data
> > > files.
> > 
> > No, the record phase is not added, it just handle the recorded data. And
> > there is something missed in current implementation, that the script in
> > browser mode is only run for the "perf.data" even if the perf report/annotate
> > is run for data with another name.
> 
> Ok, so that needs to be handled, using the filtering I suggested above.

And I need to pass the filename of the perf.data to the script browser
in case the perf report/annotate use the "-i" option.

> 
> > > How to allow the user to chose appropriate scripts to run each perf.data
> > > file is an aspect of usability that is missing...
> > 
> > Do we need to run script for another perf.data when we run report/annotation
> > for one perf.data? or should we add a option for script browser to make it
> > work in browser mode, this is something Numhyung has mentioned in his review
> 
> Well, it would be great to be able to press some key in the main hists
> browser and then switch to a different perf.data file, the tool could
> just look at the current directory, looking at the magic number in the
> first few bytes of all files, looking for perf.data files to offer.
> 
> That, done in the 'top' tool would make it stop doing continuous
> profiling, stopping the thread it uses to read the counters and instead
> switch to static profiling, i.e. just reading a perf.data file.
> 
> This is something that is a nice new feature and one that would pave the
> way for a better top/report/script/annotate integration.

I will try to see what I can do with this.

One thing I need confirm is, the filtering method you mentioned above
is only against the perf.data that is currently processed by the
report/annotate browser, and not related with the perf.data switching?

Thanks,
Feng



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

* [tip:perf/core] perf symbols: Filter samples with unresolved symbol when "--symbols" option is used
  2012-09-07  8:42 ` [PATCH v2 1/7] perf symbols: Filter samples with unresolved symbol when "--symbols" option is used Feng Tang
@ 2012-09-19 15:21   ` tip-bot for Feng Tang
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Feng Tang @ 2012-09-19 15:21 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, andi, a.p.zijlstra, namhyung,
	dsahern, tglx, feng.tang, mingo

Commit-ID:  1500b93b61fc70a1176871b64f1c8ae3bd4da9dd
Gitweb:     http://git.kernel.org/tip/1500b93b61fc70a1176871b64f1c8ae3bd4da9dd
Author:     Feng Tang <feng.tang@intel.com>
AuthorDate: Fri, 7 Sep 2012 16:42:23 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:10:57 -0300

perf symbols: Filter samples with unresolved symbol when "--symbols" option is used

Report/top commands support to only handle specific symbols with
"--symbols" option, but current code will keep those samples whose
symbol can't be resolved, which should actually be filtered.

If we run following commands:
$perf record -a tree
$perf report --symbols intel_idle -n
the output will be:

Without the patch:
==================
    46.27%        156     sshd  [unknown]
    26.05%         48  swapper  [kernel.kallsyms]
    17.26%         38     tree  libc-2.12.1.so
     7.69%         17     tree  tree
     2.73%          6     tree  ld-2.12.1.so

With the patch:
===============
   100.00%         48  swapper  [kernel.kallsyms]

Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1347007349-3102-2-git-send-email-feng.tang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 8202f5c..6715b19 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -904,8 +904,9 @@ int perf_event__preprocess_sample(const union perf_event *event,
 		al->sym = map__find_symbol(al->map, al->addr, filter);
 	}
 
-	if (symbol_conf.sym_list && al->sym &&
-	    !strlist__has_entry(symbol_conf.sym_list, al->sym->name))
+	if (symbol_conf.sym_list &&
+		(!al->sym || !strlist__has_entry(symbol_conf.sym_list,
+						al->sym->name)))
 		goto out_filtered;
 
 	return 0;

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

* [tip:perf/core] perf scripts: Add --symbols option to handle specific symbols
  2012-09-07  8:42 ` [PATCH v2 2/7] perf scripts: Add --symbols option to handle specific symbols Feng Tang
@ 2012-09-19 15:22   ` tip-bot for Feng Tang
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Feng Tang @ 2012-09-19 15:22 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, andi, a.p.zijlstra, namhyung,
	dsahern, tglx, feng.tang

Commit-ID:  36385be55da10b3271407c45c3a62d9af3db666e
Gitweb:     http://git.kernel.org/tip/36385be55da10b3271407c45c3a62d9af3db666e
Author:     Feng Tang <feng.tang@intel.com>
AuthorDate: Fri, 7 Sep 2012 16:42:24 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:11:06 -0300

perf scripts: Add --symbols option to handle specific symbols

Since perf script no longer only handle the trace points, we can add the
symbol filter option so that scripts can handle specified samples.

Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1347007349-3102-3-git-send-email-feng.tang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-script.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 6d98a83..76577e6 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -14,6 +14,7 @@
 #include "util/util.h"
 #include "util/evlist.h"
 #include "util/evsel.h"
+#include "util/sort.h"
 #include <linux/bitmap.h>
 
 static char const		*script_name;
@@ -1143,6 +1144,8 @@ static const struct option options[] = {
 		     parse_output_fields),
 	OPT_BOOLEAN('a', "all-cpus", &system_wide,
 		     "system-wide collection from all CPUs"),
+	OPT_STRING('S', "symbols", &symbol_conf.sym_list_str, "symbol[,symbol...]",
+		   "only consider these symbols"),
 	OPT_STRING('C', "cpu", &cpu_list, "cpu", "list of cpus to profile"),
 	OPT_STRING('c', "comms", &symbol_conf.comm_list_str, "comm[,comm...]",
 		   "only display events for these comms"),

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

* [tip:perf/core] perf scripts: Add event_analyzing_sample-record/ report
  2012-09-07  8:42 ` [PATCH v2 3/7] perf scripts: Add event_analyzing_sample-record/report Feng Tang
@ 2012-09-19 15:23   ` tip-bot for Feng Tang
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Feng Tang @ 2012-09-19 15:23 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, andi, a.p.zijlstra, namhyung,
	dsahern, tglx, feng.tang, mingo

Commit-ID:  59cbea229473350168930941986ebe5bf685cc23
Gitweb:     http://git.kernel.org/tip/59cbea229473350168930941986ebe5bf685cc23
Author:     Feng Tang <feng.tang@intel.com>
AuthorDate: Fri, 7 Sep 2012 16:42:25 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:11:15 -0300

perf scripts: Add event_analyzing_sample-record/report

So that event_analyzing_sample.py can be shown by "perf script -l"

Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1347007349-3102-4-git-send-email-feng.tang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 .../python/bin/event_analyzing_sample-record       |    8 ++++++++
 .../python/bin/event_analyzing_sample-report       |    3 +++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/tools/perf/scripts/python/bin/event_analyzing_sample-record b/tools/perf/scripts/python/bin/event_analyzing_sample-record
new file mode 100644
index 0000000..5ce652d
--- /dev/null
+++ b/tools/perf/scripts/python/bin/event_analyzing_sample-record
@@ -0,0 +1,8 @@
+#!/bin/bash
+
+#
+# event_analyzing_sample.py can cover all type of perf samples including
+# the tracepoints, so no special record requirements, just record what
+# you want to analyze.
+#
+perf record $@
diff --git a/tools/perf/scripts/python/bin/event_analyzing_sample-report b/tools/perf/scripts/python/bin/event_analyzing_sample-report
new file mode 100644
index 0000000..0941fc9
--- /dev/null
+++ b/tools/perf/scripts/python/bin/event_analyzing_sample-report
@@ -0,0 +1,3 @@
+#!/bin/bash
+# description: analyze all perf samples
+perf script $@ -s "$PERF_EXEC_PATH"/scripts/python/event_analyzing_sample.py

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

* [tip:perf/core] perf scripts: Export a find_scripts() function
  2012-09-07  8:42 ` [PATCH v2 4/7] perf scripts: Export a find_scripts() function Feng Tang
@ 2012-09-19 15:24   ` tip-bot for Feng Tang
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Feng Tang @ 2012-09-19 15:24 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, hpa, mingo, andi, a.p.zijlstra, namhyung,
	dsahern, tglx, feng.tang, mingo

Commit-ID:  e5f3705e62b03251797a5173024184bfc223599d
Gitweb:     http://git.kernel.org/tip/e5f3705e62b03251797a5173024184bfc223599d
Author:     Feng Tang <feng.tang@intel.com>
AuthorDate: Fri, 7 Sep 2012 16:42:26 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:11:26 -0300

perf scripts: Export a find_scripts() function

So that other perf commands/browser has a way to dig out the available
scripts info in system, this is a preparation for the script browser.

Signed-off-by: Feng Tang <feng.tang@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1347007349-3102-5-git-send-email-feng.tang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-script.c |   55 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/builtin.h        |    1 +
 2 files changed, 56 insertions(+), 0 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 76577e6..1be843a 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1032,6 +1032,61 @@ static int list_available_scripts(const struct option *opt __maybe_unused,
 	exit(0);
 }
 
+/*
+ * Return -1 if none is found, otherwise the actual scripts number.
+ *
+ * Currently the only user of this function is the script browser, which
+ * will list all statically runnable scripts, select one, execute it and
+ * show the output in a perf browser.
+ */
+int find_scripts(char **scripts_array, char **scripts_path_array)
+{
+	struct dirent *script_next, *lang_next, script_dirent, lang_dirent;
+	char scripts_path[MAXPATHLEN];
+	DIR *scripts_dir, *lang_dir;
+	char lang_path[MAXPATHLEN];
+	char *temp;
+	int i = 0;
+
+	snprintf(scripts_path, MAXPATHLEN, "%s/scripts", perf_exec_path());
+
+	scripts_dir = opendir(scripts_path);
+	if (!scripts_dir)
+		return -1;
+
+	for_each_lang(scripts_path, scripts_dir, lang_dirent, lang_next) {
+		snprintf(lang_path, MAXPATHLEN, "%s/%s", scripts_path,
+			 lang_dirent.d_name);
+#ifdef NO_LIBPERL
+		if (strstr(lang_path, "perl"))
+			continue;
+#endif
+#ifdef NO_LIBPYTHON
+		if (strstr(lang_path, "python"))
+			continue;
+#endif
+
+		lang_dir = opendir(lang_path);
+		if (!lang_dir)
+			continue;
+
+		for_each_script(lang_path, lang_dir, script_dirent, script_next) {
+			/* Skip those real time scripts: xxxtop.p[yl] */
+			if (strstr(script_dirent.d_name, "top."))
+				continue;
+			sprintf(scripts_path_array[i], "%s/%s", lang_path,
+				script_dirent.d_name);
+			temp = strchr(script_dirent.d_name, '.');
+			snprintf(scripts_array[i],
+				(temp - script_dirent.d_name) + 1,
+				"%s", script_dirent.d_name);
+			i++;
+		}
+	}
+
+	return i;
+}
+
 static char *get_script_path(const char *script_root, const char *suffix)
 {
 	struct dirent *script_next, *lang_next, script_dirent, lang_dirent;
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index b382bd5..3ea74ed 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -36,4 +36,5 @@ extern int cmd_kvm(int argc, const char **argv, const char *prefix);
 extern int cmd_test(int argc, const char **argv, const char *prefix);
 extern int cmd_inject(int argc, const char **argv, const char *prefix);
 
+extern int find_scripts(char **scripts_array, char **scripts_path_array);
 #endif

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

end of thread, other threads:[~2012-09-19 15:24 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-07  8:42 [PATCH v2 0/7] perf ui/browser: Add browser for perf script Feng Tang
2012-09-07  8:42 ` [PATCH v2 1/7] perf symbols: Filter samples with unresolved symbol when "--symbols" option is used Feng Tang
2012-09-19 15:21   ` [tip:perf/core] " tip-bot for Feng Tang
2012-09-07  8:42 ` [PATCH v2 2/7] perf scripts: Add --symbols option to handle specific symbols Feng Tang
2012-09-19 15:22   ` [tip:perf/core] " tip-bot for Feng Tang
2012-09-07  8:42 ` [PATCH v2 3/7] perf scripts: Add event_analyzing_sample-record/report Feng Tang
2012-09-19 15:23   ` [tip:perf/core] perf scripts: Add event_analyzing_sample-record/ report tip-bot for Feng Tang
2012-09-07  8:42 ` [PATCH v2 4/7] perf scripts: Export a find_scripts() function Feng Tang
2012-09-19 15:24   ` [tip:perf/core] " tip-bot for Feng Tang
2012-09-07  8:42 ` [PATCH v2 5/7] perf ui/browser: Add a browser for perf script Feng Tang
2012-09-07  8:42 ` [PATCH v2 6/7] perf ui/browser: Integrate script browser into annotation browser Feng Tang
2012-09-17 15:23   ` Arnaldo Carvalho de Melo
2012-09-18  7:40     ` Feng Tang
2012-09-18 11:30       ` Arnaldo Carvalho de Melo
2012-09-18 15:56         ` Feng Tang
2012-09-07  8:42 ` [PATCH v2 7/7] perf ui/browser: Integrate script browser into main hists browser Feng Tang
2012-09-10  1:35 ` [PATCH v2 0/7] perf ui/browser: Add browser for perf script Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).