2010-08-21 17:45:48

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 0/2] perf/core improvements

Hi Ingo,

Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core

Regards,

- Arnaldo

Arnaldo Carvalho de Melo (1):
perf tools: Add --tui and --stdio to choose the UI

Kirill A. Shutemov (1):
perf tools: add test for strlcpy()

tools/perf/Documentation/perf-annotate.txt | 11 +++++++++--
tools/perf/Documentation/perf-report.txt | 7 +++++++
tools/perf/Makefile | 9 +++++++++
tools/perf/builtin-annotate.c | 9 ++++++++-
tools/perf/builtin-report.c | 11 ++++++++++-
tools/perf/feature-tests.mak | 11 +++++++++++
tools/perf/util/cache.h | 2 ++
tools/perf/util/path.c | 3 ++-
8 files changed, 58 insertions(+), 5 deletions(-)


2010-08-21 17:45:29

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/2] perf tools: Add --tui and --stdio to choose the UI

From: Arnaldo Carvalho de Melo <[email protected]>

Relying just on ~/.perfconfig or rebuilding the tool disabling support
for the TUI is too cumbersome, so allow specifying which UI to use and
make the command line switch override whatever is in ~/.perfconfig.

Suggested-by: Christoph Hellwig <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Documentation/perf-annotate.txt | 11 +++++++++--
tools/perf/Documentation/perf-report.txt | 7 +++++++
tools/perf/builtin-annotate.c | 9 ++++++++-
tools/perf/builtin-report.c | 11 ++++++++++-
4 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
index 5164a65..b2c6330 100644
--- a/tools/perf/Documentation/perf-annotate.txt
+++ b/tools/perf/Documentation/perf-annotate.txt
@@ -8,7 +8,7 @@ perf-annotate - Read perf.data (created by perf record) and display annotated co
SYNOPSIS
--------
[verse]
-'perf annotate' [-i <file> | --input=file] symbol_name
+'perf annotate' [-i <file> | --input=file] [symbol_name]

DESCRIPTION
-----------
@@ -24,6 +24,13 @@ OPTIONS
--input=::
Input file name. (default: perf.data)

+--stdio:: Use the stdio interface.
+
+--tui:: Use the TUI interface Use of --tui requires a tty, if one is not
+ present, as when piping to other commands, the stdio interface is
+ used. This interfaces starts by centering on the line with more
+ samples, TAB/UNTAB cycles thru the lines with more samples.
+
SEE ALSO
--------
-linkperf:perf-record[1]
+linkperf:perf-record[1], linkperf:perf-report[1]
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index abfabe9..12052c9 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -65,6 +65,13 @@ OPTIONS
the tree is considered as a new profiled object. +
Default: fractal,0.5.

+--stdio:: Use the stdio interface.
+
+--tui:: Use the TUI interface, that is integrated with annotate and allows
+ zooming into DSOs or threads, among other features. Use of --tui
+ requires a tty, if one is not present, as when piping to other
+ commands, the stdio interface is used.
+
SEE ALSO
--------
linkperf:perf-stat[1]
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 20ee21d..6d5604d 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -28,7 +28,7 @@

static char const *input_name = "perf.data";

-static bool force;
+static bool force, use_tui, use_stdio;

static bool full_paths;

@@ -427,6 +427,8 @@ static const struct option options[] = {
"be more verbose (show symbol address, etc)"),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
"dump raw trace in ASCII"),
+ OPT_BOOLEAN(0, "tui", &use_tui, "Use the TUI interface"),
+ OPT_BOOLEAN(0, "stdio", &use_stdio, "Use the stdio interface"),
OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
"file", "vmlinux pathname"),
OPT_BOOLEAN('m', "modules", &symbol_conf.use_modules,
@@ -442,6 +444,11 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __used)
{
argc = parse_options(argc, argv, options, annotate_usage, 0);

+ if (use_stdio)
+ use_browser = 0;
+ else if (use_tui)
+ use_browser = 1;
+
setup_browser();

symbol_conf.priv_size = sizeof(struct sym_priv);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 55fc1f4..7d35a71 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -32,7 +32,7 @@

static char const *input_name = "perf.data";

-static bool force;
+static bool force, use_tui, use_stdio;
static bool hide_unresolved;
static bool dont_use_callchains;

@@ -450,6 +450,8 @@ static const struct option options[] = {
"Show per-thread event counters"),
OPT_STRING(0, "pretty", &pretty_printing_style, "key",
"pretty printing style key: normal raw"),
+ OPT_BOOLEAN(0, "tui", &use_tui, "Use the TUI interface"),
+ OPT_BOOLEAN(0, "stdio", &use_stdio, "Use the stdio interface"),
OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
"sort by key(s): pid, comm, dso, symbol, parent"),
OPT_BOOLEAN(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
@@ -482,8 +484,15 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
{
argc = parse_options(argc, argv, options, report_usage, 0);

+ if (use_stdio)
+ use_browser = 0;
+ else if (use_tui)
+ use_browser = 1;
+
if (strcmp(input_name, "-") != 0)
setup_browser();
+ else
+ use_browser = 0;
/*
* Only in the newt browser we are doing integrated annotation,
* so don't allocate extra space that won't be used in the stdio
--
1.6.2.5

2010-08-21 17:45:32

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 2/2] perf tools: add test for strlcpy()

From: Kirill A. Shutemov <[email protected]>

Some Linux distributions like ALT Linux provides patched glibc with
contains strlcpy(). It's confilcts with strlcpy() from perf.

Let's add check for strlcpy().

Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Kirill A. Shutemov <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Makefile | 9 +++++++++
tools/perf/feature-tests.mak | 11 +++++++++++
tools/perf/util/cache.h | 2 ++
tools/perf/util/path.c | 3 ++-
4 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index dcb9700..7473de0 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -647,6 +647,15 @@ else
endif
endif

+
+ifdef NO_STRLCPY
+ BASIC_CFLAGS += -DNO_STRLCPY
+else
+ ifneq ($(call try-cc,$(SOURCE_STRLCPY),),y)
+ BASIC_CFLAGS += -DNO_STRLCPY
+ endif
+endif
+
ifndef CC_LD_DYNPATH
ifdef NO_R_TO_GCC_LINKER
# Some gcc does not accept and pass -R to the linker to specify
diff --git a/tools/perf/feature-tests.mak b/tools/perf/feature-tests.mak
index ddb68e6..ef3d0d6 100644
--- a/tools/perf/feature-tests.mak
+++ b/tools/perf/feature-tests.mak
@@ -110,6 +110,17 @@ int main(void)
}
endef

+define SOURCE_STRLCPY
+#include <stdlib.h>
+extern size_t strlcpy(char *dest, const char *src, size_t size);
+
+int main(void)
+{
+ strlcpy(NULL, NULL, 0);
+ return 0;
+}
+endef
+
# try-cc
# Usage: option = $(call try-cc, source-to-build, cc-options)
try-cc = $(shell sh -c \
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 27e9ebe..a772979 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -82,6 +82,8 @@ extern char *perf_path(const char *fmt, ...) __attribute__((format (printf, 1, 2
extern char *perf_pathdup(const char *fmt, ...)
__attribute__((format (printf, 1, 2)));

+#ifdef NO_STRLCPY
extern size_t strlcpy(char *dest, const char *src, size_t size);
+#endif

#endif /* __PERF_CACHE_H */
diff --git a/tools/perf/util/path.c b/tools/perf/util/path.c
index 58a470d..bd74977 100644
--- a/tools/perf/util/path.c
+++ b/tools/perf/util/path.c
@@ -22,6 +22,7 @@ static const char *get_perf_dir(void)
return ".";
}

+#ifdef NO_STRLCPY
size_t strlcpy(char *dest, const char *src, size_t size)
{
size_t ret = strlen(src);
@@ -33,7 +34,7 @@ size_t strlcpy(char *dest, const char *src, size_t size)
}
return ret;
}
-
+#endif

static char *get_pathname(void)
{
--
1.6.2.5

2010-08-21 17:48:00

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf tools: Add --tui and --stdio to choose the UI

On Sat, Aug 21, 2010 at 02:45:11PM -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <[email protected]>
>
> Relying just on ~/.perfconfig or rebuilding the tool disabling support
> for the TUI is too cumbersome, so allow specifying which UI to use and
> make the command line switch override whatever is in ~/.perfconfig.

Much better than the current situation, thanks!

I still thinks commands like perf report should not be overloaded for
two different user interfaces, that is a

perf report

should always be the line oriented interface, and the newt UI should be
something like

perf treport

Btw, am I the only one that doesn't find the next interface too helpful?
In general the line oriented interface seem to present the information
much better, and in the cases where it gets too complicated the newt
interface doesn't really helper either. A real gtk/qt interface with
a proper tree widget would seem like the more useful interface for that.

2010-08-21 18:27:10

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf tools: Add --tui and --stdio to choose the UI

Em Sat, Aug 21, 2010 at 01:47:54PM -0400, Christoph Hellwig escreveu:
> On Sat, Aug 21, 2010 at 02:45:11PM -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <[email protected]>
> >
> > Relying just on ~/.perfconfig or rebuilding the tool disabling support
> > for the TUI is too cumbersome, so allow specifying which UI to use and
> > make the command line switch override whatever is in ~/.perfconfig.
>
> Much better than the current situation, thanks!
>
> I still thinks commands like perf report should not be overloaded for
> two different user interfaces, that is a
>
> perf report
>
> should always be the line oriented interface, and the newt UI should be
> something like
>
> perf treport
>
> Btw, am I the only one that doesn't find the next interface too helpful?
> In general the line oriented interface seem to present the information
> much better, and in the cases where it gets too complicated the newt

Much better how, exactly?

> interface doesn't really helper either. A real gtk/qt interface with
> a proper tree widget would seem like the more useful interface for that.

What would a gtk/qt interface buy us that is not present on the TUI
right now?

I'm not saying it wouldn't buy something, just interested in what is
this something :)

And yes, gtk/qt interfaces should be added. The experience with the TUI
is helping organize the data structures for that.

- Arnaldo

2010-08-21 19:15:36

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL 0/2] perf/core improvements


* Arnaldo Carvalho de Melo <[email protected]> wrote:

> Hi Ingo,
>
> Please pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core
>
> Regards,
>
> - Arnaldo
>
> Arnaldo Carvalho de Melo (1):
> perf tools: Add --tui and --stdio to choose the UI
>
> Kirill A. Shutemov (1):
> perf tools: add test for strlcpy()
>
> tools/perf/Documentation/perf-annotate.txt | 11 +++++++++--
> tools/perf/Documentation/perf-report.txt | 7 +++++++
> tools/perf/Makefile | 9 +++++++++
> tools/perf/builtin-annotate.c | 9 ++++++++-
> tools/perf/builtin-report.c | 11 ++++++++++-
> tools/perf/feature-tests.mak | 11 +++++++++++
> tools/perf/util/cache.h | 2 ++
> tools/perf/util/path.c | 3 ++-
> 8 files changed, 58 insertions(+), 5 deletions(-)

Pulled, thanks a lot Arnaldo!

Ingo

2010-08-22 08:20:09

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf tools: Add --tui and --stdio to choose the UI

On Sat, Aug 21, 2010 at 03:26:58PM -0300, Arnaldo Carvalho de Melo wrote:
> > Btw, am I the only one that doesn't find the next interface too helpful?
> > In general the line oriented interface seem to present the information
> > much better, and in the cases where it gets too complicated the newt
>
> Much better how, exactly?

- it uses the space available in a terminal window much more
efficiently. Output starts in row 0, column 0 and expands all the
way that's needed.
- it incluses a descruption of what the columns of output mean
- it provides percentages for the individual callchains parts after
branching out
- in graph/fractal modes there are lines that show how the parts
are connected, making it possible to read the output, unlinke
the TUI mode where the "expanind" nodes are very hard to follow.
- just running perf reports gets an overview of all callchains instead
of having to expand dozens of things
- the colour scheme is the normal shell one (light gray on black for
me) instead of one hurting the eyes


> > interface doesn't really helper either. A real gtk/qt interface with
> > a proper tree widget would seem like the more useful interface for that.
>
> What would a gtk/qt interface buy us that is not present on the TUI
> right now?

Generally for tree/graph like data structures a real X GUI provides much
nicer rendering. It'll allow to render the connected lines just as in
the line oriented interface, but in a nicer way using real thin lines
in a tree widget. It'll also genereally allow better text placement
in the window compared to the single centered window in the newt
interface. Also the GUI programs follow the common color scheme of the
rest of the desktop instead of hurting your eye. In addition to that
scrollbars (horizontal / vertical) are a lot more intuitive in a typical
X GUI than in newt.

2010-08-22 14:11:39

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf tools: Add --tui and --stdio to choose the UI

Em Sun, Aug 22, 2010 at 04:20:03AM -0400, Christoph Hellwig escreveu:
> On Sat, Aug 21, 2010 at 03:26:58PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Btw, am I the only one that doesn't find the next interface too helpful?
> > > In general the line oriented interface seem to present the information
> > > much better, and in the cases where it gets too complicated the newt

> > Much better how, exactly?

> - it uses the space available in a terminal window much more
> efficiently. Output starts in row 0, column 0 and expands all the
> way that's needed.
> - it incluses a descruption of what the columns of output mean
> - it provides percentages for the individual callchains parts after
> branching out
> - in graph/fractal modes there are lines that show how the parts
> are connected, making it possible to read the output, unlinke
> the TUI mode where the "expanind" nodes are very hard to follow.
> - just running perf reports gets an overview of all callchains instead
> of having to expand dozens of things
> - the colour scheme is the normal shell one (light gray on black for
> me) instead of one hurting the eyes

All good points, will work on them, thanks for pointing them out!

>
> > > interface doesn't really helper either. A real gtk/qt interface with
> > > a proper tree widget would seem like the more useful interface for that.
> >
> > What would a gtk/qt interface buy us that is not present on the TUI
> > right now?
>
> Generally for tree/graph like data structures a real X GUI provides much
> nicer rendering. It'll allow to render the connected lines just as in
> the line oriented interface, but in a nicer way using real thin lines

Cool, connected lines is something that indeed has to happen, just like
mutt in threaded mode, will work on that.

> in a tree widget. It'll also genereally allow better text placement
> in the window compared to the single centered window in the newt
> interface. Also the GUI programs follow the common color scheme of the
> rest of the desktop instead of hurting your eye. In addition to that

Lets stop hurting eyes!

> scrollbars (horizontal / vertical) are a lot more intuitive in a typical
> X GUI than in newt.

Ok, thanks for the detailed description of the problems.

- Arnaldo