* [PATCH 01/12] perf diff: Make diff command work with evsel hists
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
@ 2012-09-06 15:46 ` Jiri Olsa
2012-09-08 11:41 ` [tip:perf/core] " tip-bot for Jiri Olsa
2012-09-06 15:46 ` [PATCH 02/12] perf tools: Replace sort's standalone field_sep with symbol_conf.field_sep Jiri Olsa
` (12 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:46 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
Putting 'perf diff' command back on track with the 'latest'
evsel hists changes. Each evsel has its own 'hists' object
gathering stats for the particular event.
While currently counts are accumulated for the whole session
regardless of the events diversification within compared
sessions.
The 'perf diff' command now outputs all matching events within
compared sessions (with event name specified). The per event
diff output stays the same.
$ ./perf diff
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol
# ........ .......... ................. ..............................
#
0.00% +15.14% [kernel.kallsyms] [k] __wake_up
0.00% +13.38% [kernel.kallsyms] [k] ext4fs_dirhash
... SNIP
0.00% +0.42% [kernel.kallsyms] [k] local_clock
0.17% -0.05% [kernel.kallsyms] [k] native_write_msr_safe
# Event 'faults'
#
# Baseline Delta Shared Object Symbol
# ........ .......... ................. ..............................
#
0.00% +79.12% ld-2.15.so [.] _dl_relocate_object
0.00% +11.62% ld-2.15.so [.] openaux
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 3 ++
tools/perf/builtin-diff.c | 93 ++++++++++++++++++++++------------
tools/perf/util/evsel.h | 7 +++
tools/perf/util/session.h | 4 +-
4 files changed, 72 insertions(+), 35 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 74d7481..ab7f667 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -17,6 +17,9 @@ captured via perf record.
If no parameters are passed it will assume perf.data.old and perf.data.
+The differential profile is displayed only for events matching both
+specified perf.data files.
+
OPTIONS
-------
-M::
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index d29d350..e9933fd 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -10,6 +10,7 @@
#include "util/event.h"
#include "util/hist.h"
#include "util/evsel.h"
+#include "util/evlist.h"
#include "util/session.h"
#include "util/tool.h"
#include "util/sort.h"
@@ -24,11 +25,6 @@ static char diff__default_sort_order[] = "dso,symbol";
static bool force;
static bool show_displacement;
-struct perf_diff {
- struct perf_tool tool;
- struct perf_session *session;
-};
-
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
{
@@ -37,14 +33,12 @@ static int hists__add_entry(struct hists *self,
return -ENOMEM;
}
-static int diff__process_sample_event(struct perf_tool *tool,
+static int diff__process_sample_event(struct perf_tool *tool __used,
union perf_event *event,
struct perf_sample *sample,
- struct perf_evsel *evsel __used,
+ struct perf_evsel *evsel,
struct machine *machine)
{
- struct perf_diff *_diff = container_of(tool, struct perf_diff, tool);
- struct perf_session *session = _diff->session;
struct addr_location al;
if (perf_event__preprocess_sample(event, machine, &al, sample, NULL) < 0) {
@@ -56,26 +50,24 @@ static int diff__process_sample_event(struct perf_tool *tool,
if (al.filtered || al.sym == NULL)
return 0;
- if (hists__add_entry(&session->hists, &al, sample->period)) {
+ if (hists__add_entry(&evsel->hists, &al, sample->period)) {
pr_warning("problem incrementing symbol period, skipping event\n");
return -1;
}
- session->hists.stats.total_period += sample->period;
+ evsel->hists.stats.total_period += sample->period;
return 0;
}
-static struct perf_diff diff = {
- .tool = {
- .sample = diff__process_sample_event,
- .mmap = perf_event__process_mmap,
- .comm = perf_event__process_comm,
- .exit = perf_event__process_task,
- .fork = perf_event__process_task,
- .lost = perf_event__process_lost,
- .ordered_samples = true,
- .ordering_requires_timestamps = true,
- },
+static struct perf_tool tool = {
+ .sample = diff__process_sample_event,
+ .mmap = perf_event__process_mmap,
+ .comm = perf_event__process_comm,
+ .exit = perf_event__process_task,
+ .fork = perf_event__process_task,
+ .lost = perf_event__process_lost,
+ .ordered_samples = true,
+ .ordering_requires_timestamps = true,
};
static void perf_session__insert_hist_entry_by_name(struct rb_root *root,
@@ -146,34 +138,71 @@ static void hists__match(struct hists *older, struct hists *newer)
}
}
+static struct perf_evsel *evsel_match(struct perf_evsel *evsel,
+ struct perf_evlist *evlist)
+{
+ struct perf_evsel *e;
+
+ list_for_each_entry(e, &evlist->entries, node)
+ if (perf_evsel__match2(evsel, e))
+ return e;
+
+ return NULL;
+}
+
static int __cmd_diff(void)
{
int ret, i;
#define older (session[0])
#define newer (session[1])
struct perf_session *session[2];
+ struct perf_evlist *evlist_new, *evlist_old;
+ struct perf_evsel *evsel;
+ bool first = true;
older = perf_session__new(input_old, O_RDONLY, force, false,
- &diff.tool);
+ &tool);
newer = perf_session__new(input_new, O_RDONLY, force, false,
- &diff.tool);
+ &tool);
if (session[0] == NULL || session[1] == NULL)
return -ENOMEM;
for (i = 0; i < 2; ++i) {
- diff.session = session[i];
- ret = perf_session__process_events(session[i], &diff.tool);
+ ret = perf_session__process_events(session[i], &tool);
if (ret)
goto out_delete;
- hists__output_resort(&session[i]->hists);
}
- if (show_displacement)
- hists__resort_entries(&older->hists);
+ evlist_old = older->evlist;
+ evlist_new = newer->evlist;
+
+ list_for_each_entry(evsel, &evlist_new->entries, node)
+ hists__output_resort(&evsel->hists);
+
+ list_for_each_entry(evsel, &evlist_old->entries, node) {
+ hists__output_resort(&evsel->hists);
+
+ if (show_displacement)
+ hists__resort_entries(&evsel->hists);
+ }
+
+ list_for_each_entry(evsel, &evlist_new->entries, node) {
+ struct perf_evsel *evsel_old;
+
+ evsel_old = evsel_match(evsel, evlist_old);
+ if (!evsel_old)
+ continue;
+
+ fprintf(stdout, "%s# Event '%s'\n#\n", first ? "" : "\n",
+ perf_evsel__name(evsel));
+
+ first = false;
+
+ hists__match(&evsel_old->hists, &evsel->hists);
+ hists__fprintf(&evsel->hists, &evsel_old->hists,
+ show_displacement, true, 0, 0, stdout);
+ }
- hists__match(&older->hists, &newer->hists);
- hists__fprintf(&newer->hists, &older->hists,
- show_displacement, true, 0, 0, stdout);
out_delete:
for (i = 0; i < 2; ++i)
perf_session__delete(session[i]);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 94f6ba1..fa57e48 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -122,6 +122,13 @@ void perf_evsel__close(struct perf_evsel *evsel, int ncpus, int nthreads);
(evsel->attr.type == PERF_TYPE_##t && \
evsel->attr.config == PERF_COUNT_##c)
+static inline bool perf_evsel__match2(struct perf_evsel *e1,
+ struct perf_evsel *e2)
+{
+ return (e1->attr.type == e2->attr.type) &&
+ (e1->attr.config == e2->attr.config);
+}
+
int __perf_evsel__read_on_cpu(struct perf_evsel *evsel,
int cpu, int thread, bool scale);
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 176a609..aab414f 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -36,9 +36,7 @@ struct perf_session {
struct pevent *pevent;
/*
* FIXME: Need to split this up further, we need global
- * stats + per event stats. 'perf diff' also needs
- * to properly support multiple events in a single
- * perf.data file.
+ * stats + per event stats.
*/
struct hists hists;
int fd;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [tip:perf/core] perf diff: Make diff command work with evsel hists
2012-09-06 15:46 ` [PATCH 01/12] perf diff: Make diff command work with evsel hists Jiri Olsa
@ 2012-09-08 11:41 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 40+ messages in thread
From: tip-bot for Jiri Olsa @ 2012-09-08 11:41 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, andi, a.p.zijlstra,
namhyung, jolsa, paulmck, fweisbec, dsahern, tglx, cjashfor,
mingo
Commit-ID: 863e451f69ddf255e877567e325298edd3b21519
Gitweb: http://git.kernel.org/tip/863e451f69ddf255e877567e325298edd3b21519
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Thu, 6 Sep 2012 17:46:55 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 7 Sep 2012 21:44:02 -0300
perf diff: Make diff command work with evsel hists
Putting 'perf diff' command back on track with the 'latest'
evsel hists changes. Each evsel has its own 'hists' object
gathering stats for the particular event.
While currently counts are accumulated for the whole session
regardless of the events diversification within compared
sessions.
The 'perf diff' command now outputs all matching events within
compared sessions (with event name specified). The per event
diff output stays the same.
$ ./perf diff
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol
# ........ .......... ................. ..............................
#
0.00% +15.14% [kernel.kallsyms] [k] __wake_up
0.00% +13.38% [kernel.kallsyms] [k] ext4fs_dirhash
... SNIP
0.00% +0.42% [kernel.kallsyms] [k] local_clock
0.17% -0.05% [kernel.kallsyms] [k] native_write_msr_safe
# Event 'faults'
#
# Baseline Delta Shared Object Symbol
# ........ .......... ................. ..............................
#
0.00% +79.12% ld-2.15.so [.] _dl_relocate_object
0.00% +11.62% ld-2.15.so [.] openaux
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Andi Kleen <andi@firstfloor.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@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1346946426-13496-2-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 3 +
tools/perf/builtin-diff.c | 93 +++++++++++++++++++++-----------
tools/perf/util/evsel.h | 7 +++
tools/perf/util/session.h | 4 +-
4 files changed, 72 insertions(+), 35 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 74d7481..ab7f667 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -17,6 +17,9 @@ captured via perf record.
If no parameters are passed it will assume perf.data.old and perf.data.
+The differential profile is displayed only for events matching both
+specified perf.data files.
+
OPTIONS
-------
-M::
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index d29d350..e9933fd 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -10,6 +10,7 @@
#include "util/event.h"
#include "util/hist.h"
#include "util/evsel.h"
+#include "util/evlist.h"
#include "util/session.h"
#include "util/tool.h"
#include "util/sort.h"
@@ -24,11 +25,6 @@ static char diff__default_sort_order[] = "dso,symbol";
static bool force;
static bool show_displacement;
-struct perf_diff {
- struct perf_tool tool;
- struct perf_session *session;
-};
-
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
{
@@ -37,14 +33,12 @@ static int hists__add_entry(struct hists *self,
return -ENOMEM;
}
-static int diff__process_sample_event(struct perf_tool *tool,
+static int diff__process_sample_event(struct perf_tool *tool __used,
union perf_event *event,
struct perf_sample *sample,
- struct perf_evsel *evsel __used,
+ struct perf_evsel *evsel,
struct machine *machine)
{
- struct perf_diff *_diff = container_of(tool, struct perf_diff, tool);
- struct perf_session *session = _diff->session;
struct addr_location al;
if (perf_event__preprocess_sample(event, machine, &al, sample, NULL) < 0) {
@@ -56,26 +50,24 @@ static int diff__process_sample_event(struct perf_tool *tool,
if (al.filtered || al.sym == NULL)
return 0;
- if (hists__add_entry(&session->hists, &al, sample->period)) {
+ if (hists__add_entry(&evsel->hists, &al, sample->period)) {
pr_warning("problem incrementing symbol period, skipping event\n");
return -1;
}
- session->hists.stats.total_period += sample->period;
+ evsel->hists.stats.total_period += sample->period;
return 0;
}
-static struct perf_diff diff = {
- .tool = {
- .sample = diff__process_sample_event,
- .mmap = perf_event__process_mmap,
- .comm = perf_event__process_comm,
- .exit = perf_event__process_task,
- .fork = perf_event__process_task,
- .lost = perf_event__process_lost,
- .ordered_samples = true,
- .ordering_requires_timestamps = true,
- },
+static struct perf_tool tool = {
+ .sample = diff__process_sample_event,
+ .mmap = perf_event__process_mmap,
+ .comm = perf_event__process_comm,
+ .exit = perf_event__process_task,
+ .fork = perf_event__process_task,
+ .lost = perf_event__process_lost,
+ .ordered_samples = true,
+ .ordering_requires_timestamps = true,
};
static void perf_session__insert_hist_entry_by_name(struct rb_root *root,
@@ -146,34 +138,71 @@ static void hists__match(struct hists *older, struct hists *newer)
}
}
+static struct perf_evsel *evsel_match(struct perf_evsel *evsel,
+ struct perf_evlist *evlist)
+{
+ struct perf_evsel *e;
+
+ list_for_each_entry(e, &evlist->entries, node)
+ if (perf_evsel__match2(evsel, e))
+ return e;
+
+ return NULL;
+}
+
static int __cmd_diff(void)
{
int ret, i;
#define older (session[0])
#define newer (session[1])
struct perf_session *session[2];
+ struct perf_evlist *evlist_new, *evlist_old;
+ struct perf_evsel *evsel;
+ bool first = true;
older = perf_session__new(input_old, O_RDONLY, force, false,
- &diff.tool);
+ &tool);
newer = perf_session__new(input_new, O_RDONLY, force, false,
- &diff.tool);
+ &tool);
if (session[0] == NULL || session[1] == NULL)
return -ENOMEM;
for (i = 0; i < 2; ++i) {
- diff.session = session[i];
- ret = perf_session__process_events(session[i], &diff.tool);
+ ret = perf_session__process_events(session[i], &tool);
if (ret)
goto out_delete;
- hists__output_resort(&session[i]->hists);
}
- if (show_displacement)
- hists__resort_entries(&older->hists);
+ evlist_old = older->evlist;
+ evlist_new = newer->evlist;
+
+ list_for_each_entry(evsel, &evlist_new->entries, node)
+ hists__output_resort(&evsel->hists);
+
+ list_for_each_entry(evsel, &evlist_old->entries, node) {
+ hists__output_resort(&evsel->hists);
+
+ if (show_displacement)
+ hists__resort_entries(&evsel->hists);
+ }
+
+ list_for_each_entry(evsel, &evlist_new->entries, node) {
+ struct perf_evsel *evsel_old;
+
+ evsel_old = evsel_match(evsel, evlist_old);
+ if (!evsel_old)
+ continue;
+
+ fprintf(stdout, "%s# Event '%s'\n#\n", first ? "" : "\n",
+ perf_evsel__name(evsel));
+
+ first = false;
+
+ hists__match(&evsel_old->hists, &evsel->hists);
+ hists__fprintf(&evsel->hists, &evsel_old->hists,
+ show_displacement, true, 0, 0, stdout);
+ }
- hists__match(&older->hists, &newer->hists);
- hists__fprintf(&newer->hists, &older->hists,
- show_displacement, true, 0, 0, stdout);
out_delete:
for (i = 0; i < 2; ++i)
perf_session__delete(session[i]);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index a3f562c..390690e 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -124,6 +124,13 @@ void perf_evsel__close(struct perf_evsel *evsel, int ncpus, int nthreads);
(evsel->attr.type == PERF_TYPE_##t && \
evsel->attr.config == PERF_COUNT_##c)
+static inline bool perf_evsel__match2(struct perf_evsel *e1,
+ struct perf_evsel *e2)
+{
+ return (e1->attr.type == e2->attr.type) &&
+ (e1->attr.config == e2->attr.config);
+}
+
int __perf_evsel__read_on_cpu(struct perf_evsel *evsel,
int cpu, int thread, bool scale);
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 176a609..aab414f 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -36,9 +36,7 @@ struct perf_session {
struct pevent *pevent;
/*
* FIXME: Need to split this up further, we need global
- * stats + per event stats. 'perf diff' also needs
- * to properly support multiple events in a single
- * perf.data file.
+ * stats + per event stats.
*/
struct hists hists;
int fd;
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 02/12] perf tools: Replace sort's standalone field_sep with symbol_conf.field_sep
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
2012-09-06 15:46 ` [PATCH 01/12] perf diff: Make diff command work with evsel hists Jiri Olsa
@ 2012-09-06 15:46 ` Jiri Olsa
2012-09-08 11:42 ` [tip:perf/core] perf tools: Replace sort' s " tip-bot for Jiri Olsa
2012-09-06 15:46 ` [PATCH 03/12] perf hists: Add struct hists pointer to struct hist_entry Jiri Olsa
` (11 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:46 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
The repsep_snprintf function was still using standalone field_sep,
which not even set anymore.
Replacing it with 'symbol_conf.field_sep'.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/util/sort.c | 6 ++----
tools/perf/util/sort.h | 1 -
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 0f5a0a4..7a2fbd8 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -12,8 +12,6 @@ int sort__branch_mode = -1; /* -1 = means not set */
enum sort_type sort__first_dimension;
-char * field_sep;
-
LIST_HEAD(hist_entry__sort_list);
static int repsep_snprintf(char *bf, size_t size, const char *fmt, ...)
@@ -23,11 +21,11 @@ static int repsep_snprintf(char *bf, size_t size, const char *fmt, ...)
va_start(ap, fmt);
n = vsnprintf(bf, size, fmt, ap);
- if (field_sep && n > 0) {
+ if (symbol_conf.field_sep && n > 0) {
char *sep = bf;
while (1) {
- sep = strchr(sep, *field_sep);
+ sep = strchr(sep, *symbol_conf.field_sep);
if (sep == NULL)
break;
*sep = '.';
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index e724b26..e459c98 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -32,7 +32,6 @@ extern const char default_sort_order[];
extern int sort__need_collapse;
extern int sort__has_parent;
extern int sort__branch_mode;
-extern char *field_sep;
extern struct sort_entry sort_comm;
extern struct sort_entry sort_dso;
extern struct sort_entry sort_sym;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [tip:perf/core] perf tools: Replace sort' s standalone field_sep with symbol_conf.field_sep
2012-09-06 15:46 ` [PATCH 02/12] perf tools: Replace sort's standalone field_sep with symbol_conf.field_sep Jiri Olsa
@ 2012-09-08 11:42 ` tip-bot for Jiri Olsa
0 siblings, 0 replies; 40+ messages in thread
From: tip-bot for Jiri Olsa @ 2012-09-08 11:42 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, andi, a.p.zijlstra,
namhyung, jolsa, paulmck, fweisbec, dsahern, tglx, cjashfor,
mingo
Commit-ID: 0ca0c130419a4aa05d28fbecc5d360f051944251
Gitweb: http://git.kernel.org/tip/0ca0c130419a4aa05d28fbecc5d360f051944251
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Thu, 6 Sep 2012 17:46:56 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 7 Sep 2012 21:50:11 -0300
perf tools: Replace sort's standalone field_sep with symbol_conf.field_sep
The repsep_snprintf function was still using standalone field_sep, which
not even set anymore.
Replacing it with 'symbol_conf.field_sep'.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Andi Kleen <andi@firstfloor.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@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1346946426-13496-3-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/sort.c | 6 ++----
tools/perf/util/sort.h | 1 -
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 0f5a0a4..7a2fbd8 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -12,8 +12,6 @@ int sort__branch_mode = -1; /* -1 = means not set */
enum sort_type sort__first_dimension;
-char * field_sep;
-
LIST_HEAD(hist_entry__sort_list);
static int repsep_snprintf(char *bf, size_t size, const char *fmt, ...)
@@ -23,11 +21,11 @@ static int repsep_snprintf(char *bf, size_t size, const char *fmt, ...)
va_start(ap, fmt);
n = vsnprintf(bf, size, fmt, ap);
- if (field_sep && n > 0) {
+ if (symbol_conf.field_sep && n > 0) {
char *sep = bf;
while (1) {
- sep = strchr(sep, *field_sep);
+ sep = strchr(sep, *symbol_conf.field_sep);
if (sep == NULL)
break;
*sep = '.';
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index e724b26..e459c98 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -32,7 +32,6 @@ extern const char default_sort_order[];
extern int sort__need_collapse;
extern int sort__has_parent;
extern int sort__branch_mode;
-extern char *field_sep;
extern struct sort_entry sort_comm;
extern struct sort_entry sort_dso;
extern struct sort_entry sort_sym;
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH 03/12] perf hists: Add struct hists pointer to struct hist_entry
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
2012-09-06 15:46 ` [PATCH 01/12] perf diff: Make diff command work with evsel hists Jiri Olsa
2012-09-06 15:46 ` [PATCH 02/12] perf tools: Replace sort's standalone field_sep with symbol_conf.field_sep Jiri Olsa
@ 2012-09-06 15:46 ` Jiri Olsa
2012-09-06 15:46 ` [PATCH 04/12] perf diff: Refactor diff displacement possition info Jiri Olsa
` (10 subsequent siblings)
13 siblings, 0 replies; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:46 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
Adding pointer back to the parent hists struct for struct hists_entry.
This will be useful in future for any hist_entry's data computation,
that depends on total data of its parent hists.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/util/hist.c | 2 ++
tools/perf/util/sort.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b1817f1..0ed0683 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -325,6 +325,7 @@ struct hist_entry *__hists__add_branch_entry(struct hists *self,
.parent = sym_parent,
.filtered = symbol__parent_filter(sym_parent),
.branch_info = bi,
+ .hists = self,
};
return add_hist_entry(self, &entry, al, period);
@@ -346,6 +347,7 @@ struct hist_entry *__hists__add_entry(struct hists *self,
.period = period,
.parent = sym_parent,
.filtered = symbol__parent_filter(sym_parent),
+ .hists = self,
};
return add_hist_entry(self, &entry, al, period);
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index e459c98..b0b4752 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -78,6 +78,7 @@ struct hist_entry {
struct rb_root sorted_chain;
};
struct branch_info *branch_info;
+ struct hists *hists;
struct callchain_root callchain[0];
};
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 04/12] perf diff: Refactor diff displacement possition info
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (2 preceding siblings ...)
2012-09-06 15:46 ` [PATCH 03/12] perf hists: Add struct hists pointer to struct hist_entry Jiri Olsa
@ 2012-09-06 15:46 ` Jiri Olsa
2012-09-08 0:56 ` Arnaldo Carvalho de Melo
2012-09-06 15:46 ` [PATCH 05/12] perf diff: Refactor stdio ui data columns output Jiri Olsa
` (9 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:46 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
Moving the position calculation into the diff command, so the position
is prepared inside hist_entry data and there's no need to compute in
the output display path.
It will be needed for next cleanup patches.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/builtin-diff.c | 47 ++++++++++++++++++++++++++++++----------------
tools/perf/ui/stdio/hist.c | 4 +---
tools/perf/util/sort.h | 2 +-
3 files changed, 33 insertions(+), 20 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index e9933fd..4e63979 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -70,8 +70,8 @@ static struct perf_tool tool = {
.ordering_requires_timestamps = true,
};
-static void perf_session__insert_hist_entry_by_name(struct rb_root *root,
- struct hist_entry *he)
+static void insert_hist_entry_by_name(struct rb_root *root,
+ struct hist_entry *he)
{
struct rb_node **p = &root->rb_node;
struct rb_node *parent = NULL;
@@ -90,7 +90,7 @@ static void perf_session__insert_hist_entry_by_name(struct rb_root *root,
rb_insert_color(&he->rb_node, root);
}
-static void hists__resort_entries(struct hists *self)
+static void hists__name_resort(struct hists *self, bool sort)
{
unsigned long position = 1;
struct rb_root tmp = RB_ROOT;
@@ -100,12 +100,16 @@ static void hists__resort_entries(struct hists *self)
struct hist_entry *n = rb_entry(next, struct hist_entry, rb_node);
next = rb_next(&n->rb_node);
- rb_erase(&n->rb_node, &self->entries);
n->position = position++;
- perf_session__insert_hist_entry_by_name(&tmp, n);
+
+ if (sort) {
+ rb_erase(&n->rb_node, &self->entries);
+ insert_hist_entry_by_name(&tmp, n);
+ }
}
- self->entries = tmp;
+ if (sort)
+ self->entries = tmp;
}
static struct hist_entry *hists__find_entry(struct hists *self,
@@ -121,7 +125,7 @@ static struct hist_entry *hists__find_entry(struct hists *self,
n = n->rb_left;
else if (cmp > 0)
n = n->rb_right;
- else
+ else
return iter;
}
@@ -150,6 +154,24 @@ static struct perf_evsel *evsel_match(struct perf_evsel *evsel,
return NULL;
}
+static void resort_evlist(struct perf_evlist *evlist, bool name)
+{
+ struct perf_evsel *evsel;
+
+ list_for_each_entry(evsel, &evlist->entries, node) {
+ struct hists *hists = &evsel->hists;
+
+ hists__output_resort(hists);
+
+ /*
+ * The hists__name_resort only sets possition
+ * if name is false.
+ */
+ if (name || ((!name) && show_displacement))
+ hists__name_resort(hists, name);
+ }
+}
+
static int __cmd_diff(void)
{
int ret, i;
@@ -176,15 +198,8 @@ static int __cmd_diff(void)
evlist_old = older->evlist;
evlist_new = newer->evlist;
- list_for_each_entry(evsel, &evlist_new->entries, node)
- hists__output_resort(&evsel->hists);
-
- list_for_each_entry(evsel, &evlist_old->entries, node) {
- hists__output_resort(&evsel->hists);
-
- if (show_displacement)
- hists__resort_entries(&evsel->hists);
- }
+ resort_evlist(evlist_old, true);
+ resort_evlist(evlist_new, false);
list_for_each_entry(evsel, &evlist_new->entries, node) {
struct perf_evsel *evsel_old;
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 9bf7e9e..4ae9ee5 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -472,7 +472,6 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
struct rb_node *nd;
size_t ret = 0;
u64 total_period;
- unsigned long position = 1;
long displacement = 0;
unsigned int width;
const char *sep = symbol_conf.field_sep;
@@ -607,10 +606,9 @@ print_entries:
if (show_displacement) {
if (h->pair != NULL)
displacement = ((long)h->pair->position -
- (long)position);
+ (long)h->position);
else
displacement = 0;
- ++position;
}
ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
displacement, total_period, fp);
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index b0b4752..967d381 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -72,8 +72,8 @@ struct hist_entry {
u8 filtered;
char *srcline;
struct symbol *parent;
+ unsigned long position;
union {
- unsigned long position;
struct hist_entry *pair;
struct rb_root sorted_chain;
};
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 04/12] perf diff: Refactor diff displacement possition info
2012-09-06 15:46 ` [PATCH 04/12] perf diff: Refactor diff displacement possition info Jiri Olsa
@ 2012-09-08 0:56 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 40+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-08 0:56 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Paul Mackerras,
Corey Ashford, Frederic Weisbecker, Paul E. McKenney, Andi Kleen,
David Ahern, Namhyung Kim
Em Thu, Sep 06, 2012 at 05:46:58PM +0200, Jiri Olsa escreveu:
> @@ -150,6 +154,24 @@ static struct perf_evsel *evsel_match(struct perf_evsel *evsel,
> return NULL;
> }
>
> +static void resort_evlist(struct perf_evlist *evlist, bool name)
For consistency, please use:
+static void perf_evlist__resort_hists(struct perf_evlist *evlist, bool name)
> +{
> + struct perf_evsel *evsel;
> +
> + list_for_each_entry(evsel, &evlist->entries, node) {
> + struct hists *hists = &evsel->hists;
> +
> + hists__output_resort(hists);
> +
> + /*
> + * The hists__name_resort only sets possition
> + * if name is false.
> + */
> + if (name || ((!name) && show_displacement))
> + hists__name_resort(hists, name);
- Arnaldo
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 05/12] perf diff: Refactor stdio ui data columns output
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (3 preceding siblings ...)
2012-09-06 15:46 ` [PATCH 04/12] perf diff: Refactor diff displacement possition info Jiri Olsa
@ 2012-09-06 15:46 ` Jiri Olsa
2012-09-07 2:55 ` Namhyung Kim
2012-09-06 15:47 ` [PATCH 06/12] perf diff: Add -b option for perf diff to display paired entries only Jiri Olsa
` (8 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:46 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
Currently for any of the data columns (like Overhead/Period..) in
stdio ui, there's separate code to print header/dots/value scattered
along the display code path.
Adding hists_stdio_column struct to centralize all info needed
to print column header/dots/value.
This change eases up addition for new columns, which is now mostly
matter only of adding new hists_stdio_column struct.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/builtin-diff.c | 14 +-
tools/perf/builtin-report.c | 6 +-
tools/perf/builtin-top.c | 6 +-
tools/perf/ui/stdio/hist.c | 506 +++++++++++++++++++++++++-------------------
tools/perf/ui/stdio/hist.h | 19 ++
tools/perf/util/hist.c | 5 +-
tools/perf/util/hist.h | 19 +-
7 files changed, 345 insertions(+), 230 deletions(-)
create mode 100644 tools/perf/ui/stdio/hist.h
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 4e63979..282fd5e 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -16,6 +16,7 @@
#include "util/sort.h"
#include "util/symbol.h"
#include "util/util.h"
+#include "ui/stdio/hist.h"
#include <stdlib.h>
@@ -214,8 +215,7 @@ static int __cmd_diff(void)
first = false;
hists__match(&evsel_old->hists, &evsel->hists);
- hists__fprintf(&evsel->hists, &evsel_old->hists,
- show_displacement, true, 0, 0, stdout);
+ hists__fprintf(&evsel->hists, true, 0, 0, stdout);
}
out_delete:
@@ -257,6 +257,15 @@ static const struct option options[] = {
OPT_END()
};
+static void setup_ui_stdio(void)
+{
+ hists_stdio_column__register_idx(HISTC_BASELINE);
+ hists_stdio_column__register_global();
+ hists_stdio_column__register_idx(HISTC_DELTA);
+ if (show_displacement)
+ hists_stdio_column__register_idx(HISTC_DISPLACEMENT);
+}
+
int cmd_diff(int argc, const char **argv, const char *prefix __used)
{
sort_order = diff__default_sort_order;
@@ -280,6 +289,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix __used)
return -1;
setup_sorting(diff_usage, options);
+ setup_ui_stdio();
setup_pager();
sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", NULL);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index d618253..c855c9a 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -33,6 +33,7 @@
#include "util/thread.h"
#include "util/sort.h"
#include "util/hist.h"
+#include "ui/stdio/hist.h"
#include <linux/bitmap.h>
@@ -315,12 +316,15 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
{
struct perf_evsel *pos;
+ hists_stdio_column__register_idx(HISTC_OVERHEAD);
+ hists_stdio_column__register_global();
+
list_for_each_entry(pos, &evlist->entries, node) {
struct hists *hists = &pos->hists;
const char *evname = perf_evsel__name(pos);
hists__fprintf_nr_sample_events(hists, evname, stdout);
- hists__fprintf(hists, NULL, false, true, 0, 0, stdout);
+ hists__fprintf(hists, true, 0, 0, stdout);
fprintf(stdout, "\n\n");
}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0513aaa..26da442 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -41,6 +41,7 @@
#include "util/intlist.h"
#include "util/debug.h"
+#include "ui/stdio/hist.h"
#include <assert.h>
#include <elf.h>
@@ -315,7 +316,7 @@ static void perf_top__print_sym_table(struct perf_top *top)
hists__output_recalc_col_len(&top->sym_evsel->hists,
top->winsize.ws_row - 3);
putchar('\n');
- hists__fprintf(&top->sym_evsel->hists, NULL, false, false,
+ hists__fprintf(&top->sym_evsel->hists, false,
top->winsize.ws_row - 4 - printed, win_width, stdout);
}
@@ -607,6 +608,9 @@ static void *display_thread(void *arg)
struct perf_top *top = arg;
int delay_msecs, c;
+ hists_stdio_column__register_idx(HISTC_OVERHEAD);
+ hists_stdio_column__register_global();
+
tcgetattr(0, &save);
tc = save;
tc.c_lflag &= ~(ICANON | ECHO);
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 4ae9ee5..48fb0b3 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -1,10 +1,180 @@
#include <stdio.h>
#include <math.h>
+#include "hist.h"
#include "../../util/util.h"
#include "../../util/hist.h"
#include "../../util/sort.h"
+static double get_period_percent(struct hist_entry *he, u64 period)
+{
+ u64 total = he->hists->stats.total_period;
+ return (period * 100.0) / total;
+}
+
+static int
+hists_stdio_column__delta_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ struct hist_entry *pair = he->pair;
+ double new_percent = get_period_percent(he, he->period);
+ double old_percent = pair ? get_period_percent(pair, pair->period) : 0.0;
+ double diff = new_percent - old_percent;
+ int ret;
+
+ if (fabs(diff) >= 0.01)
+ ret = percent_color_snprintf(bf, size, "%+7.2F%%", diff);
+ else
+ ret = scnprintf(bf, size, "%8s", " ");
+
+ return ret;
+}
+
+static int
+hists_stdio_column__baseline_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ struct hist_entry *pair = he->pair;
+ double percent = pair ? get_period_percent(pair, pair->period) : 0.0;
+
+ return percent_color_snprintf(bf, size, "%7.2F%%", percent);
+}
+
+static int
+hists_stdio_column__overhead_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ double percent = get_period_percent(he, he->period);
+ return percent_color_snprintf(bf, size, "%7.2f%%", percent);
+}
+
+static int
+hists_stdio_column__nr_samples_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ return scnprintf(bf, size, "%12" PRIu64, he->nr_events);
+}
+
+static int
+hists_stdio_column__total_period_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ return scnprintf(bf, size , "%12" PRIu64, he->period);
+}
+
+static int
+hists_stdio_column__cpu_sys_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ double percent = get_period_percent(he, he->period_sys);
+ return percent_color_snprintf(bf, size, "%7.2f%%", percent);
+}
+
+static int
+hists_stdio_column__cpu_us_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ double percent = get_period_percent(he, he->period_us);
+ return percent_color_snprintf(bf, size, "%7.2f%%", percent);
+}
+
+static int
+hists_stdio_column__cpu_guest_sys_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ double percent = get_period_percent(he, he->period_guest_sys);
+ return percent_color_snprintf(bf, size, "%8.2f%%", percent);
+}
+
+static int
+hists_stdio_column__cpu_guest_us_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ double percent = get_period_percent(he, he->period_guest_us);
+ return percent_color_snprintf(bf, size, "%7.2f%%", percent);
+}
+
+static int
+hists_stdio_column__displacement_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ struct hist_entry *pair = he->pair;
+ unsigned long displ = pair ? pair->position - he->position : 0;
+ return scnprintf(bf, size, displ ? "%+5ld" : " ", displ);
+}
+
+LIST_HEAD(hists_stdio_column__list);
+
+#define DEF_COLUMN(name, c, w, h) \
+[ c ] = { \
+ .list = LIST_HEAD_INIT((hists_stdio_column__array[ c ]).list),\
+ .header = h, \
+ .snprintf = hists_stdio_column__ ## name ## _snprintf, \
+ .col_idx = c, \
+ .col_width = w, \
+},
+
+struct hists_stdio_column hists_stdio_column__array[HISTC_STDIO_NR_COLS] = {
+DEF_COLUMN(overhead, HISTC_OVERHEAD, 8, "Overhead")
+DEF_COLUMN(baseline, HISTC_BASELINE, 8, "Baseline")
+DEF_COLUMN(cpu_sys, HISTC_CPU_UTILIZATION_SYS, 8, "sys")
+DEF_COLUMN(cpu_us, HISTC_CPU_UTILIZATION_US, 8, "us")
+DEF_COLUMN(cpu_guest_sys, HISTC_CPU_UTILIZATION_GUEST_SYS, 8, "guest sys")
+DEF_COLUMN(cpu_guest_us, HISTC_CPU_UTILIZATION_GUEST_US, 8, "guest us")
+DEF_COLUMN(nr_samples, HISTC_NR_SAMPLES, 12, "Samples")
+DEF_COLUMN(total_period, HISTC_TOTAL_PERIOD, 12, "Period")
+DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
+DEF_COLUMN(displacement, HISTC_DISPLACEMENT, 5, "Displ")
+};
+
+int hists_stdio_column__register_idx(int idx)
+{
+ struct hists_stdio_column *col;
+
+ if (idx >= HISTC_STDIO_NR_COLS)
+ return -EINVAL;
+
+ col = &hists_stdio_column__array[idx];
+
+ if (!list_empty(&col->list))
+ return -EBUSY;
+
+ list_add_tail(&col->list, &hists_stdio_column__list);
+ return 0;
+}
+
+void hists_stdio_column__set_width(struct hists *hists)
+{
+ struct hists_stdio_column *col;
+ unsigned i;
+
+ for (i = 0; i < HISTC_STDIO_NR_COLS; i++) {
+ col = &hists_stdio_column__array[i];
+ hists__set_col_len(hists, i, col->col_width);
+ }
+}
+
+void hists_stdio_column__register_global(void)
+{
+#define reg(idx) hists_stdio_column__register_idx(idx)
+
+ if (symbol_conf.show_cpu_utilization) {
+ reg(HISTC_CPU_UTILIZATION_SYS);
+ reg(HISTC_CPU_UTILIZATION_US);
+ }
+
+ if (perf_guest) {
+ reg(HISTC_CPU_UTILIZATION_GUEST_SYS);
+ reg(HISTC_CPU_UTILIZATION_GUEST_US);
+ }
+
+ if (symbol_conf.show_nr_samples)
+ reg(HISTC_NR_SAMPLES);
+
+ if (symbol_conf.show_total_period)
+ reg(HISTC_TOTAL_PERIOD);
+#undef reg
+}
static size_t callchain__fprintf_left_margin(FILE *fp, int left_margin)
{
@@ -291,114 +461,20 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
return 0;
}
-static int hist_entry__period_snprintf(struct hist_entry *he, char *s,
- size_t size, struct hists *pair_hists,
- bool show_displacement, long displacement,
- bool color, u64 total_period)
+static int
+hists_stdio_column__snprintf(struct hist_entry *he, char *s, size_t size,
+ struct hists *hists)
{
- u64 period, total, period_sys, period_us, period_guest_sys, period_guest_us;
- u64 nr_events;
const char *sep = symbol_conf.field_sep;
- int ret;
-
- if (symbol_conf.exclude_other && !he->parent)
- return 0;
-
- if (pair_hists) {
- period = he->pair ? he->pair->period : 0;
- nr_events = he->pair ? he->pair->nr_events : 0;
- total = pair_hists->stats.total_period;
- period_sys = he->pair ? he->pair->period_sys : 0;
- period_us = he->pair ? he->pair->period_us : 0;
- period_guest_sys = he->pair ? he->pair->period_guest_sys : 0;
- period_guest_us = he->pair ? he->pair->period_guest_us : 0;
- } else {
- period = he->period;
- nr_events = he->nr_events;
- total = total_period;
- period_sys = he->period_sys;
- period_us = he->period_us;
- period_guest_sys = he->period_guest_sys;
- period_guest_us = he->period_guest_us;
- }
-
- if (total) {
- if (color)
- ret = percent_color_snprintf(s, size,
- sep ? "%.2f" : " %6.2f%%",
- (period * 100.0) / total);
- else
- ret = scnprintf(s, size, sep ? "%.2f" : " %6.2f%%",
- (period * 100.0) / total);
- if (symbol_conf.show_cpu_utilization) {
- ret += percent_color_snprintf(s + ret, size - ret,
- sep ? "%.2f" : " %6.2f%%",
- (period_sys * 100.0) / total);
- ret += percent_color_snprintf(s + ret, size - ret,
- sep ? "%.2f" : " %6.2f%%",
- (period_us * 100.0) / total);
- if (perf_guest) {
- ret += percent_color_snprintf(s + ret,
- size - ret,
- sep ? "%.2f" : " %6.2f%%",
- (period_guest_sys * 100.0) /
- total);
- ret += percent_color_snprintf(s + ret,
- size - ret,
- sep ? "%.2f" : " %6.2f%%",
- (period_guest_us * 100.0) /
- total);
- }
- }
- } else
- ret = scnprintf(s, size, sep ? "%" PRIu64 : "%12" PRIu64 " ", period);
-
- if (symbol_conf.show_nr_samples) {
- if (sep)
- ret += scnprintf(s + ret, size - ret, "%c%" PRIu64, *sep, nr_events);
- else
- ret += scnprintf(s + ret, size - ret, "%11" PRIu64, nr_events);
- }
-
- if (symbol_conf.show_total_period) {
- if (sep)
- ret += scnprintf(s + ret, size - ret, "%c%" PRIu64, *sep, period);
- else
- ret += scnprintf(s + ret, size - ret, " %12" PRIu64, period);
- }
-
- if (pair_hists) {
- char bf[32];
- double old_percent = 0, new_percent = 0, diff;
-
- if (total > 0)
- old_percent = (period * 100.0) / total;
- if (total_period > 0)
- new_percent = (he->period * 100.0) / total_period;
-
- diff = new_percent - old_percent;
-
- if (fabs(diff) >= 0.01)
- scnprintf(bf, sizeof(bf), "%+4.2F%%", diff);
- else
- scnprintf(bf, sizeof(bf), " ");
-
- if (sep)
- ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
- else
- ret += scnprintf(s + ret, size - ret, "%11.11s", bf);
+ struct hists_stdio_column *col;
+ int ret = 0;
- if (show_displacement) {
- if (displacement)
- scnprintf(bf, sizeof(bf), "%+4ld", displacement);
- else
- scnprintf(bf, sizeof(bf), " ");
+ ret = snprintf(s, size, " ");
- if (sep)
- ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
- else
- ret += scnprintf(s + ret, size - ret, "%6.6s", bf);
- }
+ list_for_each_entry(col, &hists_stdio_column__list, list) {
+ ret += scnprintf(s + ret, size - ret, "%s", sep ?: " ");
+ ret += col->snprintf(he, s + ret, size - ret,
+ hists__col_len(hists, col->col_idx));
}
return ret;
@@ -440,9 +516,8 @@ static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
}
static int hist_entry__fprintf(struct hist_entry *he, size_t size,
- struct hists *hists, struct hists *pair_hists,
- bool show_displacement, long displacement,
- u64 total_period, FILE *fp)
+ struct hists *hists, u64 total_period,
+ FILE *fp)
{
char bf[512];
int ret;
@@ -450,9 +525,8 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
if (size == 0 || size > sizeof(bf))
size = sizeof(bf);
- ret = hist_entry__period_snprintf(he, bf, size, pair_hists,
- show_displacement, displacement,
- true, total_period);
+ ret = hists_stdio_column__snprintf(he, bf, size, hists);
+
hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
ret = fprintf(fp, "%s\n", bf);
@@ -464,138 +538,117 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
return ret;
}
-size_t hists__fprintf(struct hists *hists, struct hists *pair,
- bool show_displacement, bool show_header, int max_rows,
- int max_cols, FILE *fp)
+static void fprintf_header(FILE *fp, struct hists *hists,
+ const char *header, int col)
{
- struct sort_entry *se;
- struct rb_node *nd;
- size_t ret = 0;
- u64 total_period;
- long displacement = 0;
- unsigned int width;
- const char *sep = symbol_conf.field_sep;
const char *col_width = symbol_conf.col_width_list_str;
- int nr_rows = 0;
-
- init_rem_hits();
-
- if (!show_header)
- goto print_entries;
+ const char *sep = symbol_conf.field_sep;
+ unsigned int width;
- fprintf(fp, "# %s", pair ? "Baseline" : "Overhead");
+ if (sep) {
+ fprintf(fp, "%c%s", *sep, header);
+ return;
+ }
- if (symbol_conf.show_cpu_utilization) {
- if (sep) {
- ret += fprintf(fp, "%csys", *sep);
- ret += fprintf(fp, "%cus", *sep);
- if (perf_guest) {
- ret += fprintf(fp, "%cguest sys", *sep);
- ret += fprintf(fp, "%cguest us", *sep);
- }
- } else {
- ret += fprintf(fp, " sys ");
- ret += fprintf(fp, " us ");
- if (perf_guest) {
- ret += fprintf(fp, " guest sys ");
- ret += fprintf(fp, " guest us ");
- }
+ width = strlen(header);
+ if (symbol_conf.col_width_list_str) {
+ if (col_width) {
+ hists__set_col_len(hists, col,
+ atoi(col_width));
+ col_width = strchr(col_width, ',');
+ if (col_width)
+ ++col_width;
}
}
- if (symbol_conf.show_nr_samples) {
- if (sep)
- fprintf(fp, "%cSamples", *sep);
- else
- fputs(" Samples ", fp);
- }
+ if (!hists__new_col_len(hists, col, width))
+ width = hists__col_len(hists, col);
- if (symbol_conf.show_total_period) {
- if (sep)
- ret += fprintf(fp, "%cPeriod", *sep);
- else
- ret += fprintf(fp, " Period ");
- }
+ fprintf(fp, " %*s", width, header);
+}
- if (pair) {
- if (sep)
- ret += fprintf(fp, "%cDelta", *sep);
- else
- ret += fprintf(fp, " Delta ");
+static void fprintf_dots(FILE *fp, struct hists *hists,
+ const char *header, int col)
+{
+ unsigned int width;
+ unsigned int i;
- if (show_displacement) {
- if (sep)
- ret += fprintf(fp, "%cDisplacement", *sep);
- else
- ret += fprintf(fp, " Displ");
- }
- }
+ fprintf(fp, " ");
+ width = hists__col_len(hists, col);
+
+ if (width == 0)
+ width = strlen(header);
+
+ for (i = 0; i < width; i++)
+ fprintf(fp, ".");
+}
+
+static int hists__fprintf_header(struct hists *hists, FILE *fp,
+ int max_rows, int max_cols __used)
+{
+ struct sort_entry *se;
+ struct hists_stdio_column *col;
+ int nr_rows = 0;
+
+ /*
+ * TODO Both fprintf_header and fprintf_dots should take
+ * max_cols and output data appropriatelly. Seems there was
+ * no need so far ;)
+ */
+
+ /* First line - headers */
+ fprintf(fp, "# ");
+
+ list_for_each_entry(col, &hists_stdio_column__list, list)
+ fprintf_header(fp, hists, col->header, col->col_idx);
list_for_each_entry(se, &hist_entry__sort_list, list) {
if (se->elide)
continue;
- if (sep) {
- fprintf(fp, "%c%s", *sep, se->se_header);
- continue;
- }
- width = strlen(se->se_header);
- if (symbol_conf.col_width_list_str) {
- if (col_width) {
- hists__set_col_len(hists, se->se_width_idx,
- atoi(col_width));
- col_width = strchr(col_width, ',');
- if (col_width)
- ++col_width;
- }
- }
- if (!hists__new_col_len(hists, se->se_width_idx, width))
- width = hists__col_len(hists, se->se_width_idx);
- fprintf(fp, " %*s", width, se->se_header);
+
+ fprintf_header(fp, hists, se->se_header, se->se_width_idx);
}
fprintf(fp, "\n");
+
if (max_rows && ++nr_rows >= max_rows)
- goto out;
+ return nr_rows;
- if (sep)
- goto print_entries;
+ if (symbol_conf.field_sep)
+ return nr_rows;
- fprintf(fp, "# ........");
- if (symbol_conf.show_cpu_utilization)
- fprintf(fp, " ....... .......");
- if (symbol_conf.show_nr_samples)
- fprintf(fp, " ..........");
- if (symbol_conf.show_total_period)
- fprintf(fp, " ............");
- if (pair) {
- fprintf(fp, " ..........");
- if (show_displacement)
- fprintf(fp, " .....");
- }
- list_for_each_entry(se, &hist_entry__sort_list, list) {
- unsigned int i;
+ /* Second line - dots */
+ fprintf(fp, "# ");
+
+ list_for_each_entry(col, &hists_stdio_column__list, list)
+ fprintf_dots(fp, hists, col->header, col->col_idx);
+ list_for_each_entry(se, &hist_entry__sort_list, list) {
if (se->elide)
continue;
- fprintf(fp, " ");
- width = hists__col_len(hists, se->se_width_idx);
- if (width == 0)
- width = strlen(se->se_header);
- for (i = 0; i < width; i++)
- fprintf(fp, ".");
+ fprintf_dots(fp, hists, se->se_header, se->se_width_idx);
}
fprintf(fp, "\n");
if (max_rows && ++nr_rows >= max_rows)
- goto out;
+ return nr_rows;
fprintf(fp, "#\n");
- if (max_rows && ++nr_rows >= max_rows)
- goto out;
+ if (max_rows)
+ ++nr_rows;
-print_entries:
- total_period = hists->stats.total_period;
+ return nr_rows;
+}
+
+static size_t
+hists__fprint_data(struct hists *hists, FILE *fp, int nr_rows,
+ int max_rows, int max_cols)
+{
+ struct rb_node *nd;
+ size_t ret = 0;
+
+ init_rem_hits();
for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
@@ -603,18 +656,12 @@ print_entries:
if (h->filtered)
continue;
- if (show_displacement) {
- if (h->pair != NULL)
- displacement = ((long)h->pair->position -
- (long)h->position);
- else
- displacement = 0;
- }
- ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
- displacement, total_period, fp);
+ ret += hist_entry__fprintf(h, max_cols, hists,
+ hists->stats.total_period,
+ fp);
if (max_rows && ++nr_rows >= max_rows)
- goto out;
+ break;
if (h->ms.map == NULL && verbose > 1) {
__map_groups__fprintf_maps(&h->thread->mg,
@@ -622,12 +669,25 @@ print_entries:
fprintf(fp, "%.10s end\n", graph_dotted_line);
}
}
-out:
- free(rem_sq_bracket);
+ free(rem_sq_bracket);
return ret;
}
+size_t hists__fprintf(struct hists *hists, bool show_header,
+ int max_rows, int max_cols, FILE *fp)
+{
+ int nr_rows = 0;
+
+ if (show_header)
+ nr_rows = hists__fprintf_header(hists, fp, max_rows, max_cols);
+
+ if (max_rows && nr_rows >= max_rows)
+ return 0;
+
+ return hists__fprint_data(hists, fp, nr_rows, max_rows, max_cols);
+}
+
size_t hists__fprintf_nr_events(struct hists *hists, FILE *fp)
{
int i;
diff --git a/tools/perf/ui/stdio/hist.h b/tools/perf/ui/stdio/hist.h
new file mode 100644
index 0000000..8e15d88
--- /dev/null
+++ b/tools/perf/ui/stdio/hist.h
@@ -0,0 +1,19 @@
+#ifndef __PERF_UI_STDIO_HIST_H
+#define __PERF_UI_STDIO_HIST_H
+
+#include <linux/list.h>
+#include "../../util/hist.h"
+
+struct hists_stdio_column {
+ struct list_head list;
+ const char *header;
+ int col_idx;
+ int col_width;
+ int (*snprintf)(struct hist_entry *self, char *bf,
+ size_t size, unsigned int width);
+};
+
+int hists_stdio_column__register_idx(int idx);
+void hists_stdio_column__register_global(void);
+void hists_stdio_column__set_width(struct hists *hists);
+#endif /* __PERF_UI_STDIO_HIST_H */
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 0ed0683..5fb5cd8 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -5,6 +5,7 @@
#include "session.h"
#include "sort.h"
#include <math.h>
+#include "../ui/stdio/hist.h"
static bool hists__filter_entry_by_dso(struct hists *hists,
struct hist_entry *he);
@@ -49,7 +50,9 @@ void hists__reset_col_len(struct hists *hists)
{
enum hist_column col;
- for (col = 0; col < HISTC_NR_COLS; ++col)
+ hists_stdio_column__set_width(hists);
+
+ for (col = HISTC_SYMBOL; col < HISTC_NR_COLS; ++col)
hists__set_col_len(hists, col, 0);
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2e650ff..36ff4c5 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -36,6 +36,19 @@ struct events_stats {
};
enum hist_column {
+ /* stdio (hists_stdio_column__list) */
+ HISTC_OVERHEAD,
+ HISTC_BASELINE,
+ HISTC_CPU_UTILIZATION_SYS,
+ HISTC_CPU_UTILIZATION_US,
+ HISTC_CPU_UTILIZATION_GUEST_SYS,
+ HISTC_CPU_UTILIZATION_GUEST_US,
+ HISTC_NR_SAMPLES,
+ HISTC_TOTAL_PERIOD,
+ HISTC_DELTA,
+ HISTC_DISPLACEMENT,
+
+ /* sorted (hist_entry__sort_list) */
HISTC_SYMBOL,
HISTC_DSO,
HISTC_THREAD,
@@ -49,6 +62,9 @@ enum hist_column {
HISTC_DSO_TO,
HISTC_SRCLINE,
HISTC_NR_COLS, /* Last entry */
+
+ /* Last stdio ui data column */
+ HISTC_STDIO_NR_COLS = HISTC_DISPLACEMENT + 1,
};
struct thread;
@@ -98,8 +114,7 @@ void hists__output_recalc_col_len(struct hists *hists, int max_rows);
void hists__inc_nr_events(struct hists *self, u32 type);
size_t hists__fprintf_nr_events(struct hists *self, FILE *fp);
-size_t hists__fprintf(struct hists *self, struct hists *pair,
- bool show_displacement, bool show_header,
+size_t hists__fprintf(struct hists *self, bool show_header,
int max_rows, int max_cols, FILE *fp);
int hist_entry__inc_addr_samples(struct hist_entry *self, int evidx, u64 addr);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 05/12] perf diff: Refactor stdio ui data columns output
2012-09-06 15:46 ` [PATCH 05/12] perf diff: Refactor stdio ui data columns output Jiri Olsa
@ 2012-09-07 2:55 ` Namhyung Kim
2012-09-07 9:20 ` Jiri Olsa
2012-09-08 12:35 ` Jiri Olsa
0 siblings, 2 replies; 40+ messages in thread
From: Namhyung Kim @ 2012-09-07 2:55 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
Hi, Jiri
On Thu, 6 Sep 2012 17:46:59 +0200, Jiri Olsa wrote:
> Currently for any of the data columns (like Overhead/Period..) in
> stdio ui, there's separate code to print header/dots/value scattered
> along the display code path.
>
> Adding hists_stdio_column struct to centralize all info needed
> to print column header/dots/value.
>
> This change eases up addition for new columns, which is now mostly
> matter only of adding new hists_stdio_column struct.
As you may know, I submitted a similar patchset few days ago for the
same reason and it handles TUI/GTK cases as well. I'm waiting for
reviews.
Thanks,
Namhyung
>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
> tools/perf/builtin-diff.c | 14 +-
> tools/perf/builtin-report.c | 6 +-
> tools/perf/builtin-top.c | 6 +-
> tools/perf/ui/stdio/hist.c | 506 +++++++++++++++++++++++++-------------------
> tools/perf/ui/stdio/hist.h | 19 ++
> tools/perf/util/hist.c | 5 +-
> tools/perf/util/hist.h | 19 +-
> 7 files changed, 345 insertions(+), 230 deletions(-)
> create mode 100644 tools/perf/ui/stdio/hist.h
>
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index 4e63979..282fd5e 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -16,6 +16,7 @@
> #include "util/sort.h"
> #include "util/symbol.h"
> #include "util/util.h"
> +#include "ui/stdio/hist.h"
>
> #include <stdlib.h>
>
> @@ -214,8 +215,7 @@ static int __cmd_diff(void)
> first = false;
>
> hists__match(&evsel_old->hists, &evsel->hists);
> - hists__fprintf(&evsel->hists, &evsel_old->hists,
> - show_displacement, true, 0, 0, stdout);
> + hists__fprintf(&evsel->hists, true, 0, 0, stdout);
> }
>
> out_delete:
> @@ -257,6 +257,15 @@ static const struct option options[] = {
> OPT_END()
> };
>
> +static void setup_ui_stdio(void)
> +{
> + hists_stdio_column__register_idx(HISTC_BASELINE);
> + hists_stdio_column__register_global();
> + hists_stdio_column__register_idx(HISTC_DELTA);
> + if (show_displacement)
> + hists_stdio_column__register_idx(HISTC_DISPLACEMENT);
> +}
> +
> int cmd_diff(int argc, const char **argv, const char *prefix __used)
> {
> sort_order = diff__default_sort_order;
> @@ -280,6 +289,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix __used)
> return -1;
>
> setup_sorting(diff_usage, options);
> + setup_ui_stdio();
> setup_pager();
>
> sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", NULL);
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index d618253..c855c9a 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -33,6 +33,7 @@
> #include "util/thread.h"
> #include "util/sort.h"
> #include "util/hist.h"
> +#include "ui/stdio/hist.h"
>
> #include <linux/bitmap.h>
>
> @@ -315,12 +316,15 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
> {
> struct perf_evsel *pos;
>
> + hists_stdio_column__register_idx(HISTC_OVERHEAD);
> + hists_stdio_column__register_global();
> +
> list_for_each_entry(pos, &evlist->entries, node) {
> struct hists *hists = &pos->hists;
> const char *evname = perf_evsel__name(pos);
>
> hists__fprintf_nr_sample_events(hists, evname, stdout);
> - hists__fprintf(hists, NULL, false, true, 0, 0, stdout);
> + hists__fprintf(hists, true, 0, 0, stdout);
> fprintf(stdout, "\n\n");
> }
>
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 0513aaa..26da442 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -41,6 +41,7 @@
> #include "util/intlist.h"
>
> #include "util/debug.h"
> +#include "ui/stdio/hist.h"
>
> #include <assert.h>
> #include <elf.h>
> @@ -315,7 +316,7 @@ static void perf_top__print_sym_table(struct perf_top *top)
> hists__output_recalc_col_len(&top->sym_evsel->hists,
> top->winsize.ws_row - 3);
> putchar('\n');
> - hists__fprintf(&top->sym_evsel->hists, NULL, false, false,
> + hists__fprintf(&top->sym_evsel->hists, false,
> top->winsize.ws_row - 4 - printed, win_width, stdout);
> }
>
> @@ -607,6 +608,9 @@ static void *display_thread(void *arg)
> struct perf_top *top = arg;
> int delay_msecs, c;
>
> + hists_stdio_column__register_idx(HISTC_OVERHEAD);
> + hists_stdio_column__register_global();
> +
> tcgetattr(0, &save);
> tc = save;
> tc.c_lflag &= ~(ICANON | ECHO);
> diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
> index 4ae9ee5..48fb0b3 100644
> --- a/tools/perf/ui/stdio/hist.c
> +++ b/tools/perf/ui/stdio/hist.c
> @@ -1,10 +1,180 @@
> #include <stdio.h>
> #include <math.h>
>
> +#include "hist.h"
> #include "../../util/util.h"
> #include "../../util/hist.h"
> #include "../../util/sort.h"
>
> +static double get_period_percent(struct hist_entry *he, u64 period)
> +{
> + u64 total = he->hists->stats.total_period;
> + return (period * 100.0) / total;
> +}
> +
> +static int
> +hists_stdio_column__delta_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + struct hist_entry *pair = he->pair;
> + double new_percent = get_period_percent(he, he->period);
> + double old_percent = pair ? get_period_percent(pair, pair->period) : 0.0;
> + double diff = new_percent - old_percent;
> + int ret;
> +
> + if (fabs(diff) >= 0.01)
> + ret = percent_color_snprintf(bf, size, "%+7.2F%%", diff);
> + else
> + ret = scnprintf(bf, size, "%8s", " ");
> +
> + return ret;
> +}
> +
> +static int
> +hists_stdio_column__baseline_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + struct hist_entry *pair = he->pair;
> + double percent = pair ? get_period_percent(pair, pair->period) : 0.0;
> +
> + return percent_color_snprintf(bf, size, "%7.2F%%", percent);
> +}
> +
> +static int
> +hists_stdio_column__overhead_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + double percent = get_period_percent(he, he->period);
> + return percent_color_snprintf(bf, size, "%7.2f%%", percent);
> +}
> +
> +static int
> +hists_stdio_column__nr_samples_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + return scnprintf(bf, size, "%12" PRIu64, he->nr_events);
> +}
> +
> +static int
> +hists_stdio_column__total_period_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + return scnprintf(bf, size , "%12" PRIu64, he->period);
> +}
> +
> +static int
> +hists_stdio_column__cpu_sys_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + double percent = get_period_percent(he, he->period_sys);
> + return percent_color_snprintf(bf, size, "%7.2f%%", percent);
> +}
> +
> +static int
> +hists_stdio_column__cpu_us_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + double percent = get_period_percent(he, he->period_us);
> + return percent_color_snprintf(bf, size, "%7.2f%%", percent);
> +}
> +
> +static int
> +hists_stdio_column__cpu_guest_sys_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + double percent = get_period_percent(he, he->period_guest_sys);
> + return percent_color_snprintf(bf, size, "%8.2f%%", percent);
> +}
> +
> +static int
> +hists_stdio_column__cpu_guest_us_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + double percent = get_period_percent(he, he->period_guest_us);
> + return percent_color_snprintf(bf, size, "%7.2f%%", percent);
> +}
> +
> +static int
> +hists_stdio_column__displacement_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + struct hist_entry *pair = he->pair;
> + unsigned long displ = pair ? pair->position - he->position : 0;
> + return scnprintf(bf, size, displ ? "%+5ld" : " ", displ);
> +}
> +
> +LIST_HEAD(hists_stdio_column__list);
> +
> +#define DEF_COLUMN(name, c, w, h) \
> +[ c ] = { \
> + .list = LIST_HEAD_INIT((hists_stdio_column__array[ c ]).list),\
> + .header = h, \
> + .snprintf = hists_stdio_column__ ## name ## _snprintf, \
> + .col_idx = c, \
> + .col_width = w, \
> +},
> +
> +struct hists_stdio_column hists_stdio_column__array[HISTC_STDIO_NR_COLS] = {
> +DEF_COLUMN(overhead, HISTC_OVERHEAD, 8, "Overhead")
> +DEF_COLUMN(baseline, HISTC_BASELINE, 8, "Baseline")
> +DEF_COLUMN(cpu_sys, HISTC_CPU_UTILIZATION_SYS, 8, "sys")
> +DEF_COLUMN(cpu_us, HISTC_CPU_UTILIZATION_US, 8, "us")
> +DEF_COLUMN(cpu_guest_sys, HISTC_CPU_UTILIZATION_GUEST_SYS, 8, "guest sys")
> +DEF_COLUMN(cpu_guest_us, HISTC_CPU_UTILIZATION_GUEST_US, 8, "guest us")
> +DEF_COLUMN(nr_samples, HISTC_NR_SAMPLES, 12, "Samples")
> +DEF_COLUMN(total_period, HISTC_TOTAL_PERIOD, 12, "Period")
> +DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
> +DEF_COLUMN(displacement, HISTC_DISPLACEMENT, 5, "Displ")
> +};
> +
> +int hists_stdio_column__register_idx(int idx)
> +{
> + struct hists_stdio_column *col;
> +
> + if (idx >= HISTC_STDIO_NR_COLS)
> + return -EINVAL;
> +
> + col = &hists_stdio_column__array[idx];
> +
> + if (!list_empty(&col->list))
> + return -EBUSY;
> +
> + list_add_tail(&col->list, &hists_stdio_column__list);
> + return 0;
> +}
> +
> +void hists_stdio_column__set_width(struct hists *hists)
> +{
> + struct hists_stdio_column *col;
> + unsigned i;
> +
> + for (i = 0; i < HISTC_STDIO_NR_COLS; i++) {
> + col = &hists_stdio_column__array[i];
> + hists__set_col_len(hists, i, col->col_width);
> + }
> +}
> +
> +void hists_stdio_column__register_global(void)
> +{
> +#define reg(idx) hists_stdio_column__register_idx(idx)
> +
> + if (symbol_conf.show_cpu_utilization) {
> + reg(HISTC_CPU_UTILIZATION_SYS);
> + reg(HISTC_CPU_UTILIZATION_US);
> + }
> +
> + if (perf_guest) {
> + reg(HISTC_CPU_UTILIZATION_GUEST_SYS);
> + reg(HISTC_CPU_UTILIZATION_GUEST_US);
> + }
> +
> + if (symbol_conf.show_nr_samples)
> + reg(HISTC_NR_SAMPLES);
> +
> + if (symbol_conf.show_total_period)
> + reg(HISTC_TOTAL_PERIOD);
> +#undef reg
> +}
>
> static size_t callchain__fprintf_left_margin(FILE *fp, int left_margin)
> {
> @@ -291,114 +461,20 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
> return 0;
> }
>
> -static int hist_entry__period_snprintf(struct hist_entry *he, char *s,
> - size_t size, struct hists *pair_hists,
> - bool show_displacement, long displacement,
> - bool color, u64 total_period)
> +static int
> +hists_stdio_column__snprintf(struct hist_entry *he, char *s, size_t size,
> + struct hists *hists)
> {
> - u64 period, total, period_sys, period_us, period_guest_sys, period_guest_us;
> - u64 nr_events;
> const char *sep = symbol_conf.field_sep;
> - int ret;
> -
> - if (symbol_conf.exclude_other && !he->parent)
> - return 0;
> -
> - if (pair_hists) {
> - period = he->pair ? he->pair->period : 0;
> - nr_events = he->pair ? he->pair->nr_events : 0;
> - total = pair_hists->stats.total_period;
> - period_sys = he->pair ? he->pair->period_sys : 0;
> - period_us = he->pair ? he->pair->period_us : 0;
> - period_guest_sys = he->pair ? he->pair->period_guest_sys : 0;
> - period_guest_us = he->pair ? he->pair->period_guest_us : 0;
> - } else {
> - period = he->period;
> - nr_events = he->nr_events;
> - total = total_period;
> - period_sys = he->period_sys;
> - period_us = he->period_us;
> - period_guest_sys = he->period_guest_sys;
> - period_guest_us = he->period_guest_us;
> - }
> -
> - if (total) {
> - if (color)
> - ret = percent_color_snprintf(s, size,
> - sep ? "%.2f" : " %6.2f%%",
> - (period * 100.0) / total);
> - else
> - ret = scnprintf(s, size, sep ? "%.2f" : " %6.2f%%",
> - (period * 100.0) / total);
> - if (symbol_conf.show_cpu_utilization) {
> - ret += percent_color_snprintf(s + ret, size - ret,
> - sep ? "%.2f" : " %6.2f%%",
> - (period_sys * 100.0) / total);
> - ret += percent_color_snprintf(s + ret, size - ret,
> - sep ? "%.2f" : " %6.2f%%",
> - (period_us * 100.0) / total);
> - if (perf_guest) {
> - ret += percent_color_snprintf(s + ret,
> - size - ret,
> - sep ? "%.2f" : " %6.2f%%",
> - (period_guest_sys * 100.0) /
> - total);
> - ret += percent_color_snprintf(s + ret,
> - size - ret,
> - sep ? "%.2f" : " %6.2f%%",
> - (period_guest_us * 100.0) /
> - total);
> - }
> - }
> - } else
> - ret = scnprintf(s, size, sep ? "%" PRIu64 : "%12" PRIu64 " ", period);
> -
> - if (symbol_conf.show_nr_samples) {
> - if (sep)
> - ret += scnprintf(s + ret, size - ret, "%c%" PRIu64, *sep, nr_events);
> - else
> - ret += scnprintf(s + ret, size - ret, "%11" PRIu64, nr_events);
> - }
> -
> - if (symbol_conf.show_total_period) {
> - if (sep)
> - ret += scnprintf(s + ret, size - ret, "%c%" PRIu64, *sep, period);
> - else
> - ret += scnprintf(s + ret, size - ret, " %12" PRIu64, period);
> - }
> -
> - if (pair_hists) {
> - char bf[32];
> - double old_percent = 0, new_percent = 0, diff;
> -
> - if (total > 0)
> - old_percent = (period * 100.0) / total;
> - if (total_period > 0)
> - new_percent = (he->period * 100.0) / total_period;
> -
> - diff = new_percent - old_percent;
> -
> - if (fabs(diff) >= 0.01)
> - scnprintf(bf, sizeof(bf), "%+4.2F%%", diff);
> - else
> - scnprintf(bf, sizeof(bf), " ");
> -
> - if (sep)
> - ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
> - else
> - ret += scnprintf(s + ret, size - ret, "%11.11s", bf);
> + struct hists_stdio_column *col;
> + int ret = 0;
>
> - if (show_displacement) {
> - if (displacement)
> - scnprintf(bf, sizeof(bf), "%+4ld", displacement);
> - else
> - scnprintf(bf, sizeof(bf), " ");
> + ret = snprintf(s, size, " ");
>
> - if (sep)
> - ret += scnprintf(s + ret, size - ret, "%c%s", *sep, bf);
> - else
> - ret += scnprintf(s + ret, size - ret, "%6.6s", bf);
> - }
> + list_for_each_entry(col, &hists_stdio_column__list, list) {
> + ret += scnprintf(s + ret, size - ret, "%s", sep ?: " ");
> + ret += col->snprintf(he, s + ret, size - ret,
> + hists__col_len(hists, col->col_idx));
> }
>
> return ret;
> @@ -440,9 +516,8 @@ static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
> }
>
> static int hist_entry__fprintf(struct hist_entry *he, size_t size,
> - struct hists *hists, struct hists *pair_hists,
> - bool show_displacement, long displacement,
> - u64 total_period, FILE *fp)
> + struct hists *hists, u64 total_period,
> + FILE *fp)
> {
> char bf[512];
> int ret;
> @@ -450,9 +525,8 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
> if (size == 0 || size > sizeof(bf))
> size = sizeof(bf);
>
> - ret = hist_entry__period_snprintf(he, bf, size, pair_hists,
> - show_displacement, displacement,
> - true, total_period);
> + ret = hists_stdio_column__snprintf(he, bf, size, hists);
> +
> hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
>
> ret = fprintf(fp, "%s\n", bf);
> @@ -464,138 +538,117 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
> return ret;
> }
>
> -size_t hists__fprintf(struct hists *hists, struct hists *pair,
> - bool show_displacement, bool show_header, int max_rows,
> - int max_cols, FILE *fp)
> +static void fprintf_header(FILE *fp, struct hists *hists,
> + const char *header, int col)
> {
> - struct sort_entry *se;
> - struct rb_node *nd;
> - size_t ret = 0;
> - u64 total_period;
> - long displacement = 0;
> - unsigned int width;
> - const char *sep = symbol_conf.field_sep;
> const char *col_width = symbol_conf.col_width_list_str;
> - int nr_rows = 0;
> -
> - init_rem_hits();
> -
> - if (!show_header)
> - goto print_entries;
> + const char *sep = symbol_conf.field_sep;
> + unsigned int width;
>
> - fprintf(fp, "# %s", pair ? "Baseline" : "Overhead");
> + if (sep) {
> + fprintf(fp, "%c%s", *sep, header);
> + return;
> + }
>
> - if (symbol_conf.show_cpu_utilization) {
> - if (sep) {
> - ret += fprintf(fp, "%csys", *sep);
> - ret += fprintf(fp, "%cus", *sep);
> - if (perf_guest) {
> - ret += fprintf(fp, "%cguest sys", *sep);
> - ret += fprintf(fp, "%cguest us", *sep);
> - }
> - } else {
> - ret += fprintf(fp, " sys ");
> - ret += fprintf(fp, " us ");
> - if (perf_guest) {
> - ret += fprintf(fp, " guest sys ");
> - ret += fprintf(fp, " guest us ");
> - }
> + width = strlen(header);
> + if (symbol_conf.col_width_list_str) {
> + if (col_width) {
> + hists__set_col_len(hists, col,
> + atoi(col_width));
> + col_width = strchr(col_width, ',');
> + if (col_width)
> + ++col_width;
> }
> }
>
> - if (symbol_conf.show_nr_samples) {
> - if (sep)
> - fprintf(fp, "%cSamples", *sep);
> - else
> - fputs(" Samples ", fp);
> - }
> + if (!hists__new_col_len(hists, col, width))
> + width = hists__col_len(hists, col);
>
> - if (symbol_conf.show_total_period) {
> - if (sep)
> - ret += fprintf(fp, "%cPeriod", *sep);
> - else
> - ret += fprintf(fp, " Period ");
> - }
> + fprintf(fp, " %*s", width, header);
> +}
>
> - if (pair) {
> - if (sep)
> - ret += fprintf(fp, "%cDelta", *sep);
> - else
> - ret += fprintf(fp, " Delta ");
> +static void fprintf_dots(FILE *fp, struct hists *hists,
> + const char *header, int col)
> +{
> + unsigned int width;
> + unsigned int i;
>
> - if (show_displacement) {
> - if (sep)
> - ret += fprintf(fp, "%cDisplacement", *sep);
> - else
> - ret += fprintf(fp, " Displ");
> - }
> - }
> + fprintf(fp, " ");
> + width = hists__col_len(hists, col);
> +
> + if (width == 0)
> + width = strlen(header);
> +
> + for (i = 0; i < width; i++)
> + fprintf(fp, ".");
> +}
> +
> +static int hists__fprintf_header(struct hists *hists, FILE *fp,
> + int max_rows, int max_cols __used)
> +{
> + struct sort_entry *se;
> + struct hists_stdio_column *col;
> + int nr_rows = 0;
> +
> + /*
> + * TODO Both fprintf_header and fprintf_dots should take
> + * max_cols and output data appropriatelly. Seems there was
> + * no need so far ;)
> + */
> +
> + /* First line - headers */
> + fprintf(fp, "# ");
> +
> + list_for_each_entry(col, &hists_stdio_column__list, list)
> + fprintf_header(fp, hists, col->header, col->col_idx);
>
> list_for_each_entry(se, &hist_entry__sort_list, list) {
> if (se->elide)
> continue;
> - if (sep) {
> - fprintf(fp, "%c%s", *sep, se->se_header);
> - continue;
> - }
> - width = strlen(se->se_header);
> - if (symbol_conf.col_width_list_str) {
> - if (col_width) {
> - hists__set_col_len(hists, se->se_width_idx,
> - atoi(col_width));
> - col_width = strchr(col_width, ',');
> - if (col_width)
> - ++col_width;
> - }
> - }
> - if (!hists__new_col_len(hists, se->se_width_idx, width))
> - width = hists__col_len(hists, se->se_width_idx);
> - fprintf(fp, " %*s", width, se->se_header);
> +
> + fprintf_header(fp, hists, se->se_header, se->se_width_idx);
> }
>
> fprintf(fp, "\n");
> +
> if (max_rows && ++nr_rows >= max_rows)
> - goto out;
> + return nr_rows;
>
> - if (sep)
> - goto print_entries;
> + if (symbol_conf.field_sep)
> + return nr_rows;
>
> - fprintf(fp, "# ........");
> - if (symbol_conf.show_cpu_utilization)
> - fprintf(fp, " ....... .......");
> - if (symbol_conf.show_nr_samples)
> - fprintf(fp, " ..........");
> - if (symbol_conf.show_total_period)
> - fprintf(fp, " ............");
> - if (pair) {
> - fprintf(fp, " ..........");
> - if (show_displacement)
> - fprintf(fp, " .....");
> - }
> - list_for_each_entry(se, &hist_entry__sort_list, list) {
> - unsigned int i;
> + /* Second line - dots */
> + fprintf(fp, "# ");
> +
> + list_for_each_entry(col, &hists_stdio_column__list, list)
> + fprintf_dots(fp, hists, col->header, col->col_idx);
>
> + list_for_each_entry(se, &hist_entry__sort_list, list) {
> if (se->elide)
> continue;
>
> - fprintf(fp, " ");
> - width = hists__col_len(hists, se->se_width_idx);
> - if (width == 0)
> - width = strlen(se->se_header);
> - for (i = 0; i < width; i++)
> - fprintf(fp, ".");
> + fprintf_dots(fp, hists, se->se_header, se->se_width_idx);
> }
>
> fprintf(fp, "\n");
> if (max_rows && ++nr_rows >= max_rows)
> - goto out;
> + return nr_rows;
>
> fprintf(fp, "#\n");
> - if (max_rows && ++nr_rows >= max_rows)
> - goto out;
> + if (max_rows)
> + ++nr_rows;
>
> -print_entries:
> - total_period = hists->stats.total_period;
> + return nr_rows;
> +}
> +
> +static size_t
> +hists__fprint_data(struct hists *hists, FILE *fp, int nr_rows,
> + int max_rows, int max_cols)
> +{
> + struct rb_node *nd;
> + size_t ret = 0;
> +
> + init_rem_hits();
>
> for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
> struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
> @@ -603,18 +656,12 @@ print_entries:
> if (h->filtered)
> continue;
>
> - if (show_displacement) {
> - if (h->pair != NULL)
> - displacement = ((long)h->pair->position -
> - (long)h->position);
> - else
> - displacement = 0;
> - }
> - ret += hist_entry__fprintf(h, max_cols, hists, pair, show_displacement,
> - displacement, total_period, fp);
> + ret += hist_entry__fprintf(h, max_cols, hists,
> + hists->stats.total_period,
> + fp);
>
> if (max_rows && ++nr_rows >= max_rows)
> - goto out;
> + break;
>
> if (h->ms.map == NULL && verbose > 1) {
> __map_groups__fprintf_maps(&h->thread->mg,
> @@ -622,12 +669,25 @@ print_entries:
> fprintf(fp, "%.10s end\n", graph_dotted_line);
> }
> }
> -out:
> - free(rem_sq_bracket);
>
> + free(rem_sq_bracket);
> return ret;
> }
>
> +size_t hists__fprintf(struct hists *hists, bool show_header,
> + int max_rows, int max_cols, FILE *fp)
> +{
> + int nr_rows = 0;
> +
> + if (show_header)
> + nr_rows = hists__fprintf_header(hists, fp, max_rows, max_cols);
> +
> + if (max_rows && nr_rows >= max_rows)
> + return 0;
> +
> + return hists__fprint_data(hists, fp, nr_rows, max_rows, max_cols);
> +}
> +
> size_t hists__fprintf_nr_events(struct hists *hists, FILE *fp)
> {
> int i;
> diff --git a/tools/perf/ui/stdio/hist.h b/tools/perf/ui/stdio/hist.h
> new file mode 100644
> index 0000000..8e15d88
> --- /dev/null
> +++ b/tools/perf/ui/stdio/hist.h
> @@ -0,0 +1,19 @@
> +#ifndef __PERF_UI_STDIO_HIST_H
> +#define __PERF_UI_STDIO_HIST_H
> +
> +#include <linux/list.h>
> +#include "../../util/hist.h"
> +
> +struct hists_stdio_column {
> + struct list_head list;
> + const char *header;
> + int col_idx;
> + int col_width;
> + int (*snprintf)(struct hist_entry *self, char *bf,
> + size_t size, unsigned int width);
> +};
> +
> +int hists_stdio_column__register_idx(int idx);
> +void hists_stdio_column__register_global(void);
> +void hists_stdio_column__set_width(struct hists *hists);
> +#endif /* __PERF_UI_STDIO_HIST_H */
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 0ed0683..5fb5cd8 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -5,6 +5,7 @@
> #include "session.h"
> #include "sort.h"
> #include <math.h>
> +#include "../ui/stdio/hist.h"
>
> static bool hists__filter_entry_by_dso(struct hists *hists,
> struct hist_entry *he);
> @@ -49,7 +50,9 @@ void hists__reset_col_len(struct hists *hists)
> {
> enum hist_column col;
>
> - for (col = 0; col < HISTC_NR_COLS; ++col)
> + hists_stdio_column__set_width(hists);
> +
> + for (col = HISTC_SYMBOL; col < HISTC_NR_COLS; ++col)
> hists__set_col_len(hists, col, 0);
> }
>
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 2e650ff..36ff4c5 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -36,6 +36,19 @@ struct events_stats {
> };
>
> enum hist_column {
> + /* stdio (hists_stdio_column__list) */
> + HISTC_OVERHEAD,
> + HISTC_BASELINE,
> + HISTC_CPU_UTILIZATION_SYS,
> + HISTC_CPU_UTILIZATION_US,
> + HISTC_CPU_UTILIZATION_GUEST_SYS,
> + HISTC_CPU_UTILIZATION_GUEST_US,
> + HISTC_NR_SAMPLES,
> + HISTC_TOTAL_PERIOD,
> + HISTC_DELTA,
> + HISTC_DISPLACEMENT,
> +
> + /* sorted (hist_entry__sort_list) */
> HISTC_SYMBOL,
> HISTC_DSO,
> HISTC_THREAD,
> @@ -49,6 +62,9 @@ enum hist_column {
> HISTC_DSO_TO,
> HISTC_SRCLINE,
> HISTC_NR_COLS, /* Last entry */
> +
> + /* Last stdio ui data column */
> + HISTC_STDIO_NR_COLS = HISTC_DISPLACEMENT + 1,
> };
>
> struct thread;
> @@ -98,8 +114,7 @@ void hists__output_recalc_col_len(struct hists *hists, int max_rows);
> void hists__inc_nr_events(struct hists *self, u32 type);
> size_t hists__fprintf_nr_events(struct hists *self, FILE *fp);
>
> -size_t hists__fprintf(struct hists *self, struct hists *pair,
> - bool show_displacement, bool show_header,
> +size_t hists__fprintf(struct hists *self, bool show_header,
> int max_rows, int max_cols, FILE *fp);
>
> int hist_entry__inc_addr_samples(struct hist_entry *self, int evidx, u64 addr);
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 05/12] perf diff: Refactor stdio ui data columns output
2012-09-07 2:55 ` Namhyung Kim
@ 2012-09-07 9:20 ` Jiri Olsa
2012-09-08 12:35 ` Jiri Olsa
1 sibling, 0 replies; 40+ messages in thread
From: Jiri Olsa @ 2012-09-07 9:20 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
On Fri, Sep 07, 2012 at 11:55:02AM +0900, Namhyung Kim wrote:
> Hi, Jiri
>
> On Thu, 6 Sep 2012 17:46:59 +0200, Jiri Olsa wrote:
> > Currently for any of the data columns (like Overhead/Period..) in
> > stdio ui, there's separate code to print header/dots/value scattered
> > along the display code path.
> >
> > Adding hists_stdio_column struct to centralize all info needed
> > to print column header/dots/value.
> >
> > This change eases up addition for new columns, which is now mostly
> > matter only of adding new hists_stdio_column struct.
>
> As you may know, I submitted a similar patchset few days ago for the
> same reason and it handles TUI/GTK cases as well. I'm waiting for
> reviews.
I must have missed it.. I'll check ;)
jirka
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 05/12] perf diff: Refactor stdio ui data columns output
2012-09-07 2:55 ` Namhyung Kim
2012-09-07 9:20 ` Jiri Olsa
@ 2012-09-08 12:35 ` Jiri Olsa
2012-09-08 12:50 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 40+ messages in thread
From: Jiri Olsa @ 2012-09-08 12:35 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
David Ahern
On Fri, Sep 07, 2012 at 11:55:02AM +0900, Namhyung Kim wrote:
> Hi, Jiri
>
> On Thu, 6 Sep 2012 17:46:59 +0200, Jiri Olsa wrote:
> > Currently for any of the data columns (like Overhead/Period..) in
> > stdio ui, there's separate code to print header/dots/value scattered
> > along the display code path.
> >
> > Adding hists_stdio_column struct to centralize all info needed
> > to print column header/dots/value.
> >
> > This change eases up addition for new columns, which is now mostly
> > matter only of adding new hists_stdio_column struct.
>
> As you may know, I submitted a similar patchset few days ago for the
> same reason and it handles TUI/GTK cases as well. I'm waiting for
> reviews.
ok, I'll rebase this to acme/tmp.perf/hpp
jirka
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 05/12] perf diff: Refactor stdio ui data columns output
2012-09-08 12:35 ` Jiri Olsa
@ 2012-09-08 12:50 ` Arnaldo Carvalho de Melo
2012-09-08 14:37 ` Namhyung Kim
0 siblings, 1 reply; 40+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-08 12:50 UTC (permalink / raw)
To: Jiri Olsa
Cc: Namhyung Kim, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, David Ahern
Em Sat, Sep 08, 2012 at 02:35:14PM +0200, Jiri Olsa escreveu:
> On Fri, Sep 07, 2012 at 11:55:02AM +0900, Namhyung Kim wrote:
> > Hi, Jiri
> >
> > On Thu, 6 Sep 2012 17:46:59 +0200, Jiri Olsa wrote:
> > > Currently for any of the data columns (like Overhead/Period..) in
> > > stdio ui, there's separate code to print header/dots/value scattered
> > > along the display code path.
> > >
> > > Adding hists_stdio_column struct to centralize all info needed
> > > to print column header/dots/value.
> > >
> > > This change eases up addition for new columns, which is now mostly
> > > matter only of adding new hists_stdio_column struct.
> >
> > As you may know, I submitted a similar patchset few days ago for the
> > same reason and it handles TUI/GTK cases as well. I'm waiting for
> > reviews.
>
> ok, I'll rebase this to acme/tmp.perf/hpp
well, you can, but that one is buggy and you will not be able to test
'perf diff' at all...
- Arnaldo
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 05/12] perf diff: Refactor stdio ui data columns output
2012-09-08 12:50 ` Arnaldo Carvalho de Melo
@ 2012-09-08 14:37 ` Namhyung Kim
2012-09-08 15:10 ` Arnaldo Carvalho de Melo
2012-09-08 15:12 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 40+ messages in thread
From: Namhyung Kim @ 2012-09-08 14:37 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, David Ahern
Hi Jiri,
On Sat, 8 Sep 2012 05:50:59 -0700, Arnaldo Carvalho de Melo wrote:
> Em Sat, Sep 08, 2012 at 02:35:14PM +0200, Jiri Olsa escreveu:
>> On Fri, Sep 07, 2012 at 11:55:02AM +0900, Namhyung Kim wrote:
>> > Hi, Jiri
>> >
>> > On Thu, 6 Sep 2012 17:46:59 +0200, Jiri Olsa wrote:
>> > > Currently for any of the data columns (like Overhead/Period..) in
>> > > stdio ui, there's separate code to print header/dots/value scattered
>> > > along the display code path.
>> > >
>> > > Adding hists_stdio_column struct to centralize all info needed
>> > > to print column header/dots/value.
>> > >
>> > > This change eases up addition for new columns, which is now mostly
>> > > matter only of adding new hists_stdio_column struct.
>> >
>> > As you may know, I submitted a similar patchset few days ago for the
>> > same reason and it handles TUI/GTK cases as well. I'm waiting for
>> > reviews.
>>
>> ok, I'll rebase this to acme/tmp.perf/hpp
>
> well, you can, but that one is buggy and you will not be able to test
> 'perf diff' at all...
I posted the fix right before, and you will also need patch below for
fixing broken "baseline" output.
>From 8f2d9010979e244c6565f3a9134e7bfa61936e38 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Sat, 8 Sep 2012 23:28:59 +0900
Subject: [PATCH] perf hist: Fix perf diff baseline output
When perf diff is running, the overhead field should print old hist
entry's period as a baseline.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/hist.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 16dc486d02be..326f4e9e6911 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -21,6 +21,18 @@ static int hpp__width_overhead(struct perf_hpp *hpp __used)
static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
{
double percent = 100.0 * he->period / hpp->total_period;
+
+ if (hpp->ptr) {
+ struct hists *old_hists = hpp->ptr;
+ u64 total_period = old_hists->stats.total_period;
+ u64 base_period = he->pair ? he->pair->period : 0;
+
+ if (total_period)
+ percent = 100.0 * base_period / total_period;
+ else
+ percent = 0.0;
+ }
+
return percent_color_snprintf(hpp->buf, hpp->size, " %5.2f%%", percent);
}
@@ -29,6 +41,17 @@ static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
double percent = 100.0 * he->period / hpp->total_period;
const char *fmt = symbol_conf.field_sep ? "%.2f" : " %5.2f%%";
+ if (hpp->ptr) {
+ struct hists *old_hists = hpp->ptr;
+ u64 total_period = old_hists->stats.total_period;
+ u64 base_period = he->pair ? he->pair->period : 0;
+
+ if (total_period)
+ percent = 100.0 * base_period / total_period;
+ else
+ percent = 0.0;
+ }
+
return scnprintf(hpp->buf, hpp->size, fmt, percent);
}
--
1.7.9.2
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 05/12] perf diff: Refactor stdio ui data columns output
2012-09-08 14:37 ` Namhyung Kim
@ 2012-09-08 15:10 ` Arnaldo Carvalho de Melo
2012-09-08 15:12 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 40+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-08 15:10 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, David Ahern
Em Sat, Sep 08, 2012 at 11:37:31PM +0900, Namhyung Kim escreveu:
> I posted the fix right before, and you will also need patch below for
> fixing broken "baseline" output.
Ok, folding this as well, as it should fix the problem I just reported,
sorry mid air collision report (crossed reports, bugzilla jargon :P).
- Arnaldo
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 05/12] perf diff: Refactor stdio ui data columns output
2012-09-08 14:37 ` Namhyung Kim
2012-09-08 15:10 ` Arnaldo Carvalho de Melo
@ 2012-09-08 15:12 ` Arnaldo Carvalho de Melo
2012-09-08 15:21 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 40+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-08 15:12 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, David Ahern
Em Sat, Sep 08, 2012 at 11:37:31PM +0900, Namhyung Kim escreveu:
> On Sat, 8 Sep 2012 05:50:59 -0700, Arnaldo Carvalho de Melo wrote:
> > well, you can, but that one is buggy and you will not be able to test
> > 'perf diff' at all...
>
> I posted the fix right before, and you will also need patch below for
> fixing broken "baseline" output.
>
Trying to figure out when to apply this patch, as it should fix the
problem one experiences right after applying the first patch of your
series plus the small fix you sent before this one, and it is not
applying...
- Arnaldo
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH 05/12] perf diff: Refactor stdio ui data columns output
2012-09-08 15:12 ` Arnaldo Carvalho de Melo
@ 2012-09-08 15:21 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 40+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-08 15:21 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, David Ahern
Em Sat, Sep 08, 2012 at 08:12:53AM -0700, Arnaldo Carvalho de Melo escreveu:
> Trying to figure out when to apply this patch, as it should fix the
> problem one experiences right after applying the first patch of your
> series plus the small fix you sent before this one, and it is not
> applying...
Ok, I think I figured it out, results now are the same and I'll proceed
to the next patches.
- Arnaldo
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 06/12] perf diff: Add -b option for perf diff to display paired entries only
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (4 preceding siblings ...)
2012-09-06 15:46 ` [PATCH 05/12] perf diff: Refactor stdio ui data columns output Jiri Olsa
@ 2012-09-06 15:47 ` Jiri Olsa
2012-09-06 15:47 ` [PATCH 07/12] perf diff: Add ratio computation way to compare hist entries Jiri Olsa
` (7 subsequent siblings)
13 siblings, 0 replies; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:47 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
Adding -b option to perf diff command to display only entries
with match in the baseline.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 4 ++++
tools/perf/builtin-diff.c | 31 +++++++++++++++++++++++++++++--
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index ab7f667..6ce9585 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -72,6 +72,10 @@ OPTIONS
--symfs=<directory>::
Look for files with symbols relative to this directory.
+-b::
+--baseline-only::
+ Show only items with match in baseline.
+
SEE ALSO
--------
linkperf:perf-record[1]
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 282fd5e..46f4a10 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -25,6 +25,7 @@ static char const *input_old = "perf.data.old",
static char diff__default_sort_order[] = "dso,symbol";
static bool force;
static bool show_displacement;
+static bool show_baseline_only;
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
@@ -173,6 +174,31 @@ static void resort_evlist(struct perf_evlist *evlist, bool name)
}
}
+static void hists__baseline_only(struct hists *hists)
+{
+ struct rb_node *next = rb_first(&hists->entries);
+
+ while (next != NULL) {
+ struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
+
+ next = rb_next(&he->rb_node);
+ if (!he->pair) {
+ rb_erase(&he->rb_node, &hists->entries);
+ hist_entry__free(he);
+ }
+ }
+}
+
+static void hists__process(struct hists *old, struct hists *new)
+{
+ hists__match(old, new);
+
+ if (show_baseline_only)
+ hists__baseline_only(new);
+
+ hists__fprintf(new, true, 0, 0, stdout);
+}
+
static int __cmd_diff(void)
{
int ret, i;
@@ -214,8 +240,7 @@ static int __cmd_diff(void)
first = false;
- hists__match(&evsel_old->hists, &evsel->hists);
- hists__fprintf(&evsel->hists, true, 0, 0, stdout);
+ hists__process(&evsel_old->hists, &evsel->hists);
}
out_delete:
@@ -236,6 +261,8 @@ static const struct option options[] = {
"be more verbose (show symbol address, etc)"),
OPT_BOOLEAN('M', "displacement", &show_displacement,
"Show position displacement relative to baseline"),
+ OPT_BOOLEAN('b', "baseline-only", &show_baseline_only,
+ "Show only items with match in baseline"),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
"dump raw trace in ASCII"),
OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 07/12] perf diff: Add ratio computation way to compare hist entries
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (5 preceding siblings ...)
2012-09-06 15:47 ` [PATCH 06/12] perf diff: Add -b option for perf diff to display paired entries only Jiri Olsa
@ 2012-09-06 15:47 ` Jiri Olsa
2012-09-07 5:45 ` Namhyung Kim
2012-09-06 15:47 ` [PATCH 08/12] perf diff: Add option to sort entries based on diff computation Jiri Olsa
` (6 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:47 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
Adding -c option to select computation method with the current
'Delta' computation as default. Current posible values are of
this option are: 'delta' and 'ratio'.
Adding 'ratio' as new computation way to compare hist entries.
If specified the 'Ratio' column is displayed with value 'r'
computed as:
r = A->period / B->period
with:
- A/B being matching hist entry from first/second file specified
(or perf.data/perf.data.old) respectively.
- period being the hist entry period value
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 33 ++++++++++++++++++++++
tools/perf/builtin-diff.c | 51 +++++++++++++++++++++++++++++++++-
tools/perf/ui/stdio/hist.c | 13 +++++++++
tools/perf/util/hist.h | 1 +
4 files changed, 97 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 6ce9585..8fff061 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -76,6 +76,39 @@ OPTIONS
--baseline-only::
Show only items with match in baseline.
+-c::
+--compute::
+ Differential computation selection - delta,ratio (default is delta).
+ See COMPARISON METHODS section for more info.
+
+COMPARISON METHODS
+------------------
+delta
+~~~~~
+If specified the 'Delta' column is displayed with value 'd' computed as:
+
+ d = A->period_percent - B->period_percent
+
+with:
+ - A/B being matching hist entry from first/second file specified
+ (or perf.data/perf.data.old) respectively.
+
+ - period_percent being the % of the hist entry period value within
+ single data file
+
+ratio
+~~~~~
+If specified the 'Ratio' column is displayed with value 'r' computed as:
+
+ r = A->period / B->period
+
+with:
+ - A/B being matching hist entry from first/second file specified
+ (or perf.data/perf.data.old) respectively.
+
+ - period being the hist entry period value
+
+
SEE ALSO
--------
linkperf:perf-record[1]
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 46f4a10..cde08d4 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -27,6 +27,39 @@ static bool force;
static bool show_displacement;
static bool show_baseline_only;
+enum {
+ COMPUTE_DELTA,
+ COMPUTE_RATIO,
+ COMPUTE_MAX,
+};
+
+const char *compute_names[COMPUTE_MAX] = {
+ [COMPUTE_DELTA] = "delta",
+ [COMPUTE_RATIO] = "ratio",
+};
+
+static const char *compute_str;
+static int compute;
+
+static int setup_compute(void)
+{
+ unsigned i;
+
+ if (!compute_str) {
+ compute = COMPUTE_DELTA;
+ return 0;
+ }
+
+ for (i = 0; i < COMPUTE_MAX; i++)
+ if (!strcmp(compute_str, compute_names[i])) {
+ compute = i;
+ return 0;
+ }
+
+ pr_err("Failed to find valid compute string\n");
+ return -EINVAL;
+}
+
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
{
@@ -263,6 +296,8 @@ static const struct option options[] = {
"Show position displacement relative to baseline"),
OPT_BOOLEAN('b', "baseline-only", &show_baseline_only,
"Show only items with match in baseline"),
+ OPT_STRING('c', "compute", &compute_str, "delta,ratio (default delta)",
+ "Entries differential computation selection"),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
"dump raw trace in ASCII"),
OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
@@ -288,7 +323,18 @@ static void setup_ui_stdio(void)
{
hists_stdio_column__register_idx(HISTC_BASELINE);
hists_stdio_column__register_global();
- hists_stdio_column__register_idx(HISTC_DELTA);
+
+ switch (compute) {
+ case COMPUTE_DELTA:
+ hists_stdio_column__register_idx(HISTC_DELTA);
+ break;
+ case COMPUTE_RATIO:
+ hists_stdio_column__register_idx(HISTC_RATIO);
+ break;
+ default:
+ BUG_ON(1);
+ };
+
if (show_displacement)
hists_stdio_column__register_idx(HISTC_DISPLACEMENT);
}
@@ -315,6 +361,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix __used)
if (symbol__init() < 0)
return -1;
+ if (setup_compute())
+ usage_with_options(diff_usage, options);
+
setup_sorting(diff_usage, options);
setup_ui_stdio();
setup_pager();
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 48fb0b3..02ba1c7 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -31,6 +31,18 @@ hists_stdio_column__delta_snprintf(struct hist_entry *he, char *bf,
}
static int
+hists_stdio_column__ratio_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ struct hist_entry *pair = he->pair;
+ double new_period = he->period;
+ double old_period = pair ? pair->period : 0;
+ double ratio = pair ? new_period / old_period : 0;
+
+ return percent_color_snprintf(bf, size, "%14.3F", ratio);
+}
+
+static int
hists_stdio_column__baseline_snprintf(struct hist_entry *he, char *bf,
size_t size, unsigned int width __used)
{
@@ -124,6 +136,7 @@ DEF_COLUMN(cpu_guest_us, HISTC_CPU_UTILIZATION_GUEST_US, 8, "guest us")
DEF_COLUMN(nr_samples, HISTC_NR_SAMPLES, 12, "Samples")
DEF_COLUMN(total_period, HISTC_TOTAL_PERIOD, 12, "Period")
DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
+DEF_COLUMN(ratio, HISTC_RATIO, 14, "Ratio")
DEF_COLUMN(displacement, HISTC_DISPLACEMENT, 5, "Displ")
};
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 36ff4c5..b24341d 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -46,6 +46,7 @@ enum hist_column {
HISTC_NR_SAMPLES,
HISTC_TOTAL_PERIOD,
HISTC_DELTA,
+ HISTC_RATIO,
HISTC_DISPLACEMENT,
/* sorted (hist_entry__sort_list) */
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 07/12] perf diff: Add ratio computation way to compare hist entries
2012-09-06 15:47 ` [PATCH 07/12] perf diff: Add ratio computation way to compare hist entries Jiri Olsa
@ 2012-09-07 5:45 ` Namhyung Kim
2012-09-07 9:26 ` Jiri Olsa
2012-09-07 15:33 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 40+ messages in thread
From: Namhyung Kim @ 2012-09-07 5:45 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
On Thu, 6 Sep 2012 17:47:01 +0200, Jiri Olsa wrote:
> Adding -c option to select computation method with the current
> 'Delta' computation as default. Current posible values are of
> this option are: 'delta' and 'ratio'.
>
> Adding 'ratio' as new computation way to compare hist entries.
> If specified the 'Ratio' column is displayed with value 'r'
> computed as:
>
> r = A->period / B->period
>
> with:
> - A/B being matching hist entry from first/second file specified
> (or perf.data/perf.data.old) respectively.
> - period being the hist entry period value
>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
> tools/perf/Documentation/perf-diff.txt | 33 ++++++++++++++++++++++
> tools/perf/builtin-diff.c | 51 +++++++++++++++++++++++++++++++++-
> tools/perf/ui/stdio/hist.c | 13 +++++++++
> tools/perf/util/hist.h | 1 +
> 4 files changed, 97 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
> index 6ce9585..8fff061 100644
> --- a/tools/perf/Documentation/perf-diff.txt
> +++ b/tools/perf/Documentation/perf-diff.txt
> @@ -76,6 +76,39 @@ OPTIONS
> --baseline-only::
> Show only items with match in baseline.
>
> +-c::
> +--compute::
> + Differential computation selection - delta,ratio (default is delta).
> + See COMPARISON METHODS section for more info.
> +
> +COMPARISON METHODS
> +------------------
> +delta
> +~~~~~
> +If specified the 'Delta' column is displayed with value 'd' computed as:
> +
> + d = A->period_percent - B->period_percent
> +
> +with:
> + - A/B being matching hist entry from first/second file specified
> + (or perf.data/perf.data.old) respectively.
> +
> + - period_percent being the % of the hist entry period value within
> + single data file
> +
> +ratio
> +~~~~~
> +If specified the 'Ratio' column is displayed with value 'r' computed as:
> +
> + r = A->period / B->period
> +
> +with:
> + - A/B being matching hist entry from first/second file specified
> + (or perf.data/perf.data.old) respectively.
> +
> + - period being the hist entry period value
> +
> +
> SEE ALSO
> --------
> linkperf:perf-record[1]
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index 46f4a10..cde08d4 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -27,6 +27,39 @@ static bool force;
> static bool show_displacement;
> static bool show_baseline_only;
>
> +enum {
> + COMPUTE_DELTA,
> + COMPUTE_RATIO,
> + COMPUTE_MAX,
> +};
> +
> +const char *compute_names[COMPUTE_MAX] = {
> + [COMPUTE_DELTA] = "delta",
> + [COMPUTE_RATIO] = "ratio",
> +};
> +
> +static const char *compute_str;
> +static int compute;
> +
> +static int setup_compute(void)
> +{
> + unsigned i;
> +
> + if (!compute_str) {
> + compute = COMPUTE_DELTA;
> + return 0;
> + }
> +
> + for (i = 0; i < COMPUTE_MAX; i++)
> + if (!strcmp(compute_str, compute_names[i])) {
> + compute = i;
> + return 0;
> + }
> +
> + pr_err("Failed to find valid compute string\n");
> + return -EINVAL;
> +}
> +
> static int hists__add_entry(struct hists *self,
> struct addr_location *al, u64 period)
> {
> @@ -263,6 +296,8 @@ static const struct option options[] = {
> "Show position displacement relative to baseline"),
> OPT_BOOLEAN('b', "baseline-only", &show_baseline_only,
> "Show only items with match in baseline"),
> + OPT_STRING('c', "compute", &compute_str, "delta,ratio (default delta)",
> + "Entries differential computation selection"),
Why not make it OPT_CALLBACK?
Thanks,
Namhyung
> OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
> "dump raw trace in ASCII"),
> OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
> @@ -288,7 +323,18 @@ static void setup_ui_stdio(void)
> {
> hists_stdio_column__register_idx(HISTC_BASELINE);
> hists_stdio_column__register_global();
> - hists_stdio_column__register_idx(HISTC_DELTA);
> +
> + switch (compute) {
> + case COMPUTE_DELTA:
> + hists_stdio_column__register_idx(HISTC_DELTA);
> + break;
> + case COMPUTE_RATIO:
> + hists_stdio_column__register_idx(HISTC_RATIO);
> + break;
> + default:
> + BUG_ON(1);
> + };
> +
> if (show_displacement)
> hists_stdio_column__register_idx(HISTC_DISPLACEMENT);
> }
> @@ -315,6 +361,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix __used)
> if (symbol__init() < 0)
> return -1;
>
> + if (setup_compute())
> + usage_with_options(diff_usage, options);
> +
> setup_sorting(diff_usage, options);
> setup_ui_stdio();
> setup_pager();
> diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
> index 48fb0b3..02ba1c7 100644
> --- a/tools/perf/ui/stdio/hist.c
> +++ b/tools/perf/ui/stdio/hist.c
> @@ -31,6 +31,18 @@ hists_stdio_column__delta_snprintf(struct hist_entry *he, char *bf,
> }
>
> static int
> +hists_stdio_column__ratio_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + struct hist_entry *pair = he->pair;
> + double new_period = he->period;
> + double old_period = pair ? pair->period : 0;
> + double ratio = pair ? new_period / old_period : 0;
> +
> + return percent_color_snprintf(bf, size, "%14.3F", ratio);
> +}
> +
> +static int
> hists_stdio_column__baseline_snprintf(struct hist_entry *he, char *bf,
> size_t size, unsigned int width __used)
> {
> @@ -124,6 +136,7 @@ DEF_COLUMN(cpu_guest_us, HISTC_CPU_UTILIZATION_GUEST_US, 8, "guest us")
> DEF_COLUMN(nr_samples, HISTC_NR_SAMPLES, 12, "Samples")
> DEF_COLUMN(total_period, HISTC_TOTAL_PERIOD, 12, "Period")
> DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
> +DEF_COLUMN(ratio, HISTC_RATIO, 14, "Ratio")
> DEF_COLUMN(displacement, HISTC_DISPLACEMENT, 5, "Displ")
> };
>
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 36ff4c5..b24341d 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -46,6 +46,7 @@ enum hist_column {
> HISTC_NR_SAMPLES,
> HISTC_TOTAL_PERIOD,
> HISTC_DELTA,
> + HISTC_RATIO,
> HISTC_DISPLACEMENT,
>
> /* sorted (hist_entry__sort_list) */
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 07/12] perf diff: Add ratio computation way to compare hist entries
2012-09-07 5:45 ` Namhyung Kim
@ 2012-09-07 9:26 ` Jiri Olsa
2012-09-07 15:33 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 40+ messages in thread
From: Jiri Olsa @ 2012-09-07 9:26 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
On Fri, Sep 07, 2012 at 02:45:28PM +0900, Namhyung Kim wrote:
> On Thu, 6 Sep 2012 17:47:01 +0200, Jiri Olsa wrote:
> > Adding -c option to select computation method with the current
> > 'Delta' computation as default. Current posible values are of
> > this option are: 'delta' and 'ratio'.
> >
SNIP
> > @@ -263,6 +296,8 @@ static const struct option options[] = {
> > "Show position displacement relative to baseline"),
> > OPT_BOOLEAN('b', "baseline-only", &show_baseline_only,
> > "Show only items with match in baseline"),
> > + OPT_STRING('c', "compute", &compute_str, "delta,ratio (default delta)",
> > + "Entries differential computation selection"),
>
> Why not make it OPT_CALLBACK?
>
right, thanks
jirka
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 07/12] perf diff: Add ratio computation way to compare hist entries
2012-09-07 5:45 ` Namhyung Kim
2012-09-07 9:26 ` Jiri Olsa
@ 2012-09-07 15:33 ` Arnaldo Carvalho de Melo
2012-09-07 15:41 ` Namhyung Kim
1 sibling, 1 reply; 40+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-07 15:33 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
Em Fri, Sep 07, 2012 at 02:45:28PM +0900, Namhyung Kim escreveu:
> On Thu, 6 Sep 2012 17:47:01 +0200, Jiri Olsa wrote:
> > @@ -263,6 +296,8 @@ static const struct option options[] = {
> > + OPT_STRING('c', "compute", &compute_str, "delta,ratio (default delta)",
> > + "Entries differential computation selection"),
>
> Why not make it OPT_CALLBACK?
Namhyung, please trim the message to contain just the snippet that is
related to your comment,
Thanks,
- Arnaldo
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 07/12] perf diff: Add ratio computation way to compare hist entries
2012-09-07 15:33 ` Arnaldo Carvalho de Melo
@ 2012-09-07 15:41 ` Namhyung Kim
0 siblings, 0 replies; 40+ messages in thread
From: Namhyung Kim @ 2012-09-07 15:41 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
2012-09-07 (금), 08:33 -0700, Arnaldo Carvalho de Melo:
> Em Fri, Sep 07, 2012 at 02:45:28PM +0900, Namhyung Kim escreveu:
> > On Thu, 6 Sep 2012 17:47:01 +0200, Jiri Olsa wrote:
> > > @@ -263,6 +296,8 @@ static const struct option options[] = {
> > > + OPT_STRING('c', "compute", &compute_str, "delta,ratio (default delta)",
> > > + "Entries differential computation selection"),
> >
> > Why not make it OPT_CALLBACK?
>
> Namhyung, please trim the message to contain just the snippet that is
> related to your comment,
Will do that later. Sorry for the inconvenience!
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 08/12] perf diff: Add option to sort entries based on diff computation
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (6 preceding siblings ...)
2012-09-06 15:47 ` [PATCH 07/12] perf diff: Add ratio computation way to compare hist entries Jiri Olsa
@ 2012-09-06 15:47 ` Jiri Olsa
2012-09-06 15:47 ` [PATCH 09/12] perf diff: Add weighted diff computation way to compare hist entries Jiri Olsa
` (5 subsequent siblings)
13 siblings, 0 replies; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:47 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
Adding support to sort hist entries based on the outcome of selected
computation. It's now possible to specify '+' as a first character
of '-c' option value to make such sort.
Example:
$ ./perf diff -cratio -b
# Event 'cache-misses'
#
# Baseline Ratio Shared Object Symbol
# ........ .............. ................. ................................
#
19.64% 0.69 [kernel.kallsyms] [k] clear_page
0.30% 0.17 [kernel.kallsyms] [k] mm_alloc
0.04% 0.20 [kernel.kallsyms] [k] kmem_cache_alloc
$ ./perf diff -c+ratio -b
# Event 'cache-misses'
#
# Baseline Ratio Shared Object Symbol
# ........ .............. ................. ................................
#
19.64% 0.69 [kernel.kallsyms] [k] clear_page
0.04% 0.20 [kernel.kallsyms] [k] kmem_cache_alloc
0.30% 0.17 [kernel.kallsyms] [k] mm_alloc
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 2 +
tools/perf/builtin-diff.c | 140 +++++++++++++++++++++++++++++++++
tools/perf/ui/stdio/hist.c | 20 +++--
tools/perf/ui/stdio/hist.h | 4 +
tools/perf/util/sort.h | 15 ++++
5 files changed, 173 insertions(+), 8 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 8fff061..cff3d9b 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -79,6 +79,8 @@ OPTIONS
-c::
--compute::
Differential computation selection - delta,ratio (default is delta).
+ If '+' is specified as a first character, the output is sorted based
+ on the computation results.
See COMPARISON METHODS section for more info.
COMPARISON METHODS
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index cde08d4..f72a2e4 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -26,6 +26,7 @@ static char diff__default_sort_order[] = "dso,symbol";
static bool force;
static bool show_displacement;
static bool show_baseline_only;
+static bool sort_compute;
enum {
COMPUTE_DELTA,
@@ -50,6 +51,13 @@ static int setup_compute(void)
return 0;
}
+ if (*compute_str == '+') {
+ sort_compute = true;
+ compute_str++;
+ if (!*compute_str)
+ return 0;
+ }
+
for (i = 0; i < COMPUTE_MAX; i++)
if (!strcmp(compute_str, compute_names[i])) {
compute = i;
@@ -60,6 +68,34 @@ static int setup_compute(void)
return -EINVAL;
}
+static double get_period_percent(struct hist_entry *he, u64 period)
+{
+ u64 total = he->hists->stats.total_period;
+ return (period * 100.0) / total;
+}
+
+double perf_diff__compute_delta(struct hist_entry *he)
+{
+ struct hist_entry *pair = he->pair;
+ double new_percent = get_period_percent(he, he->period);
+ double old_percent = pair ? get_period_percent(pair, pair->period) : 0.0;
+
+ he->diff.period_ratio_delta = new_percent - old_percent;
+ he->diff.computed = true;
+ return he->diff.period_ratio_delta;
+}
+
+double perf_diff__compute_ratio(struct hist_entry *he)
+{
+ struct hist_entry *pair = he->pair;
+ double new_period = he->period;
+ double old_period = pair ? pair->period : 0;
+
+ he->diff.computed = true;
+ he->diff.period_ratio = pair ? (new_period / old_period) : 0;
+ return he->diff.period_ratio;
+}
+
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
{
@@ -222,6 +258,105 @@ static void hists__baseline_only(struct hists *hists)
}
}
+static void hists__precompute(struct hists *hists)
+{
+ struct rb_node *next = rb_first(&hists->entries);
+
+ while (next != NULL) {
+ struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
+
+ next = rb_next(&he->rb_node);
+
+ if (!he->pair)
+ continue;
+
+ switch (compute) {
+ case COMPUTE_DELTA:
+ perf_diff__compute_delta(he);
+ break;
+ case COMPUTE_RATIO:
+ perf_diff__compute_ratio(he);
+ break;
+ default:
+ BUG_ON(1);
+ }
+ }
+}
+
+static int64_t cmp_doubles(double l, double r)
+{
+ if (l > r)
+ return -1;
+ else if (l < r)
+ return 1;
+ else
+ return 0;
+}
+
+static int64_t
+hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
+ int c)
+{
+ switch (c) {
+ case COMPUTE_DELTA:
+ {
+ double l = left->diff.period_ratio_delta;
+ double r = right->diff.period_ratio_delta;
+
+ return cmp_doubles(l, r);
+ }
+ case COMPUTE_RATIO:
+ {
+ double l = left->diff.period_ratio;
+ double r = right->diff.period_ratio;
+
+ return cmp_doubles(l, r);
+ }
+ default:
+ BUG_ON(1);
+ }
+
+ return 0;
+}
+
+static void insert_hist_entry_by_compute(struct rb_root *root,
+ struct hist_entry *he,
+ int c)
+{
+ struct rb_node **p = &root->rb_node;
+ struct rb_node *parent = NULL;
+ struct hist_entry *iter;
+
+ while (*p != NULL) {
+ parent = *p;
+ iter = rb_entry(parent, struct hist_entry, rb_node);
+ if (hist_entry__cmp_compute(he, iter, c) < 0)
+ p = &(*p)->rb_left;
+ else
+ p = &(*p)->rb_right;
+ }
+
+ rb_link_node(&he->rb_node, parent, p);
+ rb_insert_color(&he->rb_node, root);
+}
+
+static void hists__compute_resort(struct hists *hists)
+{
+ struct rb_root tmp = RB_ROOT;
+ struct rb_node *next = rb_first(&hists->entries);
+
+ while (next != NULL) {
+ struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
+
+ next = rb_next(&he->rb_node);
+
+ rb_erase(&he->rb_node, &hists->entries);
+ insert_hist_entry_by_compute(&tmp, he, compute);
+ }
+
+ hists->entries = tmp;
+}
+
static void hists__process(struct hists *old, struct hists *new)
{
hists__match(old, new);
@@ -229,6 +364,11 @@ static void hists__process(struct hists *old, struct hists *new)
if (show_baseline_only)
hists__baseline_only(new);
+ if (sort_compute) {
+ hists__precompute(new);
+ hists__compute_resort(new);
+ }
+
hists__fprintf(new, true, 0, 0, stdout);
}
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 02ba1c7..8c717ab 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -16,12 +16,14 @@ static int
hists_stdio_column__delta_snprintf(struct hist_entry *he, char *bf,
size_t size, unsigned int width __used)
{
- struct hist_entry *pair = he->pair;
- double new_percent = get_period_percent(he, he->period);
- double old_percent = pair ? get_period_percent(pair, pair->period) : 0.0;
- double diff = new_percent - old_percent;
+ double diff;
int ret;
+ if (he->diff.computed)
+ diff = he->diff.period_ratio_delta;
+ else
+ diff = perf_diff__compute_delta(he);
+
if (fabs(diff) >= 0.01)
ret = percent_color_snprintf(bf, size, "%+7.2F%%", diff);
else
@@ -34,10 +36,12 @@ static int
hists_stdio_column__ratio_snprintf(struct hist_entry *he, char *bf,
size_t size, unsigned int width __used)
{
- struct hist_entry *pair = he->pair;
- double new_period = he->period;
- double old_period = pair ? pair->period : 0;
- double ratio = pair ? new_period / old_period : 0;
+ double ratio;
+
+ if (he->diff.computed)
+ ratio = he->diff.period_ratio;
+ else
+ ratio = perf_diff__compute_ratio(he);
return percent_color_snprintf(bf, size, "%14.3F", ratio);
}
diff --git a/tools/perf/ui/stdio/hist.h b/tools/perf/ui/stdio/hist.h
index 8e15d88..c8ac633 100644
--- a/tools/perf/ui/stdio/hist.h
+++ b/tools/perf/ui/stdio/hist.h
@@ -16,4 +16,8 @@ struct hists_stdio_column {
int hists_stdio_column__register_idx(int idx);
void hists_stdio_column__register_global(void);
void hists_stdio_column__set_width(struct hists *hists);
+
+double perf_diff__compute_delta(struct hist_entry *he);
+double perf_diff__compute_ratio(struct hist_entry *he);
+
#endif /* __PERF_UI_STDIO_HIST_H */
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 967d381..9f707b7 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -42,6 +42,19 @@ extern struct sort_entry sort_sym_from;
extern struct sort_entry sort_sym_to;
extern enum sort_type sort__first_dimension;
+struct hist_entry_diff {
+ bool computed;
+
+ /* HISTC_DISPLACEMENT */
+ int displacement;
+
+ /* HISTC_DELTA */
+ double period_ratio_delta;
+
+ /* HISTC_RATIO */
+ double period_ratio;
+};
+
/**
* struct hist_entry - histogram entry
*
@@ -62,6 +75,8 @@ struct hist_entry {
s32 cpu;
u32 nr_events;
+ struct hist_entry_diff diff;
+
/* XXX These two should move to some tree widget lib */
u16 row_offset;
u16 nr_rows;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 09/12] perf diff: Add weighted diff computation way to compare hist entries
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (7 preceding siblings ...)
2012-09-06 15:47 ` [PATCH 08/12] perf diff: Add option to sort entries based on diff computation Jiri Olsa
@ 2012-09-06 15:47 ` Jiri Olsa
2012-09-07 5:58 ` Namhyung Kim
2012-09-06 15:47 ` [PATCH 10/12] perf diff: Add -p option to display period values for " Jiri Olsa
` (4 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:47 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
Adding 'wdiff' as new computation way to compare hist entries.
If specified the 'Weighted diff' column is displayed with value 'd'
computed as:
d = B->period * WEIGHT-A - A->period * WEIGHT-B
- A/B being matching hist entry from first/second file specified
(or perf.data/perf.data.old) respectively.
- period being the hist entry period value
- WEIGHT-A/WEIGHT-B being user suplied weights in the the '-c' option
behind ':' separator like '-c wdiff:1,2'.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 15 +++++-
tools/perf/builtin-diff.c | 95 +++++++++++++++++++++++++++++++++-
tools/perf/ui/stdio/hist.c | 15 ++++++
tools/perf/ui/stdio/hist.h | 1 +
tools/perf/util/hist.h | 1 +
tools/perf/util/sort.h | 3 ++
6 files changed, 127 insertions(+), 3 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index cff3d9b..fa413ac 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -78,7 +78,7 @@ OPTIONS
-c::
--compute::
- Differential computation selection - delta,ratio (default is delta).
+ Differential computation selection - delta,ratio,wdiff (default is delta).
If '+' is specified as a first character, the output is sorted based
on the computation results.
See COMPARISON METHODS section for more info.
@@ -110,6 +110,19 @@ with:
- period being the hist entry period value
+wdiff
+~~~~~
+If specified the 'Weighted diff' column is displayed with value 'd' computed as:
+
+ d = B->period * WEIGHT-A - A->period * WEIGHT-B
+
+ - A/B being matching hist entry from first/second file specified
+ (or perf.data/perf.data.old) respectively.
+
+ - period being the hist entry period value
+
+ - WEIGHT-A/WEIGHT-B being user suplied weights in the the '-c' option
+ behind ':' separator like '-c wdiff:1,2'.
SEE ALSO
--------
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index f72a2e4..6d8aba8 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -28,23 +28,79 @@ static bool show_displacement;
static bool show_baseline_only;
static bool sort_compute;
+static s64 compute_wdiff_w1;
+static s64 compute_wdiff_w2;
+
enum {
COMPUTE_DELTA,
COMPUTE_RATIO,
+ COMPUTE_WEIGHTED_DIFF,
COMPUTE_MAX,
};
const char *compute_names[COMPUTE_MAX] = {
[COMPUTE_DELTA] = "delta",
[COMPUTE_RATIO] = "ratio",
+ [COMPUTE_WEIGHTED_DIFF] = "wdiff",
};
static const char *compute_str;
static int compute;
+static int setup_compute_opt_wdiff(char *opt)
+{
+ char *w1_str = opt;
+ char *w2_str;
+
+ int ret = -EINVAL;
+
+ do {
+ if (!opt)
+ break;
+
+ w2_str = strchr(opt, ',');
+ if (!w2_str)
+ break;
+
+ *w2_str++ = 0x0;
+ if (!*w2_str)
+ break;
+
+ compute_wdiff_w1 = strtol(w1_str, NULL, 10);
+ compute_wdiff_w2 = strtol(w2_str, NULL, 10);
+
+ if (!compute_wdiff_w1 || !compute_wdiff_w2)
+ break;
+
+ pr_debug("compute wdiff w1(%" PRId64 ") w2(%" PRId64 ")\n",
+ compute_wdiff_w1, compute_wdiff_w2);
+ ret = 0;
+
+ } while (0);
+
+ if (ret)
+ pr_err("Weight parsing failed.");
+
+ return ret;
+}
+
+static int setup_compute_opt(char *opt)
+{
+ if (compute == COMPUTE_WEIGHTED_DIFF)
+ return setup_compute_opt_wdiff(opt);
+
+ if (opt) {
+ pr_err("Extra option specified.");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int setup_compute(void)
{
unsigned i;
+ char *opt;
if (!compute_str) {
compute = COMPUTE_DELTA;
@@ -58,10 +114,14 @@ static int setup_compute(void)
return 0;
}
+ opt = strchr(compute_str, ':');
+ if (opt)
+ *opt++ = 0x0;
+
for (i = 0; i < COMPUTE_MAX; i++)
if (!strcmp(compute_str, compute_names[i])) {
compute = i;
- return 0;
+ return setup_compute_opt(opt);
}
pr_err("Failed to find valid compute string\n");
@@ -96,6 +156,23 @@ double perf_diff__compute_ratio(struct hist_entry *he)
return he->diff.period_ratio;
}
+double perf_diff__compute_wdiff(struct hist_entry *he)
+{
+ struct hist_entry *pair = he->pair;
+ u64 new_period = he->period;
+ u64 old_period = pair ? pair->period : 0;
+
+ he->diff.computed = true;
+
+ if (!pair)
+ he->diff.wdiff = 0;
+ else
+ he->diff.wdiff = new_period * compute_wdiff_w2 -
+ old_period * compute_wdiff_w1;
+
+ return he->diff.wdiff;
+}
+
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
{
@@ -277,6 +354,9 @@ static void hists__precompute(struct hists *hists)
case COMPUTE_RATIO:
perf_diff__compute_ratio(he);
break;
+ case COMPUTE_WEIGHTED_DIFF:
+ perf_diff__compute_wdiff(he);
+ break;
default:
BUG_ON(1);
}
@@ -312,6 +392,13 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
return cmp_doubles(l, r);
}
+ case COMPUTE_WEIGHTED_DIFF:
+ {
+ s64 l = left->diff.wdiff;
+ s64 r = right->diff.wdiff;
+
+ return r - l;
+ }
default:
BUG_ON(1);
}
@@ -436,7 +523,8 @@ static const struct option options[] = {
"Show position displacement relative to baseline"),
OPT_BOOLEAN('b', "baseline-only", &show_baseline_only,
"Show only items with match in baseline"),
- OPT_STRING('c', "compute", &compute_str, "delta,ratio (default delta)",
+ OPT_STRING('c', "compute", &compute_str,
+ "delta,ratio,wdiff:w1,w2 (default delta)",
"Entries differential computation selection"),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
"dump raw trace in ASCII"),
@@ -471,6 +559,9 @@ static void setup_ui_stdio(void)
case COMPUTE_RATIO:
hists_stdio_column__register_idx(HISTC_RATIO);
break;
+ case COMPUTE_WEIGHTED_DIFF:
+ hists_stdio_column__register_idx(HISTC_WEIGHTED_DIFF);
+ break;
default:
BUG_ON(1);
};
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 8c717ab..f580085 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -47,6 +47,20 @@ hists_stdio_column__ratio_snprintf(struct hist_entry *he, char *bf,
}
static int
+hists_stdio_column__wdiff_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ u64 wdiff;
+
+ if (he->diff.computed)
+ wdiff = he->diff.wdiff;
+ else
+ wdiff = perf_diff__compute_wdiff(he);
+
+ return scnprintf(bf, size , "%+13ld", wdiff);
+}
+
+static int
hists_stdio_column__baseline_snprintf(struct hist_entry *he, char *bf,
size_t size, unsigned int width __used)
{
@@ -141,6 +155,7 @@ DEF_COLUMN(nr_samples, HISTC_NR_SAMPLES, 12, "Samples")
DEF_COLUMN(total_period, HISTC_TOTAL_PERIOD, 12, "Period")
DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
DEF_COLUMN(ratio, HISTC_RATIO, 14, "Ratio")
+DEF_COLUMN(wdiff, HISTC_WEIGHTED_DIFF, 13, "Weighted diff")
DEF_COLUMN(displacement, HISTC_DISPLACEMENT, 5, "Displ")
};
diff --git a/tools/perf/ui/stdio/hist.h b/tools/perf/ui/stdio/hist.h
index c8ac633..f725189 100644
--- a/tools/perf/ui/stdio/hist.h
+++ b/tools/perf/ui/stdio/hist.h
@@ -19,5 +19,6 @@ void hists_stdio_column__set_width(struct hists *hists);
double perf_diff__compute_delta(struct hist_entry *he);
double perf_diff__compute_ratio(struct hist_entry *he);
+double perf_diff__compute_wdiff(struct hist_entry *he);
#endif /* __PERF_UI_STDIO_HIST_H */
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index b24341d..745e0cc 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -47,6 +47,7 @@ enum hist_column {
HISTC_TOTAL_PERIOD,
HISTC_DELTA,
HISTC_RATIO,
+ HISTC_WEIGHTED_DIFF,
HISTC_DISPLACEMENT,
/* sorted (hist_entry__sort_list) */
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 9f707b7..73f1ffe 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -53,6 +53,9 @@ struct hist_entry_diff {
/* HISTC_RATIO */
double period_ratio;
+
+ /* HISTC_WEIGHTED_DIFF */
+ s64 wdiff;
};
/**
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 09/12] perf diff: Add weighted diff computation way to compare hist entries
2012-09-06 15:47 ` [PATCH 09/12] perf diff: Add weighted diff computation way to compare hist entries Jiri Olsa
@ 2012-09-07 5:58 ` Namhyung Kim
2012-09-07 9:28 ` Jiri Olsa
0 siblings, 1 reply; 40+ messages in thread
From: Namhyung Kim @ 2012-09-07 5:58 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
On Thu, 6 Sep 2012 17:47:03 +0200, Jiri Olsa wrote:
> Adding 'wdiff' as new computation way to compare hist entries.
>
> If specified the 'Weighted diff' column is displayed with value 'd'
> computed as:
>
> d = B->period * WEIGHT-A - A->period * WEIGHT-B
>
> - A/B being matching hist entry from first/second file specified
> (or perf.data/perf.data.old) respectively.
>
> - period being the hist entry period value
>
> - WEIGHT-A/WEIGHT-B being user suplied weights in the the '-c' option
> behind ':' separator like '-c wdiff:1,2'.
>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
> tools/perf/Documentation/perf-diff.txt | 15 +++++-
> tools/perf/builtin-diff.c | 95 +++++++++++++++++++++++++++++++++-
> tools/perf/ui/stdio/hist.c | 15 ++++++
> tools/perf/ui/stdio/hist.h | 1 +
> tools/perf/util/hist.h | 1 +
> tools/perf/util/sort.h | 3 ++
> 6 files changed, 127 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
> index cff3d9b..fa413ac 100644
> --- a/tools/perf/Documentation/perf-diff.txt
> +++ b/tools/perf/Documentation/perf-diff.txt
> @@ -78,7 +78,7 @@ OPTIONS
>
> -c::
> --compute::
> - Differential computation selection - delta,ratio (default is delta).
> + Differential computation selection - delta,ratio,wdiff (default is delta).
> If '+' is specified as a first character, the output is sorted based
> on the computation results.
> See COMPARISON METHODS section for more info.
> @@ -110,6 +110,19 @@ with:
>
> - period being the hist entry period value
>
> +wdiff
> +~~~~~
> +If specified the 'Weighted diff' column is displayed with value 'd' computed as:
> +
> + d = B->period * WEIGHT-A - A->period * WEIGHT-B
> +
> + - A/B being matching hist entry from first/second file specified
> + (or perf.data/perf.data.old) respectively.
> +
> + - period being the hist entry period value
> +
> + - WEIGHT-A/WEIGHT-B being user suplied weights in the the '-c' option
> + behind ':' separator like '-c wdiff:1,2'.
>
> SEE ALSO
> --------
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index f72a2e4..6d8aba8 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -28,23 +28,79 @@ static bool show_displacement;
> static bool show_baseline_only;
> static bool sort_compute;
>
> +static s64 compute_wdiff_w1;
> +static s64 compute_wdiff_w2;
> +
> enum {
> COMPUTE_DELTA,
> COMPUTE_RATIO,
> + COMPUTE_WEIGHTED_DIFF,
> COMPUTE_MAX,
> };
>
> const char *compute_names[COMPUTE_MAX] = {
> [COMPUTE_DELTA] = "delta",
> [COMPUTE_RATIO] = "ratio",
> + [COMPUTE_WEIGHTED_DIFF] = "wdiff",
> };
>
> static const char *compute_str;
> static int compute;
>
> +static int setup_compute_opt_wdiff(char *opt)
> +{
> + char *w1_str = opt;
> + char *w2_str;
> +
> + int ret = -EINVAL;
> +
> + do {
> + if (!opt)
> + break;
> +
> + w2_str = strchr(opt, ',');
> + if (!w2_str)
> + break;
> +
> + *w2_str++ = 0x0;
> + if (!*w2_str)
> + break;
> +
> + compute_wdiff_w1 = strtol(w1_str, NULL, 10);
> + compute_wdiff_w2 = strtol(w2_str, NULL, 10);
> +
> + if (!compute_wdiff_w1 || !compute_wdiff_w2)
> + break;
> +
> + pr_debug("compute wdiff w1(%" PRId64 ") w2(%" PRId64 ")\n",
> + compute_wdiff_w1, compute_wdiff_w2);
> + ret = 0;
> +
> + } while (0);
I don't see why this do { } while(0) loop is necessary.
How about this?
w1 = strtol(opt, &tmp, 10);
if (*tmp != ',')
goto error;
opt = tmp + 1;
w2 = strtol(opt, &tmp, 10);
if (*tmp != '\0')
goto error;
if (!w1 || !w2)
goto error;
Thanks,
Namhyung
> +
> + if (ret)
> + pr_err("Weight parsing failed.");
> +
> + return ret;
> +}
> +
> +static int setup_compute_opt(char *opt)
> +{
> + if (compute == COMPUTE_WEIGHTED_DIFF)
> + return setup_compute_opt_wdiff(opt);
> +
> + if (opt) {
> + pr_err("Extra option specified.");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int setup_compute(void)
> {
> unsigned i;
> + char *opt;
>
> if (!compute_str) {
> compute = COMPUTE_DELTA;
> @@ -58,10 +114,14 @@ static int setup_compute(void)
> return 0;
> }
>
> + opt = strchr(compute_str, ':');
> + if (opt)
> + *opt++ = 0x0;
> +
> for (i = 0; i < COMPUTE_MAX; i++)
> if (!strcmp(compute_str, compute_names[i])) {
> compute = i;
> - return 0;
> + return setup_compute_opt(opt);
> }
>
> pr_err("Failed to find valid compute string\n");
> @@ -96,6 +156,23 @@ double perf_diff__compute_ratio(struct hist_entry *he)
> return he->diff.period_ratio;
> }
>
> +double perf_diff__compute_wdiff(struct hist_entry *he)
> +{
> + struct hist_entry *pair = he->pair;
> + u64 new_period = he->period;
> + u64 old_period = pair ? pair->period : 0;
> +
> + he->diff.computed = true;
> +
> + if (!pair)
> + he->diff.wdiff = 0;
> + else
> + he->diff.wdiff = new_period * compute_wdiff_w2 -
> + old_period * compute_wdiff_w1;
> +
> + return he->diff.wdiff;
> +}
> +
> static int hists__add_entry(struct hists *self,
> struct addr_location *al, u64 period)
> {
> @@ -277,6 +354,9 @@ static void hists__precompute(struct hists *hists)
> case COMPUTE_RATIO:
> perf_diff__compute_ratio(he);
> break;
> + case COMPUTE_WEIGHTED_DIFF:
> + perf_diff__compute_wdiff(he);
> + break;
> default:
> BUG_ON(1);
> }
> @@ -312,6 +392,13 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
>
> return cmp_doubles(l, r);
> }
> + case COMPUTE_WEIGHTED_DIFF:
> + {
> + s64 l = left->diff.wdiff;
> + s64 r = right->diff.wdiff;
> +
> + return r - l;
> + }
> default:
> BUG_ON(1);
> }
> @@ -436,7 +523,8 @@ static const struct option options[] = {
> "Show position displacement relative to baseline"),
> OPT_BOOLEAN('b', "baseline-only", &show_baseline_only,
> "Show only items with match in baseline"),
> - OPT_STRING('c', "compute", &compute_str, "delta,ratio (default delta)",
> + OPT_STRING('c', "compute", &compute_str,
> + "delta,ratio,wdiff:w1,w2 (default delta)",
> "Entries differential computation selection"),
> OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
> "dump raw trace in ASCII"),
> @@ -471,6 +559,9 @@ static void setup_ui_stdio(void)
> case COMPUTE_RATIO:
> hists_stdio_column__register_idx(HISTC_RATIO);
> break;
> + case COMPUTE_WEIGHTED_DIFF:
> + hists_stdio_column__register_idx(HISTC_WEIGHTED_DIFF);
> + break;
> default:
> BUG_ON(1);
> };
> diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
> index 8c717ab..f580085 100644
> --- a/tools/perf/ui/stdio/hist.c
> +++ b/tools/perf/ui/stdio/hist.c
> @@ -47,6 +47,20 @@ hists_stdio_column__ratio_snprintf(struct hist_entry *he, char *bf,
> }
>
> static int
> +hists_stdio_column__wdiff_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + u64 wdiff;
> +
> + if (he->diff.computed)
> + wdiff = he->diff.wdiff;
> + else
> + wdiff = perf_diff__compute_wdiff(he);
> +
> + return scnprintf(bf, size , "%+13ld", wdiff);
> +}
> +
> +static int
> hists_stdio_column__baseline_snprintf(struct hist_entry *he, char *bf,
> size_t size, unsigned int width __used)
> {
> @@ -141,6 +155,7 @@ DEF_COLUMN(nr_samples, HISTC_NR_SAMPLES, 12, "Samples")
> DEF_COLUMN(total_period, HISTC_TOTAL_PERIOD, 12, "Period")
> DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
> DEF_COLUMN(ratio, HISTC_RATIO, 14, "Ratio")
> +DEF_COLUMN(wdiff, HISTC_WEIGHTED_DIFF, 13, "Weighted diff")
> DEF_COLUMN(displacement, HISTC_DISPLACEMENT, 5, "Displ")
> };
>
> diff --git a/tools/perf/ui/stdio/hist.h b/tools/perf/ui/stdio/hist.h
> index c8ac633..f725189 100644
> --- a/tools/perf/ui/stdio/hist.h
> +++ b/tools/perf/ui/stdio/hist.h
> @@ -19,5 +19,6 @@ void hists_stdio_column__set_width(struct hists *hists);
>
> double perf_diff__compute_delta(struct hist_entry *he);
> double perf_diff__compute_ratio(struct hist_entry *he);
> +double perf_diff__compute_wdiff(struct hist_entry *he);
>
> #endif /* __PERF_UI_STDIO_HIST_H */
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index b24341d..745e0cc 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -47,6 +47,7 @@ enum hist_column {
> HISTC_TOTAL_PERIOD,
> HISTC_DELTA,
> HISTC_RATIO,
> + HISTC_WEIGHTED_DIFF,
> HISTC_DISPLACEMENT,
>
> /* sorted (hist_entry__sort_list) */
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 9f707b7..73f1ffe 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -53,6 +53,9 @@ struct hist_entry_diff {
>
> /* HISTC_RATIO */
> double period_ratio;
> +
> + /* HISTC_WEIGHTED_DIFF */
> + s64 wdiff;
> };
>
> /**
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 09/12] perf diff: Add weighted diff computation way to compare hist entries
2012-09-07 5:58 ` Namhyung Kim
@ 2012-09-07 9:28 ` Jiri Olsa
2012-09-07 13:33 ` Namhyung Kim
0 siblings, 1 reply; 40+ messages in thread
From: Jiri Olsa @ 2012-09-07 9:28 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
On Fri, Sep 07, 2012 at 02:58:19PM +0900, Namhyung Kim wrote:
> On Thu, 6 Sep 2012 17:47:03 +0200, Jiri Olsa wrote:
> > Adding 'wdiff' as new computation way to compare hist entries.
SNIP
> > +static int setup_compute_opt_wdiff(char *opt)
> > +{
> > + char *w1_str = opt;
> > + char *w2_str;
> > +
> > + int ret = -EINVAL;
> > +
> > + do {
> > + if (!opt)
> > + break;
> > +
> > + w2_str = strchr(opt, ',');
> > + if (!w2_str)
> > + break;
> > +
> > + *w2_str++ = 0x0;
> > + if (!*w2_str)
> > + break;
> > +
> > + compute_wdiff_w1 = strtol(w1_str, NULL, 10);
> > + compute_wdiff_w2 = strtol(w2_str, NULL, 10);
> > +
> > + if (!compute_wdiff_w1 || !compute_wdiff_w2)
> > + break;
> > +
> > + pr_debug("compute wdiff w1(%" PRId64 ") w2(%" PRId64 ")\n",
> > + compute_wdiff_w1, compute_wdiff_w2);
> > + ret = 0;
> > +
> > + } while (0);
>
> I don't see why this do { } while(0) loop is necessary.
> How about this?
>
> w1 = strtol(opt, &tmp, 10);
> if (*tmp != ',')
> goto error;
>
> opt = tmp + 1;
> w2 = strtol(opt, &tmp, 10);
> if (*tmp != '\0')
> goto error;
>
> if (!w1 || !w2)
> goto error;
I do this not to use labels & goto ;)
jirka
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 09/12] perf diff: Add weighted diff computation way to compare hist entries
2012-09-07 9:28 ` Jiri Olsa
@ 2012-09-07 13:33 ` Namhyung Kim
2012-09-07 15:26 ` Peter Zijlstra
2012-09-07 15:31 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 40+ messages in thread
From: Namhyung Kim @ 2012-09-07 13:33 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
2012-09-07 (금), 11:28 +0200, Jiri Olsa:
> On Fri, Sep 07, 2012 at 02:58:19PM +0900, Namhyung Kim wrote:
> > I don't see why this do { } while(0) loop is necessary.
> > How about this?
> >
> > w1 = strtol(opt, &tmp, 10);
> > if (*tmp != ',')
> > goto error;
> >
> > opt = tmp + 1;
> > w2 = strtol(opt, &tmp, 10);
> > if (*tmp != '\0')
> > goto error;
> >
> > if (!w1 || !w2)
> > goto error;
>
> I do this not to use labels & goto ;)
But isn't it usual? Do you have a reason not to do it?
I was a bit confused finding which path actually make it a loop... but
there's none. :-/
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 09/12] perf diff: Add weighted diff computation way to compare hist entries
2012-09-07 13:33 ` Namhyung Kim
@ 2012-09-07 15:26 ` Peter Zijlstra
2012-09-07 15:31 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 40+ messages in thread
From: Peter Zijlstra @ 2012-09-07 15:26 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
On Fri, 2012-09-07 at 22:33 +0900, Namhyung Kim wrote:
> 2012-09-07 (금), 11:28 +0200, Jiri Olsa:
> > On Fri, Sep 07, 2012 at 02:58:19PM +0900, Namhyung Kim wrote:
> > > I don't see why this do { } while(0) loop is necessary.
> > > How about this?
> > >
> > > w1 = strtol(opt, &tmp, 10);
> > > if (*tmp != ',')
> > > goto error;
> > >
> > > opt = tmp + 1;
> > > w2 = strtol(opt, &tmp, 10);
> > > if (*tmp != '\0')
> > > goto error;
> > >
> > > if (!w1 || !w2)
> > > goto error;
> >
> > I do this not to use labels & goto ;)
>
> But isn't it usual? Do you have a reason not to do it?
>
> I was a bit confused finding which path actually make it a loop... but
> there's none. :-/
I fully agree, please use goto, there's nothing wrong with them, esp. if
not using them obfuscates the code.
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 09/12] perf diff: Add weighted diff computation way to compare hist entries
2012-09-07 13:33 ` Namhyung Kim
2012-09-07 15:26 ` Peter Zijlstra
@ 2012-09-07 15:31 ` Arnaldo Carvalho de Melo
2012-09-07 16:08 ` Peter Zijlstra
1 sibling, 1 reply; 40+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-09-07 15:31 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
Em Fri, Sep 07, 2012 at 10:33:04PM +0900, Namhyung Kim escreveu:
> 2012-09-07 (금), 11:28 +0200, Jiri Olsa:
> > On Fri, Sep 07, 2012 at 02:58:19PM +0900, Namhyung Kim wrote:
> > > I don't see why this do { } while(0) loop is necessary.
> > > How about this?
> > >
> > > w1 = strtol(opt, &tmp, 10);
> > > if (*tmp != ',')
> > > goto error;
> > >
> > > opt = tmp + 1;
> > > w2 = strtol(opt, &tmp, 10);
> > > if (*tmp != '\0')
> > > goto error;
> > >
> > > if (!w1 || !w2)
> > > goto error;
> >
> > I do this not to use labels & goto ;)
>
> But isn't it usual? Do you have a reason not to do it?
People don't like goto's, but that is overstated, for error handling it
is perfectly fine :-)
> I was a bit confused finding which path actually make it a loop... but
> there's none. :-/
- Arnaldo
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 09/12] perf diff: Add weighted diff computation way to compare hist entries
2012-09-07 15:31 ` Arnaldo Carvalho de Melo
@ 2012-09-07 16:08 ` Peter Zijlstra
0 siblings, 0 replies; 40+ messages in thread
From: Peter Zijlstra @ 2012-09-07 16:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Namhyung Kim, Jiri Olsa, linux-kernel, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
On Fri, 2012-09-07 at 08:31 -0700, Arnaldo Carvalho de Melo wrote:
> People don't like goto's, but that is overstated, for error handling
> it
> is perfectly fine :-)
http://marc.info/?l=linux-arch&m=120852974023791&w=2
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 10/12] perf diff: Add -p option to display period values for hist entries
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (8 preceding siblings ...)
2012-09-06 15:47 ` [PATCH 09/12] perf diff: Add weighted diff computation way to compare hist entries Jiri Olsa
@ 2012-09-06 15:47 ` Jiri Olsa
2012-09-06 15:47 ` [PATCH 11/12] perf diff: Add -F option to display formula for computation Jiri Olsa
` (3 subsequent siblings)
13 siblings, 0 replies; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:47 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
Adding -p option to show period values for both compared hist
entries - showing hist columns HISTC_TOTAL_PERIOD and new stdio
hist column HISTC_BASELINE_TOTAL_PERIOD.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 4 ++++
tools/perf/builtin-diff.c | 9 +++++++++
tools/perf/ui/stdio/hist.c | 18 ++++++++++++++++++
tools/perf/util/hist.h | 1 +
4 files changed, 32 insertions(+)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index fa413ac..21cc2ef 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -83,6 +83,10 @@ OPTIONS
on the computation results.
See COMPARISON METHODS section for more info.
+-p::
+--period::
+ Show period values for both compared hist entries.
+
COMPARISON METHODS
------------------
delta
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 6d8aba8..1297a01 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -25,6 +25,7 @@ static char const *input_old = "perf.data.old",
static char diff__default_sort_order[] = "dso,symbol";
static bool force;
static bool show_displacement;
+static bool show_period;
static bool show_baseline_only;
static bool sort_compute;
@@ -526,6 +527,8 @@ static const struct option options[] = {
OPT_STRING('c', "compute", &compute_str,
"delta,ratio,wdiff:w1,w2 (default delta)",
"Entries differential computation selection"),
+ OPT_BOOLEAN('p', "period", &show_period,
+ "Show period values."),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
"dump raw trace in ASCII"),
OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
@@ -568,6 +571,12 @@ static void setup_ui_stdio(void)
if (show_displacement)
hists_stdio_column__register_idx(HISTC_DISPLACEMENT);
+
+ if (show_period) {
+ hists_stdio_column__register_idx(HISTC_BASELINE_TOTAL_PERIOD);
+ hists_stdio_column__register_idx(HISTC_TOTAL_PERIOD);
+ }
+
}
int cmd_diff(int argc, const char **argv, const char *prefix __used)
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index f580085..ab5f27a 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -93,6 +93,23 @@ hists_stdio_column__total_period_snprintf(struct hist_entry *he, char *bf,
}
static int
+hists_stdio_column__baseline_total_period_snprintf(struct hist_entry *he,
+ char *bf, size_t size,
+ unsigned int width __used)
+{
+ struct hist_entry *pair = he->pair;
+ u64 period = pair ? pair->period : 0;
+ int ret;
+
+ if (period)
+ ret = scnprintf(bf, size , "%15" PRIu64, period);
+ else
+ ret = scnprintf(bf, size, "%15s", " ");
+
+ return ret;
+}
+
+static int
hists_stdio_column__cpu_sys_snprintf(struct hist_entry *he, char *bf,
size_t size, unsigned int width __used)
{
@@ -153,6 +170,7 @@ DEF_COLUMN(cpu_guest_sys, HISTC_CPU_UTILIZATION_GUEST_SYS, 8, "guest sys")
DEF_COLUMN(cpu_guest_us, HISTC_CPU_UTILIZATION_GUEST_US, 8, "guest us")
DEF_COLUMN(nr_samples, HISTC_NR_SAMPLES, 12, "Samples")
DEF_COLUMN(total_period, HISTC_TOTAL_PERIOD, 12, "Period")
+DEF_COLUMN(baseline_total_period, HISTC_BASELINE_TOTAL_PERIOD, 15, "Baseline Period")
DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
DEF_COLUMN(ratio, HISTC_RATIO, 14, "Ratio")
DEF_COLUMN(wdiff, HISTC_WEIGHTED_DIFF, 13, "Weighted diff")
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 745e0cc..81e6d20 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -45,6 +45,7 @@ enum hist_column {
HISTC_CPU_UTILIZATION_GUEST_US,
HISTC_NR_SAMPLES,
HISTC_TOTAL_PERIOD,
+ HISTC_BASELINE_TOTAL_PERIOD,
HISTC_DELTA,
HISTC_RATIO,
HISTC_WEIGHTED_DIFF,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [PATCH 11/12] perf diff: Add -F option to display formula for computation
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (9 preceding siblings ...)
2012-09-06 15:47 ` [PATCH 10/12] perf diff: Add -p option to display period values for " Jiri Olsa
@ 2012-09-06 15:47 ` Jiri Olsa
2012-09-07 6:02 ` Namhyung Kim
2012-09-06 15:47 ` [PATCH 12/12] perf diff: Add -F option for ratio computation Jiri Olsa
` (2 subsequent siblings)
13 siblings, 1 reply; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:47 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
Adding -F option to display the formula for specified computation.
This is mainly to facilitate debuging, but can be usefull anyway.
Adding this support for weighted diff computation.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 4 ++++
tools/perf/builtin-diff.c | 27 ++++++++++++++++++++++++++-
tools/perf/ui/stdio/hist.c | 8 ++++++++
tools/perf/ui/stdio/hist.h | 1 +
tools/perf/util/hist.h | 3 ++-
5 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 21cc2ef..4d65407 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -87,6 +87,10 @@ OPTIONS
--period::
Show period values for both compared hist entries.
+-F::
+--formula::
+ Show formula for given computation (supported: wdiff)
+
COMPARISON METHODS
------------------
delta
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 1297a01..9125eee 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -26,6 +26,7 @@ static char diff__default_sort_order[] = "dso,symbol";
static bool force;
static bool show_displacement;
static bool show_period;
+static bool show_formula;
static bool show_baseline_only;
static bool sort_compute;
@@ -174,6 +175,20 @@ double perf_diff__compute_wdiff(struct hist_entry *he)
return he->diff.wdiff;
}
+int perf_diff__formula_wdiff(struct hist_entry *he, char *bf, size_t size)
+{
+ struct hist_entry *pair = he->pair;
+ u64 new_period = he->period;
+ u64 old_period = pair ? pair->period : 0;
+ char formula[100];
+
+ scnprintf(formula, 100,
+ "(%" PRIu64 " * " "%" PRId64 ") - (%" PRIu64 " * " "%" PRId64 ")",
+ new_period, compute_wdiff_w2, old_period, compute_wdiff_w1);
+
+ return scnprintf(bf, size, "%-50s", formula);
+}
+
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
{
@@ -529,6 +544,8 @@ static const struct option options[] = {
"Entries differential computation selection"),
OPT_BOOLEAN('p', "period", &show_period,
"Show period values."),
+ OPT_BOOLEAN('F', "formula", &show_formula,
+ "Show formula."),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
"dump raw trace in ASCII"),
OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
@@ -572,11 +589,19 @@ static void setup_ui_stdio(void)
if (show_displacement)
hists_stdio_column__register_idx(HISTC_DISPLACEMENT);
+ if (show_formula)
+ switch (compute) {
+ case COMPUTE_WEIGHTED_DIFF:
+ hists_stdio_column__register_idx(HISTC_FORMULA_WEIGHTED_DIFF);
+ break;
+ default:
+ break;
+ };
+
if (show_period) {
hists_stdio_column__register_idx(HISTC_BASELINE_TOTAL_PERIOD);
hists_stdio_column__register_idx(HISTC_TOTAL_PERIOD);
}
-
}
int cmd_diff(int argc, const char **argv, const char *prefix __used)
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index ab5f27a..8cf4ebd 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -150,6 +150,13 @@ hists_stdio_column__displacement_snprintf(struct hist_entry *he, char *bf,
return scnprintf(bf, size, displ ? "%+5ld" : " ", displ);
}
+static int
+hists_stdio_column__formula_wdiff_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ return perf_diff__formula_wdiff(he, bf, size);
+}
+
LIST_HEAD(hists_stdio_column__list);
#define DEF_COLUMN(name, c, w, h) \
@@ -175,6 +182,7 @@ DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
DEF_COLUMN(ratio, HISTC_RATIO, 14, "Ratio")
DEF_COLUMN(wdiff, HISTC_WEIGHTED_DIFF, 13, "Weighted diff")
DEF_COLUMN(displacement, HISTC_DISPLACEMENT, 5, "Displ")
+DEF_COLUMN(formula_wdiff, HISTC_FORMULA_WEIGHTED_DIFF, 50, "Formula")
};
int hists_stdio_column__register_idx(int idx)
diff --git a/tools/perf/ui/stdio/hist.h b/tools/perf/ui/stdio/hist.h
index f725189..4f62224 100644
--- a/tools/perf/ui/stdio/hist.h
+++ b/tools/perf/ui/stdio/hist.h
@@ -21,4 +21,5 @@ double perf_diff__compute_delta(struct hist_entry *he);
double perf_diff__compute_ratio(struct hist_entry *he);
double perf_diff__compute_wdiff(struct hist_entry *he);
+int perf_diff__formula_wdiff(struct hist_entry *he, char *bf, size_t size);
#endif /* __PERF_UI_STDIO_HIST_H */
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 81e6d20..e1511cc 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -50,6 +50,7 @@ enum hist_column {
HISTC_RATIO,
HISTC_WEIGHTED_DIFF,
HISTC_DISPLACEMENT,
+ HISTC_FORMULA_WEIGHTED_DIFF,
/* sorted (hist_entry__sort_list) */
HISTC_SYMBOL,
@@ -67,7 +68,7 @@ enum hist_column {
HISTC_NR_COLS, /* Last entry */
/* Last stdio ui data column */
- HISTC_STDIO_NR_COLS = HISTC_DISPLACEMENT + 1,
+ HISTC_STDIO_NR_COLS = HISTC_FORMULA_WEIGHTED_DIFF + 1,
};
struct thread;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [PATCH 11/12] perf diff: Add -F option to display formula for computation
2012-09-06 15:47 ` [PATCH 11/12] perf diff: Add -F option to display formula for computation Jiri Olsa
@ 2012-09-07 6:02 ` Namhyung Kim
2012-09-07 9:30 ` Jiri Olsa
0 siblings, 1 reply; 40+ messages in thread
From: Namhyung Kim @ 2012-09-07 6:02 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
On Thu, 6 Sep 2012 17:47:05 +0200, Jiri Olsa wrote:
> Adding -F option to display the formula for specified computation.
> This is mainly to facilitate debuging, but can be usefull anyway.
>
> Adding this support for weighted diff computation.
>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
> tools/perf/Documentation/perf-diff.txt | 4 ++++
> tools/perf/builtin-diff.c | 27 ++++++++++++++++++++++++++-
> tools/perf/ui/stdio/hist.c | 8 ++++++++
> tools/perf/ui/stdio/hist.h | 1 +
> tools/perf/util/hist.h | 3 ++-
> 5 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
> index 21cc2ef..4d65407 100644
> --- a/tools/perf/Documentation/perf-diff.txt
> +++ b/tools/perf/Documentation/perf-diff.txt
> @@ -87,6 +87,10 @@ OPTIONS
> --period::
> Show period values for both compared hist entries.
>
> +-F::
> +--formula::
> + Show formula for given computation (supported: wdiff)
> +
> COMPARISON METHODS
> ------------------
> delta
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index 1297a01..9125eee 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -26,6 +26,7 @@ static char diff__default_sort_order[] = "dso,symbol";
> static bool force;
> static bool show_displacement;
> static bool show_period;
> +static bool show_formula;
> static bool show_baseline_only;
> static bool sort_compute;
>
> @@ -174,6 +175,20 @@ double perf_diff__compute_wdiff(struct hist_entry *he)
> return he->diff.wdiff;
> }
>
> +int perf_diff__formula_wdiff(struct hist_entry *he, char *bf, size_t size)
> +{
> + struct hist_entry *pair = he->pair;
> + u64 new_period = he->period;
> + u64 old_period = pair ? pair->period : 0;
> + char formula[100];
> +
> + scnprintf(formula, 100,
> + "(%" PRIu64 " * " "%" PRId64 ") - (%" PRIu64 " * " "%" PRId64 ")",
> + new_period, compute_wdiff_w2, old_period, compute_wdiff_w1);
> +
> + return scnprintf(bf, size, "%-50s", formula);
> +}
> +
> static int hists__add_entry(struct hists *self,
> struct addr_location *al, u64 period)
> {
> @@ -529,6 +544,8 @@ static const struct option options[] = {
> "Entries differential computation selection"),
> OPT_BOOLEAN('p', "period", &show_period,
> "Show period values."),
> + OPT_BOOLEAN('F', "formula", &show_formula,
> + "Show formula."),
> OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
> "dump raw trace in ASCII"),
> OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
> @@ -572,11 +589,19 @@ static void setup_ui_stdio(void)
> if (show_displacement)
> hists_stdio_column__register_idx(HISTC_DISPLACEMENT);
>
> + if (show_formula)
> + switch (compute) {
> + case COMPUTE_WEIGHTED_DIFF:
> + hists_stdio_column__register_idx(HISTC_FORMULA_WEIGHTED_DIFF);
> + break;
> + default:
> + break;
> + };
> +
> if (show_period) {
> hists_stdio_column__register_idx(HISTC_BASELINE_TOTAL_PERIOD);
> hists_stdio_column__register_idx(HISTC_TOTAL_PERIOD);
> }
> -
> }
>
> int cmd_diff(int argc, const char **argv, const char *prefix __used)
> diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
> index ab5f27a..8cf4ebd 100644
> --- a/tools/perf/ui/stdio/hist.c
> +++ b/tools/perf/ui/stdio/hist.c
> @@ -150,6 +150,13 @@ hists_stdio_column__displacement_snprintf(struct hist_entry *he, char *bf,
> return scnprintf(bf, size, displ ? "%+5ld" : " ", displ);
> }
>
> +static int
> +hists_stdio_column__formula_wdiff_snprintf(struct hist_entry *he, char *bf,
> + size_t size, unsigned int width __used)
> +{
> + return perf_diff__formula_wdiff(he, bf, size);
> +}
> +
> LIST_HEAD(hists_stdio_column__list);
>
> #define DEF_COLUMN(name, c, w, h) \
> @@ -175,6 +182,7 @@ DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
> DEF_COLUMN(ratio, HISTC_RATIO, 14, "Ratio")
> DEF_COLUMN(wdiff, HISTC_WEIGHTED_DIFF, 13, "Weighted diff")
> DEF_COLUMN(displacement, HISTC_DISPLACEMENT, 5, "Displ")
> +DEF_COLUMN(formula_wdiff, HISTC_FORMULA_WEIGHTED_DIFF, 50, "Formula")
This fixed 50 column width looks too large to me. What about make it
dynamic like sort_list?
Thanks,
Namhyung
> };
>
> int hists_stdio_column__register_idx(int idx)
> diff --git a/tools/perf/ui/stdio/hist.h b/tools/perf/ui/stdio/hist.h
> index f725189..4f62224 100644
> --- a/tools/perf/ui/stdio/hist.h
> +++ b/tools/perf/ui/stdio/hist.h
> @@ -21,4 +21,5 @@ double perf_diff__compute_delta(struct hist_entry *he);
> double perf_diff__compute_ratio(struct hist_entry *he);
> double perf_diff__compute_wdiff(struct hist_entry *he);
>
> +int perf_diff__formula_wdiff(struct hist_entry *he, char *bf, size_t size);
> #endif /* __PERF_UI_STDIO_HIST_H */
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 81e6d20..e1511cc 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -50,6 +50,7 @@ enum hist_column {
> HISTC_RATIO,
> HISTC_WEIGHTED_DIFF,
> HISTC_DISPLACEMENT,
> + HISTC_FORMULA_WEIGHTED_DIFF,
>
> /* sorted (hist_entry__sort_list) */
> HISTC_SYMBOL,
> @@ -67,7 +68,7 @@ enum hist_column {
> HISTC_NR_COLS, /* Last entry */
>
> /* Last stdio ui data column */
> - HISTC_STDIO_NR_COLS = HISTC_DISPLACEMENT + 1,
> + HISTC_STDIO_NR_COLS = HISTC_FORMULA_WEIGHTED_DIFF + 1,
> };
>
> struct thread;
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [PATCH 11/12] perf diff: Add -F option to display formula for computation
2012-09-07 6:02 ` Namhyung Kim
@ 2012-09-07 9:30 ` Jiri Olsa
0 siblings, 0 replies; 40+ messages in thread
From: Jiri Olsa @ 2012-09-07 9:30 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern
On Fri, Sep 07, 2012 at 03:02:37PM +0900, Namhyung Kim wrote:
> On Thu, 6 Sep 2012 17:47:05 +0200, Jiri Olsa wrote:
> > Adding -F option to display the formula for specified computation.
> > This is mainly to facilitate debuging, but can be usefull anyway.
> >
SNIP
> > @@ -175,6 +182,7 @@ DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
> > DEF_COLUMN(ratio, HISTC_RATIO, 14, "Ratio")
> > DEF_COLUMN(wdiff, HISTC_WEIGHTED_DIFF, 13, "Weighted diff")
> > DEF_COLUMN(displacement, HISTC_DISPLACEMENT, 5, "Displ")
> > +DEF_COLUMN(formula_wdiff, HISTC_FORMULA_WEIGHTED_DIFF, 50, "Formula")
>
> This fixed 50 column width looks too large to me. What about make it
> dynamic like sort_list?
ok, good idea
jirka
^ permalink raw reply [flat|nested] 40+ messages in thread
* [PATCH 12/12] perf diff: Add -F option for ratio computation
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (10 preceding siblings ...)
2012-09-06 15:47 ` [PATCH 11/12] perf diff: Add -F option to display formula for computation Jiri Olsa
@ 2012-09-06 15:47 ` Jiri Olsa
2012-09-06 17:31 ` [RFC 00/12] perf diff: Factor diff command Jiri Olsa
2012-09-06 18:41 ` Peter Zijlstra
13 siblings, 0 replies; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 15:47 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim,
Jiri Olsa
The -F option displays the formula for specified computation.
Adding this support for weighted diff computation.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 2 +-
tools/perf/builtin-diff.c | 19 +++++++++++++++++++
tools/perf/ui/stdio/hist.c | 8 ++++++++
tools/perf/ui/stdio/hist.h | 1 +
tools/perf/util/hist.h | 1 +
5 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 4d65407..1d416a5 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -89,7 +89,7 @@ OPTIONS
-F::
--formula::
- Show formula for given computation (supported: wdiff)
+ Show formula for given computation (supported: wdiff,ratio)
COMPARISON METHODS
------------------
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 9125eee..62ceda3 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -158,6 +158,22 @@ double perf_diff__compute_ratio(struct hist_entry *he)
return he->diff.period_ratio;
}
+int perf_diff__formula_ratio(struct hist_entry *he, char *bf, size_t size)
+{
+ struct hist_entry *pair = he->pair;
+ double new_period = he->period;
+ double old_period = pair ? pair->period : 0;
+ char formula[100];
+
+ if (pair)
+ scnprintf(formula, 100, "%.0F / %.0F",
+ new_period, old_period);
+ else
+ *formula = 0x0;
+
+ return scnprintf(bf, size, "%-50s", formula);
+}
+
double perf_diff__compute_wdiff(struct hist_entry *he)
{
struct hist_entry *pair = he->pair;
@@ -594,6 +610,9 @@ static void setup_ui_stdio(void)
case COMPUTE_WEIGHTED_DIFF:
hists_stdio_column__register_idx(HISTC_FORMULA_WEIGHTED_DIFF);
break;
+ case COMPUTE_RATIO:
+ hists_stdio_column__register_idx(HISTC_FORMULA_RATIO);
+ break;
default:
break;
};
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 8cf4ebd..c7ca7ed 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -157,6 +157,13 @@ hists_stdio_column__formula_wdiff_snprintf(struct hist_entry *he, char *bf,
return perf_diff__formula_wdiff(he, bf, size);
}
+static int
+hists_stdio_column__formula_ratio_snprintf(struct hist_entry *he, char *bf,
+ size_t size, unsigned int width __used)
+{
+ return perf_diff__formula_ratio(he, bf, size);
+}
+
LIST_HEAD(hists_stdio_column__list);
#define DEF_COLUMN(name, c, w, h) \
@@ -182,6 +189,7 @@ DEF_COLUMN(delta, HISTC_DELTA, 8, "Delta")
DEF_COLUMN(ratio, HISTC_RATIO, 14, "Ratio")
DEF_COLUMN(wdiff, HISTC_WEIGHTED_DIFF, 13, "Weighted diff")
DEF_COLUMN(displacement, HISTC_DISPLACEMENT, 5, "Displ")
+DEF_COLUMN(formula_ratio, HISTC_FORMULA_RATIO, 50, "Formula")
DEF_COLUMN(formula_wdiff, HISTC_FORMULA_WEIGHTED_DIFF, 50, "Formula")
};
diff --git a/tools/perf/ui/stdio/hist.h b/tools/perf/ui/stdio/hist.h
index 4f62224..dba1e64 100644
--- a/tools/perf/ui/stdio/hist.h
+++ b/tools/perf/ui/stdio/hist.h
@@ -21,5 +21,6 @@ double perf_diff__compute_delta(struct hist_entry *he);
double perf_diff__compute_ratio(struct hist_entry *he);
double perf_diff__compute_wdiff(struct hist_entry *he);
+int perf_diff__formula_ratio(struct hist_entry *he, char *bf, size_t size);
int perf_diff__formula_wdiff(struct hist_entry *he, char *bf, size_t size);
#endif /* __PERF_UI_STDIO_HIST_H */
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index e1511cc..faa77ce 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -50,6 +50,7 @@ enum hist_column {
HISTC_RATIO,
HISTC_WEIGHTED_DIFF,
HISTC_DISPLACEMENT,
+ HISTC_FORMULA_RATIO,
HISTC_FORMULA_WEIGHTED_DIFF,
/* sorted (hist_entry__sort_list) */
--
1.7.11.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [RFC 00/12] perf diff: Factor diff command
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (11 preceding siblings ...)
2012-09-06 15:47 ` [PATCH 12/12] perf diff: Add -F option for ratio computation Jiri Olsa
@ 2012-09-06 17:31 ` Jiri Olsa
2012-09-06 18:41 ` Peter Zijlstra
13 siblings, 0 replies; 40+ messages in thread
From: Jiri Olsa @ 2012-09-06 17:31 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim
forgot to mention git repo:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/linux.git
perf/diff8 branch
jirka
On Thu, Sep 06, 2012 at 05:46:54PM +0200, Jiri Olsa wrote:
> hi,
> this patchset factors the perf diff command to be usable for
> differential profiling following paper from Paul McKenney:
> (thanks to Arnaldo for sharing it with me).
>
> http://www2.rdrop.com/users/paulmck/scalability/paper/profiling.2002.06.04.pdf
>
> The 'perf diff' and 'std/hist' code is now changed to allow computations
> mentioned in the paper. Two of them are implemented within this patchset:
> 1) ratio differential profiling
> 2) weighted differential profiling
>
> The standard ratio delta computation stays as default.
>
> To sum it up:
> - perf diff displays output for matching event pairs within 2 given perf.data files
> - stdio ui code is factored to allow easy insertion of new data column
> - added perf diff '-b' option to display only matched hist entries
> (hist entries found in both files)
> - added perf diff '-c' option to choose diff computation,
> support for:
> delta: the current default one
> ratio: ratio differential profile
> wdiff: weighted differential profile
> - added perf diff '-c+' option to sort entries based on the computation data
> - added perf diff '-F' option to show formula used to compute the data
> - added perf diff '-p' option to display hist entries periods
>
>
> Attached patches:
> 01/12 perf diff: Make diff command work with evsel hists
> 02/12 perf tools: Replace sort's standalone field_sep with symbol_conf.field_sep
> 03/12 perf hists: Add struct hists pointer to struct hist_entry
> 04/12 perf diff: Refactor diff displacement possition info
> 05/12 perf diff: Refactor stdio ui data columns output
> 06/12 perf diff: Add -b option for perf diff to display paired entries only
> 07/12 perf diff: Add ratio computation way to compare hist entries
> 08/12 perf diff: Add option to sort entries based on diff computation
> 09/12 perf diff: Add weighted diff computation way to compare hist entries
> 10/12 perf diff: Add -p option to display period values for hist entries
> 11/12 perf diff: Add -F option to display formula for computation
> 12/12 perf diff: Add -F option for ratio computation
>
>
> I'm still testing this, trying to find out useful outputs/computations/options,
> so looking for any ideas and recommendations ;)
>
> thanks,
> jirka
>
>
> Eamples:
>
> display default profile
> -----------------------------------------------------------------------------------
> $ ./perf diff
> # Event 'cache-misses:u'
> #
> # Baseline Delta Shared Object Symbol
> # ........ ........ .................. .................................
> #
> 0.00% +63.54% libc-2.15.so [.] __dcigettext
> 0.00% +5.38% libc-2.15.so [.] _dl_addr
> 0.00% +5.30% libc-2.15.so [.] __register_atfork
> 0.31% +3.94% [kernel.kallsyms] [k] page_fault
> 0.00% +4.07% ld-2.15.so [.] check_match.11335
> 0.00% +3.65% ld-2.15.so [.] version_check_doit
> 0.00% +3.56% ld-2.15.so [.] _dl_fixup
> 0.00% +3.05% ld-2.15.so [.] _dl_map_object
> 0.00% +2.90% [kernel.kallsyms] [k] system_call
> 3.94% -1.53% [kernel.kallsyms] [k] device_not_available
> 0.00% +1.21% libc-2.15.so [.] __GI___libc_write
> 0.00% +0.54% libc-2.15.so [.] __memcpy_ssse3_back
> 0.00% +0.11% libc-2.15.so [.] execvp
> 7.71% -7.69% ld-2.15.so [.] _dl_start
> 0.03% -0.02% libpthread-2.15.so [.] __read_nocancel
> 0.20% -0.18% perf [.] perf_evlist__prepare_workload
>
>
>
> display ratio profile
> -----------------------------------------------------------------------------------
> $ ./perf diff -cratio
> # Event 'cache-misses:u'
> #
> # Baseline Ratio Shared Object Symbol
> # ........ .............. .................. .................................
> #
> 0.00% 0.000 libc-2.15.so [.] __dcigettext
> 0.00% 0.000 libc-2.15.so [.] _dl_addr
> 0.00% 0.000 libc-2.15.so [.] __register_atfork
> 0.31% 15.450 [kernel.kallsyms] [k] page_fault
> 0.00% 0.000 ld-2.15.so [.] check_match.11335
> 0.00% 0.000 ld-2.15.so [.] version_check_doit
> 0.00% 0.000 ld-2.15.so [.] _dl_fixup
> 0.00% 0.000 ld-2.15.so [.] _dl_map_object
> 0.00% 0.000 [kernel.kallsyms] [k] system_call
> 3.94% 0.678 [kernel.kallsyms] [k] device_not_available
> 0.00% 0.000 libc-2.15.so [.] __GI___libc_write
> 0.00% 0.000 libc-2.15.so [.] __memcpy_ssse3_back
> 0.00% 0.000 libc-2.15.so [.] execvp
> 7.71% 0.002 ld-2.15.so [.] _dl_start
> 0.03% 0.500 libpthread-2.15.so [.] __read_nocancel
> 0.20% 0.077 perf [.] perf_evlist__prepare_workload
>
>
>
> display ratio profile only with entries matched in both files
> -----------------------------------------------------------------------------------
> $ ./perf diff -cratio -b
>
> # Event 'cache-misses:u'
> #
> # Baseline Ratio Shared Object Symbol
> # ........ .............. .................. .................................
> #
> 0.31% 15.450 [kernel.kallsyms] [k] page_fault
> 3.94% 0.678 [kernel.kallsyms] [k] device_not_available
> 7.71% 0.002 ld-2.15.so [.] _dl_start
> 0.03% 0.500 libpthread-2.15.so [.] __read_nocancel
> 0.20% 0.077 perf [.] perf_evlist__prepare_workload
>
>
>
> display ratio profile only with entries matched in both files and sorted
> -----------------------------------------------------------------------------------
> $ ./perf diff -c+ratio -b
>
> # Event 'cache-misses:u'
> #
> # Baseline Ratio Shared Object Symbol
> # ........ .............. .................. .................................
> #
> 0.31% 15.450 [kernel.kallsyms] [k] page_fault
> 3.94% 0.678 [kernel.kallsyms] [k] device_not_available
> 0.03% 0.500 libpthread-2.15.so [.] __read_nocancel
> 0.20% 0.077 perf [.] perf_evlist__prepare_workload
> 7.71% 0.002 ld-2.15.so [.] _dl_start
>
>
>
> display weighted profile with weights w1=1 w2=2, with formula, sorted, matching
> entries only and with periods displayed
> -----------------------------------------------------------------------------------
> $ ./perf diff -c+wdiff:1,2 -F -b -p
>
> # Baseline Weighted diff Formula Baseline Period Period Shared Object Symbol
> # ........ ............. .................................................. ............... ............ .................. .................................
> #
> 0.31% +598 (309 * 2) - (20 * 1) 20 309 [kernel.kallsyms] [k] page_fault
> 3.94% +92 (175 * 2) - (258 * 1) 258 175 [kernel.kallsyms] [k] device_not_available
> 0.03% +0 (1 * 2) - (2 * 1) 2 1 libpthread-2.15.so [.] __read_nocancel
> 0.20% -11 (1 * 2) - (13 * 1) 13 1 perf [.] perf_evlist__prepare_workload
> 7.71% -503 (1 * 2) - (505 * 1) 505 1 ld-2.15.so [.] _dl_start
>
>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
>
> ---
> tools/perf/Documentation/perf-diff.txt | 63 ++++++++++
> tools/perf/builtin-diff.c | 488 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
> tools/perf/builtin-report.c | 6 +-
> tools/perf/builtin-top.c | 6 +-
> tools/perf/ui/stdio/hist.c | 574 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------
> tools/perf/ui/stdio/hist.h | 26 ++++
> tools/perf/util/evsel.h | 7 ++
> tools/perf/util/hist.c | 7 +-
> tools/perf/util/hist.h | 24 +++-
> tools/perf/util/session.h | 4 +-
> tools/perf/util/sort.c | 6 +-
> tools/perf/util/sort.h | 22 +++-
> 12 files changed, 957 insertions(+), 276 deletions(-)
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC 00/12] perf diff: Factor diff command
2012-09-06 15:46 [RFC 00/12] perf diff: Factor diff command Jiri Olsa
` (12 preceding siblings ...)
2012-09-06 17:31 ` [RFC 00/12] perf diff: Factor diff command Jiri Olsa
@ 2012-09-06 18:41 ` Peter Zijlstra
2012-09-06 21:25 ` Paul E. McKenney
13 siblings, 1 reply; 40+ messages in thread
From: Peter Zijlstra @ 2012-09-06 18:41 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim
On Thu, 2012-09-06 at 17:46 +0200, Jiri Olsa wrote:
> The 'perf diff' and 'std/hist' code is now changed to allow computations
> mentioned in the paper. Two of them are implemented within this patchset:
> 1) ratio differential profiling
> 2) weighted differential profiling
Seems like a useful thing indeed, the explanation of the weighted diff
method doesn't seem to contain a why. I know I could go read the paper
but... :-)
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [RFC 00/12] perf diff: Factor diff command
2012-09-06 18:41 ` Peter Zijlstra
@ 2012-09-06 21:25 ` Paul E. McKenney
2012-09-07 7:05 ` Peter Zijlstra
0 siblings, 1 reply; 40+ messages in thread
From: Paul E. McKenney @ 2012-09-06 21:25 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Jiri Olsa, linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Andi Kleen,
David Ahern, Namhyung Kim
On Thu, Sep 06, 2012 at 08:41:09PM +0200, Peter Zijlstra wrote:
> On Thu, 2012-09-06 at 17:46 +0200, Jiri Olsa wrote:
> > The 'perf diff' and 'std/hist' code is now changed to allow computations
> > mentioned in the paper. Two of them are implemented within this patchset:
> > 1) ratio differential profiling
> > 2) weighted differential profiling
>
> Seems like a useful thing indeed, the explanation of the weighted diff
> method doesn't seem to contain a why. I know I could go read the paper
> but... :-)
Or you could ask the author. ;-)
Ratio can be fooled by statistical variations on profiling buckets with
few counts. So if you are looking for a 10% difference in execution
overhead somewhere in a large program, ratio will unhelpfully sort a
bunch of statistical 2x or 3x noise to the top of the list.
So you could use the difference in buckets instead of the ratio, but this
has problems in the case where the two runs being compared got different
amounts of work done, as is usually the case for timed benchmark runs
or throughput-based benchmark runs. In these cases, you use the work
done (or the measured throughput, as the case may be) as weights.
The weighted difference will then pinpoint the code that suffered the
greatest per-unit-work increase in overhead between the two runs.
But ratio is still the method of choice when you are looking at pure
scalability issues, especially when the offending code increased in
overhead by a large factor.
Thanx, Paul
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [RFC 00/12] perf diff: Factor diff command
2012-09-06 21:25 ` Paul E. McKenney
@ 2012-09-07 7:05 ` Peter Zijlstra
0 siblings, 0 replies; 40+ messages in thread
From: Peter Zijlstra @ 2012-09-07 7:05 UTC (permalink / raw)
To: paulmck
Cc: Jiri Olsa, linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Andi Kleen,
David Ahern, Namhyung Kim
On Thu, 2012-09-06 at 14:25 -0700, Paul E. McKenney wrote:
> On Thu, Sep 06, 2012 at 08:41:09PM +0200, Peter Zijlstra wrote:
> > On Thu, 2012-09-06 at 17:46 +0200, Jiri Olsa wrote:
> > > The 'perf diff' and 'std/hist' code is now changed to allow computations
> > > mentioned in the paper. Two of them are implemented within this patchset:
> > > 1) ratio differential profiling
> > > 2) weighted differential profiling
> >
> > Seems like a useful thing indeed, the explanation of the weighted diff
> > method doesn't seem to contain a why. I know I could go read the paper
> > but... :-)
>
> Or you could ask the author. ;-)
>
> Ratio can be fooled by statistical variations on profiling buckets with
> few counts. So if you are looking for a 10% difference in execution
> overhead somewhere in a large program, ratio will unhelpfully sort a
> bunch of statistical 2x or 3x noise to the top of the list.
>
> So you could use the difference in buckets instead of the ratio, but this
> has problems in the case where the two runs being compared got different
> amounts of work done, as is usually the case for timed benchmark runs
> or throughput-based benchmark runs. In these cases, you use the work
> done (or the measured throughput, as the case may be) as weights.
> The weighted difference will then pinpoint the code that suffered the
> greatest per-unit-work increase in overhead between the two runs.
Ah, ok, I guess that makes sense. Thanks!
^ permalink raw reply [flat|nested] 40+ messages in thread