* [PATCH/RFC 00/14] perf, tool: Fix endian issues
@ 2012-11-28 13:52 Jiri Olsa
2012-11-28 13:52 ` [PATCH 01/14] perf tool: Introduce perf_hpp__list for period related columns Jiri Olsa
` (14 more replies)
0 siblings, 15 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
hi,
adding support to display diff for more than 2 perf.data files.
Sending as RFC, since the change touches lot of hists code,
so I might be breaking something I missed.. still testing.
Also it could colide with Namhyung changes for group report
wrt patches 7 and 8, where I changed the linking of matching
hists entries.
The doc was updated with info about current perf diff processing.
Attached patches:
01/14 perf tool: Introduce perf_hpp__list for period related columns
02/14 perf tool: Add struct perf_hpp_fmt into hpp callbacks
03/14 perf tool: Fix period symbol_conf.field_sep display
04/14 perf diff: Remove displacement from struct hist_entry_diff
05/14 perf diff: Change compute methods to work with pair directly
06/14 perf diff: Change formula methods to work with pair directly
07/14 perf diff: Add callback to hists__match/hists__link functions
08/14 perf diff: Change diff command to work over multiple data files
09/14 perf diff: Update perf diff documentation for multiple data comparison
10/14 perf tool: Centralize default columns init in perf_hpp__init
11/14 perf diff: Making compute functions static
12/14 perf diff: Display data file info ahead of the diff output
13/14 perf diff: Display zero calculation results
14/14 perf diff: Add generic order option for compute sorting
Example of multiple perf diff output:
$ perf diff perf.data.[123456]
# Event 'cycles:u'
#
# Data files:
# [0] perf.data.1
# [1] perf.data.2
# [2] perf.data.3
# [3] perf.data.4
# [4] perf.data.5
# [5] perf.data.6
#
# Baseline/0 Delta/1 Delta/2 Delta/3 Delta/4 Delta/5 Shared Object Symbol
# .......... ....... ....... ....... ....... ....... ................. .........................
#
+73.05% [kernel.kallsyms] [k] page_fault
+26.16% ld-2.15.so [.] _dl_next_ld_env_entry
+15.40% ld-2.15.so [.] _dl_sysdep_start
71.48% -26.52% libc-2.15.so [.] __strcmp_sse2
27.69% -1.65% -4.83% -4.68% ld-2.15.so [.] dl_main
0.82% -0.05% -0.34% -0.15% -0.13% -0.03% ld-2.15.so [.] _start
+39.15% libc-2.15.so [.] error_tail
+73.18% libc-2.15.so [.] __strcasecmp_l_sse2
+76.30% libc-2.15.so [.] __stpcpy_sse2
+76.46% libc-2.15.so [.] _IO_getline_info
thanks,
jirka
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-diff.txt | 77 ++++++++--
tools/perf/builtin-diff.c | 747 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------
tools/perf/ui/browsers/hists.c | 22 +--
tools/perf/ui/gtk/browser.c | 26 +---
tools/perf/ui/hist.c | 392 ++++++++++++++---------------------------------
tools/perf/ui/stdio/hist.c | 17 +--
tools/perf/util/hist.c | 29 ++--
tools/perf/util/hist.h | 42 ++---
tools/perf/util/sort.h | 38 ++---
9 files changed, 855 insertions(+), 535 deletions(-)
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 01/14] perf tool: Introduce perf_hpp__list for period related columns
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-29 7:44 ` Namhyung Kim
2012-11-28 13:52 ` [PATCH 02/14] perf tool: Add struct perf_hpp_fmt into hpp callbacks Jiri Olsa
` (13 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Adding perf_hpp__list list to register and contain all period
related columns the command is interested in.
This way we get rid of static array holding all possible
columns and enable commands to register their own columns.
It'll be handy for diff command in future to process and display
data for multiple files.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 21 ++++-----
tools/perf/builtin-report.c | 1 +
tools/perf/ui/browsers/hists.c | 18 +++++---
tools/perf/ui/gtk/browser.c | 28 +++++-------
tools/perf/ui/hist.c | 96 +++++++++++++++++++++++-------------------
tools/perf/ui/setup.c | 1 +
tools/perf/ui/stdio/hist.c | 17 +++-----
tools/perf/util/hist.h | 11 ++++-
8 files changed, 99 insertions(+), 94 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 93b852f..9fbbc01 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -597,40 +597,35 @@ static const struct option options[] = {
static void ui_init(void)
{
- perf_hpp__init();
-
- /* No overhead column. */
- perf_hpp__column_enable(PERF_HPP__OVERHEAD, false);
-
/*
* Display baseline/delta/ratio/displacement/
* formula/periods columns.
*/
- perf_hpp__column_enable(PERF_HPP__BASELINE, true);
+ perf_hpp__column_enable(PERF_HPP__BASELINE);
switch (compute) {
case COMPUTE_DELTA:
- perf_hpp__column_enable(PERF_HPP__DELTA, true);
+ perf_hpp__column_enable(PERF_HPP__DELTA);
break;
case COMPUTE_RATIO:
- perf_hpp__column_enable(PERF_HPP__RATIO, true);
+ perf_hpp__column_enable(PERF_HPP__RATIO);
break;
case COMPUTE_WEIGHTED_DIFF:
- perf_hpp__column_enable(PERF_HPP__WEIGHTED_DIFF, true);
+ perf_hpp__column_enable(PERF_HPP__WEIGHTED_DIFF);
break;
default:
BUG_ON(1);
};
if (show_displacement)
- perf_hpp__column_enable(PERF_HPP__DISPL, true);
+ perf_hpp__column_enable(PERF_HPP__DISPL);
if (show_formula)
- perf_hpp__column_enable(PERF_HPP__FORMULA, true);
+ perf_hpp__column_enable(PERF_HPP__FORMULA);
if (show_period) {
- perf_hpp__column_enable(PERF_HPP__PERIOD, true);
- perf_hpp__column_enable(PERF_HPP__PERIOD_BASELINE, true);
+ perf_hpp__column_enable(PERF_HPP__PERIOD);
+ perf_hpp__column_enable(PERF_HPP__PERIOD_BASELINE);
}
}
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index fc25100..5134acf 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -692,6 +692,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
setup_browser(true);
else {
use_browser = 0;
+ perf_hpp__column_enable(PERF_HPP__OVERHEAD);
perf_hpp__init();
}
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index ccc4bd1..96fdbc8 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -587,6 +587,8 @@ HPP__COLOR_FN(overhead_guest_us, period_guest_us)
void hist_browser__init_hpp(void)
{
+ perf_hpp__column_enable(PERF_HPP__OVERHEAD);
+
perf_hpp__init();
perf_hpp__format[PERF_HPP__OVERHEAD].color =
@@ -607,12 +609,13 @@ static int hist_browser__show_entry(struct hist_browser *browser,
{
char s[256];
double percent;
- int i, printed = 0;
+ int printed = 0;
int width = browser->b.width;
char folded_sign = ' ';
bool current_entry = ui_browser__is_current_entry(&browser->b, row);
off_t row_offset = entry->row_offset;
bool first = true;
+ struct perf_hpp_fmt *fmt;
if (current_entry) {
browser->he_selection = entry;
@@ -629,12 +632,11 @@ static int hist_browser__show_entry(struct hist_browser *browser,
.buf = s,
.size = sizeof(s),
};
+ int i = 0;
ui_browser__gotorc(&browser->b, row, 0);
- for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
- if (!perf_hpp__format[i].cond)
- continue;
+ perf_hpp__for_each_format(fmt) {
if (!first) {
slsmg_printf(" ");
@@ -642,10 +644,10 @@ static int hist_browser__show_entry(struct hist_browser *browser,
}
first = false;
- if (perf_hpp__format[i].color) {
+ if (fmt->color) {
hpp.ptr = &percent;
/* It will set percent for us. See HPP__COLOR_FN above. */
- width -= perf_hpp__format[i].color(&hpp, entry);
+ width -= fmt->color(&hpp, entry);
ui_browser__set_percent_color(&browser->b, percent, current_entry);
@@ -659,9 +661,11 @@ static int hist_browser__show_entry(struct hist_browser *browser,
if (!current_entry || !browser->b.navkeypressed)
ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
} else {
- width -= perf_hpp__format[i].entry(&hpp, entry);
+ width -= fmt->entry(&hpp, entry);
slsmg_printf("%s", s);
}
+
+ i++;
}
/* The scroll bar isn't being used */
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 253b621..c4aac7b 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -74,6 +74,8 @@ HPP__COLOR_FN(overhead_guest_us, period_guest_us)
void perf_gtk__init_hpp(void)
{
+ perf_hpp__column_enable(PERF_HPP__OVERHEAD);
+
perf_hpp__init();
perf_hpp__format[PERF_HPP__OVERHEAD].color =
@@ -90,6 +92,7 @@ void perf_gtk__init_hpp(void)
static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
{
+ struct perf_hpp_fmt *fmt;
GType col_types[MAX_COLUMNS];
GtkCellRenderer *renderer;
struct sort_entry *se;
@@ -107,12 +110,8 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
nr_cols = 0;
- for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
- if (!perf_hpp__format[i].cond)
- continue;
-
+ perf_hpp__for_each_format(fmt)
col_types[nr_cols++] = G_TYPE_STRING;
- }
list_for_each_entry(se, &hist_entry__sort_list, list) {
if (se->elide)
@@ -129,12 +128,8 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
col_idx = 0;
- for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
- if (!perf_hpp__format[i].cond)
- continue;
-
- perf_hpp__format[i].header(&hpp);
-
+ perf_hpp__for_each_format(fmt) {
+ fmt->header(&hpp);
gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
-1, s,
renderer, "markup",
@@ -166,14 +161,11 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
col_idx = 0;
- for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
- if (!perf_hpp__format[i].cond)
- continue;
-
- if (perf_hpp__format[i].color)
- perf_hpp__format[i].color(&hpp, h);
+ perf_hpp__for_each_format(fmt) {
+ if (fmt->color)
+ fmt->color(&hpp, h);
else
- perf_hpp__format[i].entry(&hpp, h);
+ fmt->entry(&hpp, h);
gtk_list_store_set(store, &iter, col_idx++, s, -1);
}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index aa84130..0a5281f 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -386,60 +386,71 @@ static int hpp__entry_formula(struct perf_hpp *hpp, struct hist_entry *he)
return scnprintf(hpp->buf, hpp->size, fmt, buf);
}
-#define HPP__COLOR_PRINT_FNS(_name) \
- .header = hpp__header_ ## _name, \
- .width = hpp__width_ ## _name, \
- .color = hpp__color_ ## _name, \
- .entry = hpp__entry_ ## _name
+#define HPP__COLOR_PRINT_FNS(_name) \
+ { \
+ .header = hpp__header_ ## _name, \
+ .width = hpp__width_ ## _name, \
+ .color = hpp__color_ ## _name, \
+ .entry = hpp__entry_ ## _name \
+ }
-#define HPP__PRINT_FNS(_name) \
- .header = hpp__header_ ## _name, \
- .width = hpp__width_ ## _name, \
- .entry = hpp__entry_ ## _name
+#define HPP__PRINT_FNS(_name) \
+ { \
+ .header = hpp__header_ ## _name, \
+ .width = hpp__width_ ## _name, \
+ .entry = hpp__entry_ ## _name \
+ }
struct perf_hpp_fmt perf_hpp__format[] = {
- { .cond = false, HPP__COLOR_PRINT_FNS(baseline) },
- { .cond = true, HPP__COLOR_PRINT_FNS(overhead) },
- { .cond = false, HPP__COLOR_PRINT_FNS(overhead_sys) },
- { .cond = false, HPP__COLOR_PRINT_FNS(overhead_us) },
- { .cond = false, HPP__COLOR_PRINT_FNS(overhead_guest_sys) },
- { .cond = false, HPP__COLOR_PRINT_FNS(overhead_guest_us) },
- { .cond = false, HPP__PRINT_FNS(samples) },
- { .cond = false, HPP__PRINT_FNS(period) },
- { .cond = false, HPP__PRINT_FNS(period_baseline) },
- { .cond = false, HPP__PRINT_FNS(delta) },
- { .cond = false, HPP__PRINT_FNS(ratio) },
- { .cond = false, HPP__PRINT_FNS(wdiff) },
- { .cond = false, HPP__PRINT_FNS(displ) },
- { .cond = false, HPP__PRINT_FNS(formula) }
+ HPP__COLOR_PRINT_FNS(baseline),
+ HPP__COLOR_PRINT_FNS(overhead),
+ HPP__COLOR_PRINT_FNS(overhead_sys),
+ HPP__COLOR_PRINT_FNS(overhead_us),
+ HPP__COLOR_PRINT_FNS(overhead_guest_sys),
+ HPP__COLOR_PRINT_FNS(overhead_guest_us),
+ HPP__PRINT_FNS(samples),
+ HPP__PRINT_FNS(period),
+ HPP__PRINT_FNS(period_baseline),
+ HPP__PRINT_FNS(delta),
+ HPP__PRINT_FNS(ratio),
+ HPP__PRINT_FNS(wdiff),
+ HPP__PRINT_FNS(displ),
+ HPP__PRINT_FNS(formula)
};
+LIST_HEAD(perf_hpp__list);
+
#undef HPP__COLOR_PRINT_FNS
#undef HPP__PRINT_FNS
void perf_hpp__init(void)
{
if (symbol_conf.show_cpu_utilization) {
- perf_hpp__format[PERF_HPP__OVERHEAD_SYS].cond = true;
- perf_hpp__format[PERF_HPP__OVERHEAD_US].cond = true;
+ perf_hpp__column_enable(PERF_HPP__OVERHEAD_SYS);
+ perf_hpp__column_enable(PERF_HPP__OVERHEAD_US);
if (perf_guest) {
- perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].cond = true;
- perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].cond = true;
+ perf_hpp__column_enable(PERF_HPP__OVERHEAD_GUEST_SYS);
+ perf_hpp__column_enable(PERF_HPP__OVERHEAD_GUEST_US);
}
}
if (symbol_conf.show_nr_samples)
- perf_hpp__format[PERF_HPP__SAMPLES].cond = true;
+ perf_hpp__column_enable(PERF_HPP__SAMPLES);
if (symbol_conf.show_total_period)
- perf_hpp__format[PERF_HPP__PERIOD].cond = true;
+ perf_hpp__column_enable(PERF_HPP__PERIOD);
}
-void perf_hpp__column_enable(unsigned col, bool enable)
+void perf_hpp__column_register(struct perf_hpp_fmt *format)
+{
+ list_add_tail(&format->list, &perf_hpp__list);
+}
+
+void perf_hpp__column_enable(unsigned col)
{
BUG_ON(col >= PERF_HPP__MAX_INDEX);
- perf_hpp__format[col].cond = enable;
+ perf_hpp__column_register(&perf_hpp__format[col]);
}
static inline void advance_hpp(struct perf_hpp *hpp, int inc)
@@ -452,27 +463,25 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
bool color)
{
const char *sep = symbol_conf.field_sep;
+ struct perf_hpp_fmt *fmt;
char *start = hpp->buf;
- int i, ret;
+ int ret;
bool first = true;
if (symbol_conf.exclude_other && !he->parent)
return 0;
- for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
- if (!perf_hpp__format[i].cond)
- continue;
-
+ perf_hpp__for_each_format(fmt) {
if (!sep || !first) {
ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
advance_hpp(hpp, ret);
first = false;
}
- if (color && perf_hpp__format[i].color)
- ret = perf_hpp__format[i].color(hpp, he);
+ if (color && fmt->color)
+ ret = fmt->color(hpp, he);
else
- ret = perf_hpp__format[i].entry(hpp, he);
+ ret = fmt->entry(hpp, he);
advance_hpp(hpp, ret);
}
@@ -504,16 +513,15 @@ int hist_entry__sort_snprintf(struct hist_entry *he, char *s, size_t size,
*/
unsigned int hists__sort_list_width(struct hists *hists)
{
+ struct perf_hpp_fmt *fmt;
struct sort_entry *se;
- int i, ret = 0;
+ int i = 0, ret = 0;
- for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
- if (!perf_hpp__format[i].cond)
- continue;
+ perf_hpp__for_each_format(fmt) {
if (i)
ret += 2;
- ret += perf_hpp__format[i].width(NULL);
+ ret += fmt->width(NULL);
}
list_for_each_entry(se, &hist_entry__sort_list, list)
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index ebb4cc1..166f13d 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -30,6 +30,7 @@ void setup_browser(bool fallback_to_pager)
if (fallback_to_pager)
setup_pager();
+ perf_hpp__column_enable(PERF_HPP__OVERHEAD);
perf_hpp__init();
break;
}
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index f0ee204..0eae3b2 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -335,13 +335,14 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
int max_cols, FILE *fp)
{
+ struct perf_hpp_fmt *fmt;
struct sort_entry *se;
struct rb_node *nd;
size_t ret = 0;
unsigned int width;
const char *sep = symbol_conf.field_sep;
const char *col_width = symbol_conf.col_width_list_str;
- int idx, nr_rows = 0;
+ int nr_rows = 0;
char bf[96];
struct perf_hpp dummy_hpp = {
.buf = bf,
@@ -355,16 +356,14 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
goto print_entries;
fprintf(fp, "# ");
- for (idx = 0; idx < PERF_HPP__MAX_INDEX; idx++) {
- if (!perf_hpp__format[idx].cond)
- continue;
+ perf_hpp__for_each_format(fmt) {
if (!first)
fprintf(fp, "%s", sep ?: " ");
else
first = false;
- perf_hpp__format[idx].header(&dummy_hpp);
+ fmt->header(&dummy_hpp);
fprintf(fp, "%s", bf);
}
@@ -400,18 +399,16 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
first = true;
fprintf(fp, "# ");
- for (idx = 0; idx < PERF_HPP__MAX_INDEX; idx++) {
- unsigned int i;
- if (!perf_hpp__format[idx].cond)
- continue;
+ perf_hpp__for_each_format(fmt) {
+ unsigned int i;
if (!first)
fprintf(fp, "%s", sep ?: " ");
else
first = false;
- width = perf_hpp__format[idx].width(&dummy_hpp);
+ width = fmt->width(&dummy_hpp);
for (i = 0; i < width; i++)
fprintf(fp, ".");
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 8b091a5..a935a60 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -126,13 +126,19 @@ struct perf_hpp {
};
struct perf_hpp_fmt {
- bool cond;
int (*header)(struct perf_hpp *hpp);
int (*width)(struct perf_hpp *hpp);
int (*color)(struct perf_hpp *hpp, struct hist_entry *he);
int (*entry)(struct perf_hpp *hpp, struct hist_entry *he);
+
+ struct list_head list;
};
+extern struct list_head perf_hpp__list;
+
+#define perf_hpp__for_each_format(format) \
+ list_for_each_entry(format, &perf_hpp__list, list)
+
extern struct perf_hpp_fmt perf_hpp__format[];
enum {
@@ -155,7 +161,8 @@ enum {
};
void perf_hpp__init(void);
-void perf_hpp__column_enable(unsigned col, bool enable);
+void perf_hpp__column_register(struct perf_hpp_fmt *format);
+void perf_hpp__column_enable(unsigned col);
int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
bool color);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 02/14] perf tool: Add struct perf_hpp_fmt into hpp callbacks
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
2012-11-28 13:52 ` [PATCH 01/14] perf tool: Introduce perf_hpp__list for period related columns Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-28 13:52 ` [PATCH 03/14] perf tool: Fix period symbol_conf.field_sep display Jiri Olsa
` (12 subsequent siblings)
14 siblings, 0 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Adding 'struct perf_hpp_fmt' into hpp callbacks, so commands
can access their private data.
It'll be handy for diff command in future to be able to access
file related data for each column.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/hists.c | 10 ++-
tools/perf/ui/hist.c | 169 +++++++++++++++++++++++++++--------------
tools/perf/ui/stdio/hist.c | 4 +-
tools/perf/util/hist.h | 10 ++-
4 files changed, 127 insertions(+), 66 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 96fdbc8..4e06ba5 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -568,8 +568,10 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
}
#define HPP__COLOR_FN(_name, _field) \
-static int hist_browser__hpp_color_ ## _name(struct perf_hpp *hpp, \
- struct hist_entry *he) \
+static int hist_browser__hpp_color_ ## _name( \
+ struct perf_hpp_fmt *fmt __maybe_unused, \
+ struct perf_hpp *hpp, \
+ struct hist_entry *he) \
{ \
struct hists *hists = he->hists; \
double percent = 100.0 * he->stat._field / hists->stats.total_period; \
@@ -647,7 +649,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
if (fmt->color) {
hpp.ptr = &percent;
/* It will set percent for us. See HPP__COLOR_FN above. */
- width -= fmt->color(&hpp, entry);
+ width -= fmt->color(fmt, &hpp, entry);
ui_browser__set_percent_color(&browser->b, percent, current_entry);
@@ -661,7 +663,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
if (!current_entry || !browser->b.navkeypressed)
ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
} else {
- width -= fmt->entry(&hpp, entry);
+ width -= fmt->entry(fmt, &hpp, entry);
slsmg_printf("%s", s);
}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 0a5281f..5452960 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -6,17 +6,20 @@
/* hist period print (hpp) functions */
-static int hpp__header_overhead(struct perf_hpp *hpp)
+static int hpp__header_overhead(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
return scnprintf(hpp->buf, hpp->size, "Overhead");
}
-static int hpp__width_overhead(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_overhead(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 8;
}
-static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__color_overhead(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
struct hists *hists = he->hists;
double percent = 100.0 * he->stat.period / hists->stats.total_period;
@@ -24,7 +27,8 @@ static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
}
-static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_overhead(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
struct hists *hists = he->hists;
double percent = 100.0 * he->stat.period / hists->stats.total_period;
@@ -33,19 +37,22 @@ static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
return scnprintf(hpp->buf, hpp->size, fmt, percent);
}
-static int hpp__header_overhead_sys(struct perf_hpp *hpp)
+static int hpp__header_overhead_sys(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
return scnprintf(hpp->buf, hpp->size, fmt, "sys");
}
-static int hpp__width_overhead_sys(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_overhead_sys(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 7;
}
-static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__color_overhead_sys(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
struct hists *hists = he->hists;
double percent = 100.0 * he->stat.period_sys / hists->stats.total_period;
@@ -53,7 +60,8 @@ static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
return percent_color_snprintf(hpp->buf, hpp->size, "%6.2f%%", percent);
}
-static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_overhead_sys(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
struct hists *hists = he->hists;
double percent = 100.0 * he->stat.period_sys / hists->stats.total_period;
@@ -62,19 +70,22 @@ static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
return scnprintf(hpp->buf, hpp->size, fmt, percent);
}
-static int hpp__header_overhead_us(struct perf_hpp *hpp)
+static int hpp__header_overhead_us(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
return scnprintf(hpp->buf, hpp->size, fmt, "user");
}
-static int hpp__width_overhead_us(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_overhead_us(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 7;
}
-static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__color_overhead_us(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
struct hists *hists = he->hists;
double percent = 100.0 * he->stat.period_us / hists->stats.total_period;
@@ -82,7 +93,8 @@ static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
return percent_color_snprintf(hpp->buf, hpp->size, "%6.2f%%", percent);
}
-static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_overhead_us(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
struct hists *hists = he->hists;
double percent = 100.0 * he->stat.period_us / hists->stats.total_period;
@@ -91,18 +103,24 @@ static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
return scnprintf(hpp->buf, hpp->size, fmt, percent);
}
-static int hpp__header_overhead_guest_sys(struct perf_hpp *hpp)
+static int
+hpp__header_overhead_guest_sys(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
return scnprintf(hpp->buf, hpp->size, "guest sys");
}
-static int hpp__width_overhead_guest_sys(struct perf_hpp *hpp __maybe_unused)
+static int
+hpp__width_overhead_guest_sys(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 9;
}
-static int hpp__color_overhead_guest_sys(struct perf_hpp *hpp,
- struct hist_entry *he)
+static int
+hpp__color_overhead_guest_sys(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp,
+ struct hist_entry *he)
{
struct hists *hists = he->hists;
double percent = 100.0 * he->stat.period_guest_sys / hists->stats.total_period;
@@ -110,8 +128,10 @@ static int hpp__color_overhead_guest_sys(struct perf_hpp *hpp,
return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%% ", percent);
}
-static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
- struct hist_entry *he)
+static int
+hpp__entry_overhead_guest_sys(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp,
+ struct hist_entry *he)
{
struct hists *hists = he->hists;
double percent = 100.0 * he->stat.period_guest_sys / hists->stats.total_period;
@@ -120,18 +140,24 @@ static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
return scnprintf(hpp->buf, hpp->size, fmt, percent);
}
-static int hpp__header_overhead_guest_us(struct perf_hpp *hpp)
+static int
+hpp__header_overhead_guest_us(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
return scnprintf(hpp->buf, hpp->size, "guest usr");
}
-static int hpp__width_overhead_guest_us(struct perf_hpp *hpp __maybe_unused)
+static int
+hpp__width_overhead_guest_us(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 9;
}
-static int hpp__color_overhead_guest_us(struct perf_hpp *hpp,
- struct hist_entry *he)
+static int
+hpp__color_overhead_guest_us(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp,
+ struct hist_entry *he)
{
struct hists *hists = he->hists;
double percent = 100.0 * he->stat.period_guest_us / hists->stats.total_period;
@@ -139,8 +165,10 @@ static int hpp__color_overhead_guest_us(struct perf_hpp *hpp,
return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%% ", percent);
}
-static int hpp__entry_overhead_guest_us(struct perf_hpp *hpp,
- struct hist_entry *he)
+static int
+hpp__entry_overhead_guest_us(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp,
+ struct hist_entry *he)
{
struct hists *hists = he->hists;
double percent = 100.0 * he->stat.period_guest_us / hists->stats.total_period;
@@ -149,12 +177,14 @@ static int hpp__entry_overhead_guest_us(struct perf_hpp *hpp,
return scnprintf(hpp->buf, hpp->size, fmt, percent);
}
-static int hpp__header_baseline(struct perf_hpp *hpp)
+static int hpp__header_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
return scnprintf(hpp->buf, hpp->size, "Baseline");
}
-static int hpp__width_baseline(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 8;
}
@@ -175,7 +205,8 @@ static double baseline_percent(struct hist_entry *he)
return percent;
}
-static int hpp__color_baseline(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
double percent = baseline_percent(he);
@@ -185,7 +216,8 @@ static int hpp__color_baseline(struct perf_hpp *hpp, struct hist_entry *he)
return scnprintf(hpp->buf, hpp->size, " ");
}
-static int hpp__entry_baseline(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
double percent = baseline_percent(he);
const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
@@ -196,57 +228,67 @@ static int hpp__entry_baseline(struct perf_hpp *hpp, struct hist_entry *he)
return scnprintf(hpp->buf, hpp->size, " ");
}
-static int hpp__header_samples(struct perf_hpp *hpp)
+static int hpp__header_samples(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%11s";
return scnprintf(hpp->buf, hpp->size, fmt, "Samples");
}
-static int hpp__width_samples(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_samples(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 11;
}
-static int hpp__entry_samples(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_samples(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%11" PRIu64;
return scnprintf(hpp->buf, hpp->size, fmt, he->stat.nr_events);
}
-static int hpp__header_period(struct perf_hpp *hpp)
+static int hpp__header_period(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
return scnprintf(hpp->buf, hpp->size, fmt, "Period");
}
-static int hpp__width_period(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_period(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 12;
}
-static int hpp__entry_period(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_period(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
return scnprintf(hpp->buf, hpp->size, fmt, he->stat.period);
}
-static int hpp__header_period_baseline(struct perf_hpp *hpp)
+static int hpp__header_period_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
return scnprintf(hpp->buf, hpp->size, fmt, "Period Base");
}
-static int hpp__width_period_baseline(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_period_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 12;
}
-static int hpp__entry_period_baseline(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_period_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp,
+ struct hist_entry *he)
{
struct hist_entry *pair = hist_entry__next_pair(he);
u64 period = pair ? pair->stat.period : 0;
@@ -254,19 +296,23 @@ static int hpp__entry_period_baseline(struct perf_hpp *hpp, struct hist_entry *h
return scnprintf(hpp->buf, hpp->size, fmt, period);
}
-static int hpp__header_delta(struct perf_hpp *hpp)
+
+static int hpp__header_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
return scnprintf(hpp->buf, hpp->size, fmt, "Delta");
}
-static int hpp__width_delta(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_delta(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 7;
}
-static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
char buf[32] = " ";
@@ -283,19 +329,22 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
return scnprintf(hpp->buf, hpp->size, fmt, buf);
}
-static int hpp__header_ratio(struct perf_hpp *hpp)
+static int hpp__header_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
return scnprintf(hpp->buf, hpp->size, fmt, "Ratio");
}
-static int hpp__width_ratio(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 14;
}
-static int hpp__entry_ratio(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
char buf[32] = " ";
@@ -312,19 +361,22 @@ static int hpp__entry_ratio(struct perf_hpp *hpp, struct hist_entry *he)
return scnprintf(hpp->buf, hpp->size, fmt, buf);
}
-static int hpp__header_wdiff(struct perf_hpp *hpp)
+static int hpp__header_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
return scnprintf(hpp->buf, hpp->size, fmt, "Weighted diff");
}
-static int hpp__width_wdiff(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 14;
}
-static int hpp__entry_wdiff(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
char buf[32] = " ";
@@ -341,18 +393,20 @@ static int hpp__entry_wdiff(struct perf_hpp *hpp, struct hist_entry *he)
return scnprintf(hpp->buf, hpp->size, fmt, buf);
}
-static int hpp__header_displ(struct perf_hpp *hpp)
+static int hpp__header_displ(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
return scnprintf(hpp->buf, hpp->size, "Displ.");
}
-static int hpp__width_displ(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_displ(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 6;
}
-static int hpp__entry_displ(struct perf_hpp *hpp,
- struct hist_entry *he)
+static int hpp__entry_displ(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
struct hist_entry *pair = hist_entry__next_pair(he);
long displacement = pair ? pair->position - he->position : 0;
@@ -365,19 +419,22 @@ static int hpp__entry_displ(struct perf_hpp *hpp,
return scnprintf(hpp->buf, hpp->size, fmt, buf);
}
-static int hpp__header_formula(struct perf_hpp *hpp)
+static int hpp__header_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%70s";
return scnprintf(hpp->buf, hpp->size, fmt, "Formula");
}
-static int hpp__width_formula(struct perf_hpp *hpp __maybe_unused)
+static int hpp__width_formula(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp __maybe_unused)
{
return 70;
}
-static int hpp__entry_formula(struct perf_hpp *hpp, struct hist_entry *he)
+static int hpp__entry_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%-70s";
char buf[96] = " ";
@@ -479,9 +536,9 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
}
if (color && fmt->color)
- ret = fmt->color(hpp, he);
+ ret = fmt->color(fmt, hpp, he);
else
- ret = fmt->entry(hpp, he);
+ ret = fmt->entry(fmt, hpp, he);
advance_hpp(hpp, ret);
}
@@ -521,7 +578,7 @@ unsigned int hists__sort_list_width(struct hists *hists)
if (i)
ret += 2;
- ret += fmt->width(NULL);
+ ret += fmt->width(fmt, NULL);
}
list_for_each_entry(se, &hist_entry__sort_list, list)
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 0eae3b2..531bc7a 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -363,7 +363,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
else
first = false;
- fmt->header(&dummy_hpp);
+ fmt->header(fmt, &dummy_hpp);
fprintf(fp, "%s", bf);
}
@@ -408,7 +408,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
else
first = false;
- width = fmt->width(&dummy_hpp);
+ width = fmt->width(fmt, &dummy_hpp);
for (i = 0; i < width; i++)
fprintf(fp, ".");
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index a935a60..e899e2d 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -126,10 +126,12 @@ struct perf_hpp {
};
struct perf_hpp_fmt {
- int (*header)(struct perf_hpp *hpp);
- int (*width)(struct perf_hpp *hpp);
- int (*color)(struct perf_hpp *hpp, struct hist_entry *he);
- int (*entry)(struct perf_hpp *hpp, struct hist_entry *he);
+ int (*header)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp);
+ int (*width)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp);
+ int (*color)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+ struct hist_entry *he);
+ int (*entry)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+ struct hist_entry *he);
struct list_head list;
};
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 03/14] perf tool: Fix period symbol_conf.field_sep display
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
2012-11-28 13:52 ` [PATCH 01/14] perf tool: Introduce perf_hpp__list for period related columns Jiri Olsa
2012-11-28 13:52 ` [PATCH 02/14] perf tool: Add struct perf_hpp_fmt into hpp callbacks Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-29 7:56 ` Namhyung Kim
2012-11-28 13:52 ` [PATCH 04/14] perf diff: Remove displacement from struct hist_entry_diff Jiri Olsa
` (11 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Currently we don't properly display hist data with
symbol_conf.field_sep separator. We need to display
either space or separator.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/hist.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 5452960..20a4392 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -523,17 +523,13 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
struct perf_hpp_fmt *fmt;
char *start = hpp->buf;
int ret;
- bool first = true;
if (symbol_conf.exclude_other && !he->parent)
return 0;
perf_hpp__for_each_format(fmt) {
- if (!sep || !first) {
- ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
- advance_hpp(hpp, ret);
- first = false;
- }
+ ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
+ advance_hpp(hpp, ret);
if (color && fmt->color)
ret = fmt->color(fmt, hpp, he);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 04/14] perf diff: Remove displacement from struct hist_entry_diff
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (2 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 03/14] perf tool: Fix period symbol_conf.field_sep display Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-29 8:00 ` Namhyung Kim
2013-01-24 18:50 ` [tip:perf/core] " tip-bot for Jiri Olsa
2012-11-28 13:52 ` [PATCH 05/14] perf diff: Change compute methods to work with pair directly Jiri Olsa
` (10 subsequent siblings)
14 siblings, 2 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Removing displacement from struct hist_entry_diff, because
it's not used. Displacement is not used for sorting, so
there's no reason to pre-calculate it.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/sort.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index b4e8c3b..a884be2 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -55,9 +55,6 @@ struct he_stat {
struct hist_entry_diff {
bool computed;
- /* PERF_HPP__DISPL */
- int displacement;
-
/* PERF_HPP__DELTA */
double period_ratio_delta;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 05/14] perf diff: Change compute methods to work with pair directly
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (3 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 04/14] perf diff: Remove displacement from struct hist_entry_diff Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-29 11:15 ` Namhyung Kim
2013-01-24 18:51 ` [tip:perf/core] " tip-bot for Jiri Olsa
2012-11-28 13:52 ` [PATCH 06/14] perf diff: Change formula " Jiri Olsa
` (9 subsequent siblings)
14 siblings, 2 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Changing compute methods to operate over hist entry and its
pair directly. This makes the code more obvious and readable,
instead of all time checking for pair being != NULL.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 38 +++++++++++++++++---------------------
tools/perf/ui/hist.c | 40 +++++++++++++++++++++++++---------------
tools/perf/util/hist.h | 7 ++++---
3 files changed, 46 insertions(+), 39 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 9fbbc01..342085a 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -146,47 +146,40 @@ static int setup_compute(const struct option *opt, const char *str,
return -EINVAL;
}
-static double get_period_percent(struct hist_entry *he, u64 period)
+double perf_diff__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)
+double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
- double new_percent = get_period_percent(he, he->stat.period);
- double old_percent = pair ? get_period_percent(pair, pair->stat.period) : 0.0;
+ double new_percent = perf_diff__period_percent(he, he->stat.period);
+ double old_percent = perf_diff__period_percent(pair, pair->stat.period);
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)
+double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
double new_period = he->stat.period;
- double old_period = pair ? pair->stat.period : 0;
+ double old_period = pair->stat.period;
he->diff.computed = true;
- he->diff.period_ratio = pair ? (new_period / old_period) : 0;
+ he->diff.period_ratio = new_period / old_period;
return he->diff.period_ratio;
}
-s64 perf_diff__compute_wdiff(struct hist_entry *he)
+s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
u64 new_period = he->stat.period;
- u64 old_period = pair ? pair->stat.period : 0;
+ u64 old_period = pair->stat.period;
he->diff.computed = true;
-
- if (!pair)
- he->diff.wdiff = 0;
- else
- he->diff.wdiff = new_period * compute_wdiff_w2 -
- old_period * compute_wdiff_w1;
+ he->diff.wdiff = new_period * compute_wdiff_w2 -
+ old_period * compute_wdiff_w1;
return he->diff.wdiff;
}
@@ -385,18 +378,21 @@ static void hists__precompute(struct hists *hists)
while (next != NULL) {
struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
+ struct hist_entry *pair = hist_entry__next_pair(he);
next = rb_next(&he->rb_node);
+ if (!pair)
+ continue;
switch (compute) {
case COMPUTE_DELTA:
- perf_diff__compute_delta(he);
+ perf_diff__compute_delta(he, pair);
break;
case COMPUTE_RATIO:
- perf_diff__compute_ratio(he);
+ perf_diff__compute_ratio(he, pair);
break;
case COMPUTE_WEIGHTED_DIFF:
- perf_diff__compute_wdiff(he);
+ perf_diff__compute_wdiff(he, pair);
break;
default:
BUG_ON(1);
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 20a4392..a64c7f9 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -314,14 +314,18 @@ static int hpp__width_delta(struct perf_hpp_fmt *fmt __maybe_unused,
static int hpp__entry_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
struct perf_hpp *hpp, struct hist_entry *he)
{
+ struct hist_entry *pair = hist_entry__next_pair(he);
const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
char buf[32] = " ";
- double diff;
+ double diff = 0.0;
- if (he->diff.computed)
- diff = he->diff.period_ratio_delta;
- else
- diff = perf_diff__compute_delta(he);
+ if (pair) {
+ if (he->diff.computed)
+ diff = he->diff.period_ratio_delta;
+ else
+ diff = perf_diff__compute_delta(he, pair);
+ } else
+ diff = perf_diff__period_percent(he, he->stat.period);
if (fabs(diff) >= 0.01)
scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
@@ -346,14 +350,17 @@ static int hpp__width_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
static int hpp__entry_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
struct perf_hpp *hpp, struct hist_entry *he)
{
+ struct hist_entry *pair = hist_entry__next_pair(he);
const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
char buf[32] = " ";
- double ratio;
+ double ratio = 0.0;
- if (he->diff.computed)
- ratio = he->diff.period_ratio;
- else
- ratio = perf_diff__compute_ratio(he);
+ if (pair) {
+ if (he->diff.computed)
+ ratio = he->diff.period_ratio;
+ else
+ ratio = perf_diff__compute_ratio(he, pair);
+ }
if (ratio > 0.0)
scnprintf(buf, sizeof(buf), "%+14.6F", ratio);
@@ -378,14 +385,17 @@ static int hpp__width_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
static int hpp__entry_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
struct perf_hpp *hpp, struct hist_entry *he)
{
+ struct hist_entry *pair = hist_entry__next_pair(he);
const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
char buf[32] = " ";
- s64 wdiff;
+ s64 wdiff = 0;
- if (he->diff.computed)
- wdiff = he->diff.wdiff;
- else
- wdiff = perf_diff__compute_wdiff(he);
+ if (pair) {
+ if (he->diff.computed)
+ wdiff = he->diff.wdiff;
+ else
+ wdiff = perf_diff__compute_wdiff(he, pair);
+ }
if (wdiff != 0)
scnprintf(buf, sizeof(buf), "%14ld", wdiff);
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index e899e2d..53a1679 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -228,8 +228,9 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist __maybe_unused,
unsigned int hists__sort_list_width(struct hists *self);
-double perf_diff__compute_delta(struct hist_entry *he);
-double perf_diff__compute_ratio(struct hist_entry *he);
-s64 perf_diff__compute_wdiff(struct hist_entry *he);
+double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair);
+double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair);
+s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair);
int perf_diff__formula(char *buf, size_t size, struct hist_entry *he);
+double perf_diff__period_percent(struct hist_entry *he, u64 period);
#endif /* __PERF_HIST_H */
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 06/14] perf diff: Change formula methods to work with pair directly
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (4 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 05/14] perf diff: Change compute methods to work with pair directly Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-29 11:15 ` Namhyung Kim
2013-01-24 18:52 ` [tip:perf/core] " tip-bot for Jiri Olsa
2012-11-28 13:52 ` [PATCH 07/14] perf diff: Add callback to hists__match/hists__link functions Jiri Olsa
` (8 subsequent siblings)
14 siblings, 2 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Changing formula methods to operate over hist entry and its
pair directly. This makes the code more obvious and readable,
instead of all time checking for pair being != NULL.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 35 +++++++++++++----------------------
tools/perf/ui/hist.c | 5 ++++-
tools/perf/util/hist.h | 3 ++-
3 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 342085a..d869029 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -184,13 +184,9 @@ s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair)
return he->diff.wdiff;
}
-static int formula_delta(struct hist_entry *he, char *buf, size_t size)
+static int formula_delta(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
-
- if (!pair)
- return -1;
-
return scnprintf(buf, size,
"(%" PRIu64 " * 100 / %" PRIu64 ") - "
"(%" PRIu64 " * 100 / %" PRIu64 ")",
@@ -198,41 +194,36 @@ static int formula_delta(struct hist_entry *he, char *buf, size_t size)
pair->stat.period, pair->hists->stats.total_period);
}
-static int formula_ratio(struct hist_entry *he, char *buf, size_t size)
+static int formula_ratio(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
double new_period = he->stat.period;
- double old_period = pair ? pair->stat.period : 0;
-
- if (!pair)
- return -1;
+ double old_period = pair->stat.period;
return scnprintf(buf, size, "%.0F / %.0F", new_period, old_period);
}
-static int formula_wdiff(struct hist_entry *he, char *buf, size_t size)
+static int formula_wdiff(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
u64 new_period = he->stat.period;
- u64 old_period = pair ? pair->stat.period : 0;
-
- if (!pair)
- return -1;
+ u64 old_period = pair->stat.period;
return scnprintf(buf, size,
"(%" PRIu64 " * " "%" PRId64 ") - (%" PRIu64 " * " "%" PRId64 ")",
new_period, compute_wdiff_w2, old_period, compute_wdiff_w1);
}
-int perf_diff__formula(char *buf, size_t size, struct hist_entry *he)
+int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size)
{
switch (compute) {
case COMPUTE_DELTA:
- return formula_delta(he, buf, size);
+ return formula_delta(he, pair, buf, size);
case COMPUTE_RATIO:
- return formula_ratio(he, buf, size);
+ return formula_ratio(he, pair, buf, size);
case COMPUTE_WEIGHTED_DIFF:
- return formula_wdiff(he, buf, size);
+ return formula_wdiff(he, pair, buf, size);
default:
BUG_ON(1);
}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index a64c7f9..d6fdb00 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -446,10 +446,13 @@ static int hpp__width_formula(struct perf_hpp_fmt *fmt __maybe_unused,
static int hpp__entry_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
struct perf_hpp *hpp, struct hist_entry *he)
{
+ struct hist_entry *pair = hist_entry__next_pair(he);
const char *fmt = symbol_conf.field_sep ? "%s" : "%-70s";
char buf[96] = " ";
- perf_diff__formula(buf, sizeof(buf), he);
+ if (pair)
+ perf_diff__formula(he, pair, buf, sizeof(buf));
+
return scnprintf(hpp->buf, hpp->size, fmt, buf);
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 53a1679..7f5cce8 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -231,6 +231,7 @@ unsigned int hists__sort_list_width(struct hists *self);
double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair);
double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair);
s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair);
-int perf_diff__formula(char *buf, size_t size, struct hist_entry *he);
+int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size);
double perf_diff__period_percent(struct hist_entry *he, u64 period);
#endif /* __PERF_HIST_H */
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 07/14] perf diff: Add callback to hists__match/hists__link functions
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (5 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 06/14] perf diff: Change formula " Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-28 13:52 ` [PATCH 08/14] perf diff: Change diff command to work over multiple data files Jiri Olsa
` (7 subsequent siblings)
14 siblings, 0 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
It's possible different users of hists__match/hists__link will need
specific processing of matching hists_entry data.
Adding callback to hists__match/hists__link functions to allow that.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 11 +++++++++--
tools/perf/util/hist.c | 24 +++++++++++++++++-------
tools/perf/util/hist.h | 8 ++++++--
3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index d869029..6361b55 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -472,14 +472,21 @@ static void hists__compute_resort(struct hists *hists)
hists->entries = tmp;
}
+static int match_cb(struct hist_entry *a, struct hist_entry *b,
+ void *data __maybe_unused)
+{
+ hist__entry_add_pair(a, b);
+ return 0;
+}
+
static void hists__process(struct hists *old, struct hists *new)
{
- hists__match(new, old);
+ hists__match(new, old, match_cb, NULL);
if (show_baseline_only)
hists__baseline_only(new);
else
- hists__link(new, old);
+ hists__link(new, old, match_cb, NULL);
if (sort_compute) {
hists__precompute(new);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index cb17e2a..0c5843b 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -775,18 +775,24 @@ static struct hist_entry *hists__find_entry(struct hists *hists,
/*
* Look for pairs to link to the leader buckets (hist_entries):
*/
-void hists__match(struct hists *leader, struct hists *other)
+int hists__match(struct hists *leader, struct hists *other,
+ hists__entry_cb cb, void *data)
{
struct rb_node *nd;
struct hist_entry *pos, *pair;
+ int ret = 0;
- for (nd = rb_first(&leader->entries); nd; nd = rb_next(nd)) {
+ BUG_ON(!cb);
+
+ for (nd = rb_first(&leader->entries); nd && !ret; nd = rb_next(nd)) {
pos = rb_entry(nd, struct hist_entry, rb_node);
pair = hists__find_entry(other, pos);
if (pair)
- hist__entry_add_pair(pos, pair);
+ ret = cb(pos, pair, data);
}
+
+ return ret;
}
/*
@@ -794,21 +800,25 @@ void hists__match(struct hists *leader, struct hists *other)
* we find them, just add a dummy entry on the leader hists, with period=0,
* nr_events=0, to serve as the list header.
*/
-int hists__link(struct hists *leader, struct hists *other)
+int hists__link(struct hists *leader, struct hists *other,
+ hists__entry_cb cb, void *data)
{
struct rb_node *nd;
struct hist_entry *pos, *pair;
+ int ret = 0;
- for (nd = rb_first(&other->entries); nd; nd = rb_next(nd)) {
+ BUG_ON(!cb);
+
+ for (nd = rb_first(&other->entries); nd && !ret; nd = rb_next(nd)) {
pos = rb_entry(nd, struct hist_entry, rb_node);
if (!hist_entry__has_pairs(pos)) {
pair = hists__add_dummy_entry(leader, pos);
if (pair == NULL)
return -1;
- hist__entry_add_pair(pair, pos);
+ ret = cb(pos, pair, data);
}
}
- return 0;
+ return ret;
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 7f5cce8..eb4dc83 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -115,8 +115,12 @@ bool hists__new_col_len(struct hists *self, enum hist_column col, u16 len);
void hists__reset_col_len(struct hists *hists);
void hists__calc_col_len(struct hists *hists, struct hist_entry *he);
-void hists__match(struct hists *leader, struct hists *other);
-int hists__link(struct hists *leader, struct hists *other);
+typedef int (hists__entry_cb)(struct hist_entry *a, struct hist_entry *b,
+ void *data);
+int hists__match(struct hists *leader, struct hists *other,
+ hists__entry_cb cb, void *data);
+int hists__link(struct hists *leader, struct hists *other,
+ hists__entry_cb cb, void *data);
struct perf_hpp {
char *buf;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 08/14] perf diff: Change diff command to work over multiple data files
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (6 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 07/14] perf diff: Add callback to hists__match/hists__link functions Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-29 11:37 ` Namhyung Kim
2012-11-28 13:52 ` [PATCH 09/14] perf diff: Update perf diff documentation for multiple data comparison Jiri Olsa
` (6 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Adding diff command the flexibility to specify multiple data
files on input. If not input file is given the standard behaviour
stands and diff inspects 'perf.data' and 'perf.data.old' files.
Also changing the processing and output display for data files.
For 'perf diff A B' command:
- the current way is to iterate over B data and display
A (baseline) data (symbol samples) only if found in B
- this patch iterates over A (baseline) data and display
B data (symbol samples) only if found in A
For 'perf diff A B C' command, the diff now iterates the
A (baseline) data and display B and C data (symbol samples)
only if they found in A (baseline)
All other standard diff command computation features
(delta,ratio,wdiff) stays.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 663 ++++++++++++++++++++++++++++++++++++++--------
tools/perf/ui/hist.c | 242 -----------------
tools/perf/util/hist.c | 6 +-
tools/perf/util/hist.h | 9 +-
tools/perf/util/sort.h | 32 +--
5 files changed, 560 insertions(+), 392 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 6361b55..9f4ef76 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -18,11 +18,49 @@
#include "util/util.h"
#include <stdlib.h>
+#include <math.h>
-static char const *input_old = "perf.data.old",
- *input_new = "perf.data";
-static char diff__default_sort_order[] = "dso,symbol";
-static bool force;
+/* Diff command specific HPP columns. */
+enum {
+ PERF_HPP_DIFF__BASELINE,
+ PERF_HPP_DIFF__PERIOD,
+ PERF_HPP_DIFF__PERIOD_BASELINE,
+ PERF_HPP_DIFF__DELTA,
+ PERF_HPP_DIFF__RATIO,
+ PERF_HPP_DIFF__WEIGHTED_DIFF,
+ PERF_HPP_DIFF__DISPL,
+ PERF_HPP_DIFF__FORMULA,
+
+ PERF_HPP_DIFF__MAX_INDEX
+};
+
+struct diff_data__fmt {
+ struct perf_hpp_fmt fmt;
+ int idx;
+
+ char *header;
+ int header_width;
+};
+
+struct diff_data {
+ struct perf_session *session;
+ const char *file;
+ int idx;
+
+ struct diff_data__fmt fmt[PERF_HPP_DIFF__MAX_INDEX];
+};
+
+static struct diff_data *data;
+static int data_cnt;
+
+#define for_each_data(i, d) \
+ for (i = 0, d = &data[0]; i < data_cnt; i++, d = &data[i])
+
+#define for_each_data_new(i, d) \
+ for (i = 1, d = &data[1]; i < data_cnt; i++, d = &data[i])
+
+static const char diff__default_sort_order[] = "dso,symbol";
+static bool force;
static bool show_displacement;
static bool show_period;
static bool show_formula;
@@ -39,12 +77,55 @@ enum {
COMPUTE_MAX,
};
+static int compute_2_hpp[COMPUTE_MAX] = {
+ [COMPUTE_DELTA] = PERF_HPP_DIFF__DELTA,
+ [COMPUTE_RATIO] = PERF_HPP_DIFF__RATIO,
+ [COMPUTE_WEIGHTED_DIFF] = PERF_HPP_DIFF__WEIGHTED_DIFF,
+};
+
+
const char *compute_names[COMPUTE_MAX] = {
[COMPUTE_DELTA] = "delta",
[COMPUTE_RATIO] = "ratio",
[COMPUTE_WEIGHTED_DIFF] = "wdiff",
};
+static struct header_column {
+ const char *name;
+ int width;
+} columns[PERF_HPP_DIFF__MAX_INDEX] = {
+ [PERF_HPP_DIFF__BASELINE] = {
+ .name = "Baseline",
+ },
+ [PERF_HPP_DIFF__PERIOD] = {
+ .name = "Period",
+ },
+ [PERF_HPP_DIFF__PERIOD_BASELINE] = {
+ .name = "Base period",
+ },
+ [PERF_HPP_DIFF__DELTA] = {
+ .name = "Delta",
+ .width = 7,
+ },
+ [PERF_HPP_DIFF__RATIO] = {
+ .name = "Ratio",
+ .width = 14,
+ },
+ [PERF_HPP_DIFF__WEIGHTED_DIFF] = {
+ .name = "Weighted diff",
+ .width = 14,
+ },
+ [PERF_HPP_DIFF__DISPL] = {
+ .name = "Displ",
+ },
+ [PERF_HPP_DIFF__FORMULA] = {
+ .name = "Formula",
+ .width = 70,
+ }
+};
+
+#define MAX_COL_WIDTH 70
+
static int compute;
static int setup_compute_opt_wdiff(char *opt)
@@ -146,7 +227,7 @@ static int setup_compute(const struct option *opt, const char *str,
return -EINVAL;
}
-double perf_diff__period_percent(struct hist_entry *he, u64 period)
+static double get_period_percent(struct hist_entry *he, u64 period)
{
u64 total = he->hists->stats.total_period;
return (period * 100.0) / total;
@@ -154,34 +235,34 @@ double perf_diff__period_percent(struct hist_entry *he, u64 period)
double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair)
{
- double new_percent = perf_diff__period_percent(he, he->stat.period);
- double old_percent = perf_diff__period_percent(pair, pair->stat.period);
+ double old_percent = get_period_percent(he, he->stat.period);
+ double new_percent = get_period_percent(pair, pair->stat.period);
- he->diff.period_ratio_delta = new_percent - old_percent;
- he->diff.computed = true;
- return he->diff.period_ratio_delta;
+ pair->diff.period_ratio_delta = new_percent - old_percent;
+ pair->diff.computed = true;
+ return pair->diff.period_ratio_delta;
}
double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair)
{
- double new_period = he->stat.period;
- double old_period = pair->stat.period;
+ double old_period = he->stat.period ?: 1;
+ double new_period = pair->stat.period;
- he->diff.computed = true;
- he->diff.period_ratio = new_period / old_period;
- return he->diff.period_ratio;
+ pair->diff.computed = true;
+ pair->diff.period_ratio = (new_period / old_period);
+ return pair->diff.period_ratio;
}
s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair)
{
- u64 new_period = he->stat.period;
- u64 old_period = pair->stat.period;
+ u64 old_period = he->stat.period;
+ u64 new_period = pair->stat.period;
- he->diff.computed = true;
- he->diff.wdiff = new_period * compute_wdiff_w2 -
- old_period * compute_wdiff_w1;
+ pair->diff.computed = true;
+ pair->diff.wdiff = new_period * compute_wdiff_w1 -
+ old_period * compute_wdiff_w2;
- return he->diff.wdiff;
+ return pair->diff.wdiff;
}
static int formula_delta(struct hist_entry *he, struct hist_entry *pair,
@@ -197,8 +278,8 @@ static int formula_delta(struct hist_entry *he, struct hist_entry *pair,
static int formula_ratio(struct hist_entry *he, struct hist_entry *pair,
char *buf, size_t size)
{
- double new_period = he->stat.period;
- double old_period = pair->stat.period;
+ double old_period = he->stat.period;
+ double new_period = pair->stat.period;
return scnprintf(buf, size, "%.0F / %.0F", new_period, old_period);
}
@@ -206,12 +287,12 @@ static int formula_ratio(struct hist_entry *he, struct hist_entry *pair,
static int formula_wdiff(struct hist_entry *he, struct hist_entry *pair,
char *buf, size_t size)
{
- u64 new_period = he->stat.period;
- u64 old_period = pair->stat.period;
+ u64 old_period = he->stat.period;
+ u64 new_period = pair->stat.period;
return scnprintf(buf, size,
"(%" PRIu64 " * " "%" PRId64 ") - (%" PRIu64 " * " "%" PRId64 ")",
- new_period, compute_wdiff_w2, old_period, compute_wdiff_w1);
+ new_period, compute_wdiff_w1, old_period, compute_wdiff_w2);
}
int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair,
@@ -356,22 +437,21 @@ static void hists__baseline_only(struct hists *hists)
struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
next = rb_next(&he->rb_node);
- if (!hist_entry__next_pair(he)) {
+
+ if (!he->pairs) {
rb_erase(&he->rb_node, &hists->entries);
hist_entry__free(he);
}
}
}
-static void hists__precompute(struct hists *hists)
+static void __hists__precompute(struct hist_entry *he)
{
- struct rb_node *next = rb_first(&hists->entries);
+ int i;
- while (next != NULL) {
- struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
- struct hist_entry *pair = hist_entry__next_pair(he);
+ for (i = 0; i < data_cnt; i++) {
+ struct hist_entry *pair = he->pairs[i];
- next = rb_next(&he->rb_node);
if (!pair)
continue;
@@ -391,6 +471,21 @@ static void hists__precompute(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->pairs)
+ __hists__precompute(he);
+ }
+}
+
static int64_t cmp_doubles(double l, double r)
{
if (l > r)
@@ -402,8 +497,8 @@ static int64_t cmp_doubles(double l, double r)
}
static int64_t
-hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
- int c)
+__hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
+ int c)
{
switch (c) {
case COMPUTE_DELTA:
@@ -434,6 +529,39 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
return 0;
}
+static int64_t
+hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
+ int c)
+{
+ int i;
+
+ for (i = 0; i < data_cnt; i++) {
+ struct hist_entry **pairs_left = left->pairs;
+ struct hist_entry **pairs_right = right->pairs;
+ struct hist_entry *p_right, *p_left;
+ static int64_t cmp;
+
+ if (!pairs_left || !pairs_right)
+ return pairs_right - pairs_left;
+
+ p_right = pairs_right[i];
+ p_left = pairs_left[i];
+
+ if (!p_left || !p_right)
+ return p_right - p_left;
+
+ /*
+ * If we differ, we are done, otherwise continue until all
+ * is processed or we find a difference.
+ */
+ cmp = __hist_entry__cmp_compute(p_left, p_right, c);
+ if (cmp)
+ return cmp;
+ }
+
+ return 0;
+}
+
static void insert_hist_entry_by_compute(struct rb_root *root,
struct hist_entry *he,
int c)
@@ -473,83 +601,112 @@ static void hists__compute_resort(struct hists *hists)
}
static int match_cb(struct hist_entry *a, struct hist_entry *b,
- void *data __maybe_unused)
+ void *_data)
{
- hist__entry_add_pair(a, b);
+ intptr_t i = (intptr_t) _data;
+ struct hist_entry **p;
+
+ if (!a->pairs) {
+ p = zalloc(sizeof(p) * data_cnt);
+ if (!p)
+ return -ENOMEM;
+
+ a->pairs = p;
+ }
+
+ a->pairs[i] = b;
+ b->paired = true;
return 0;
}
-static void hists__process(struct hists *old, struct hists *new)
+static void hists__process(struct hists *hists)
{
- hists__match(new, old, match_cb, NULL);
-
if (show_baseline_only)
- hists__baseline_only(new);
- else
- hists__link(new, old, match_cb, NULL);
+ hists__baseline_only(hists);
if (sort_compute) {
- hists__precompute(new);
- hists__compute_resort(new);
+ hists__precompute(hists);
+ hists__compute_resort(hists);
}
- hists__fprintf(new, true, 0, 0, stdout);
+ hists__fprintf(hists, true, 0, 0, stdout);
}
-static int __cmd_diff(void)
+static int data_process(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;
+ struct perf_evlist *evlist_base = data[0].session->evlist;
+ struct perf_evsel *evsel_base;
bool first = true;
- older = perf_session__new(input_old, O_RDONLY, force, false,
- &tool);
- newer = perf_session__new(input_new, O_RDONLY, force, false,
- &tool);
- if (session[0] == NULL || session[1] == NULL)
- return -ENOMEM;
+ list_for_each_entry(evsel_base, &evlist_base->entries, node) {
+ struct diff_data *d;
+ int i;
- for (i = 0; i < 2; ++i) {
- ret = perf_session__process_events(session[i], &tool);
- if (ret)
- goto out_delete;
- }
-
- evlist_old = older->evlist;
- evlist_new = newer->evlist;
+ for_each_data_new(i, d) {
+ struct perf_evlist *evlist = d->session->evlist;
+ struct perf_evsel *evsel_new;
- perf_evlist__resort_hists(evlist_old, true);
- perf_evlist__resort_hists(evlist_new, false);
+ evsel_new = evsel_match(evsel_base, evlist);
+ if (!evsel_new)
+ continue;
- list_for_each_entry(evsel, &evlist_new->entries, node) {
- struct perf_evsel *evsel_old;
+ hists__match(&evsel_base->hists, &evsel_new->hists,
+ match_cb, (void *) (intptr_t) i);
- evsel_old = evsel_match(evsel, evlist_old);
- if (!evsel_old)
- continue;
+ if (!show_baseline_only)
+ hists__link(&evsel_base->hists,
+ &evsel_new->hists,
+ match_cb, (void *) (intptr_t) i);
+ }
fprintf(stdout, "%s# Event '%s'\n#\n", first ? "" : "\n",
- perf_evsel__name(evsel));
+ perf_evsel__name(evsel_base));
+
+ hists__process(&evsel_base->hists);
first = false;
+ }
+
+ return 0;
+}
+
+static int __cmd_diff(void)
+{
+ struct diff_data *d;
+ int ret = -EINVAL, i;
+
+ for_each_data(i, d) {
+ d->session = perf_session__new(d->file, O_RDONLY, force,
+ false, &tool);
+ if (!d->session) {
+ pr_err("Failed to open %s\n", d->file);
+ ret = -ENOMEM;
+ goto out_delete;
+ }
- hists__process(&evsel_old->hists, &evsel->hists);
+ ret = perf_session__process_events(d->session, &tool);
+ if (ret) {
+ pr_err("Failed to process %s\n", d->file);
+ goto out_delete;
+ }
+
+ /* Sort by name all but baseline. */
+ perf_evlist__resort_hists(d->session->evlist, i != 0);
}
+ ret = data_process();
+
out_delete:
- for (i = 0; i < 2; ++i)
- perf_session__delete(session[i]);
+ for_each_data(i, d) {
+ if (d->session)
+ perf_session__delete(d->session);
+ }
+
return ret;
-#undef older
-#undef newer
}
static const char * const diff_usage[] = {
- "perf diff [<options>] [old_file] [new_file]",
+ "perf diff [<options>] <base file> <file1> [file2] ...",
NULL,
};
@@ -589,62 +746,340 @@ static const struct option options[] = {
OPT_END()
};
-static void ui_init(void)
+static double baseline_percent(struct hist_entry *he)
{
- /*
- * Display baseline/delta/ratio/displacement/
- * formula/periods columns.
- */
- perf_hpp__column_enable(PERF_HPP__BASELINE);
+ struct hists *hists = he->hists;
+ u64 total_period = hists->stats.total_period;
+ u64 base_period = he->stat.period;
- switch (compute) {
- case COMPUTE_DELTA:
- perf_hpp__column_enable(PERF_HPP__DELTA);
+ return 100.0 * base_period / total_period;
+}
+
+static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct perf_hpp *hpp, struct hist_entry *he)
+{
+ double percent = baseline_percent(he);
+ struct diff_data__fmt *dfmt =
+ container_of(fmt, struct diff_data__fmt, fmt);
+ char pfmt[20] = " ";
+
+ if (percent) {
+ scnprintf(pfmt, 20, "%%%d.2f%%%%", dfmt->header_width - 1);
+ return percent_color_snprintf(hpp->buf, hpp->size,
+ pfmt, percent);
+ } else
+ return scnprintf(hpp->buf, hpp->size, "%*s",
+ dfmt->header_width, pfmt);
+}
+
+static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt, struct perf_hpp *hpp,
+ struct hist_entry *he)
+{
+ struct diff_data__fmt *dfmt =
+ container_of(_fmt, struct diff_data__fmt, fmt);
+ double percent = baseline_percent(he);
+ const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
+ char buf[32] = " ";
+
+ if ((percent && he->pairs) || symbol_conf.field_sep)
+ return scnprintf(hpp->buf, hpp->size, fmt, percent);
+ else
+ return scnprintf(hpp->buf, hpp->size, "%*s",
+ dfmt->header_width, buf);
+}
+
+static struct hist_entry *get_pair(struct hist_entry *he,
+ struct perf_hpp_fmt *fmt)
+{
+ struct hist_entry *pair = NULL;
+
+ if (he->pairs) {
+ struct diff_data *d;
+ struct diff_data__fmt *dfmt;
+ void *ptr;
+
+ dfmt = container_of(fmt, struct diff_data__fmt, fmt);
+ ptr = dfmt - dfmt->idx;
+ d = container_of(ptr, struct diff_data, fmt);
+ pair = he->pairs[d->idx];
+ }
+
+ return pair;
+}
+
+static void
+hpp__entry_unpair(struct hist_entry *he, int idx, char *buf, size_t size)
+{
+ switch (idx) {
+ case PERF_HPP_DIFF__PERIOD_BASELINE:
+ scnprintf(buf, size, "%" PRIu64, he->stat.period);
break;
- case COMPUTE_RATIO:
- perf_hpp__column_enable(PERF_HPP__RATIO);
+
+ default:
break;
- case COMPUTE_WEIGHTED_DIFF:
- perf_hpp__column_enable(PERF_HPP__WEIGHTED_DIFF);
+ }
+}
+
+static void
+hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
+ int idx, char *buf, size_t size)
+{
+ long displacement;
+ double diff;
+ double ratio;
+ s64 wdiff;
+
+ switch (idx) {
+ case PERF_HPP_DIFF__DELTA:
+ if (pair->diff.computed)
+ diff = pair->diff.period_ratio_delta;
+ else
+ diff = perf_diff__compute_delta(he, pair);
+
+ if (fabs(diff) >= 0.01)
+ scnprintf(buf, size, "%+4.2F%%", diff);
+ break;
+
+ case PERF_HPP_DIFF__RATIO:
+ if (pair->diff.computed)
+ ratio = pair->diff.period_ratio;
+ else
+ ratio = perf_diff__compute_ratio(he, pair);
+
+ if (ratio > 0.0)
+ scnprintf(buf, size, "%14.6F", ratio);
+ break;
+
+ case PERF_HPP_DIFF__WEIGHTED_DIFF:
+ if (pair->diff.computed)
+ wdiff = pair->diff.wdiff;
+ else
+ wdiff = perf_diff__compute_wdiff(he, pair);
+
+ if (wdiff != 0)
+ scnprintf(buf, size, "%14ld", wdiff);
+ break;
+
+ case PERF_HPP_DIFF__DISPL:
+ displacement = pair->position - he->position;
+
+ if (displacement)
+ scnprintf(buf, size, "%+4ld", displacement);
break;
+
+ case PERF_HPP_DIFF__FORMULA:
+ perf_diff__formula(he, pair, buf, size);
+ break;
+
+ case PERF_HPP_DIFF__PERIOD:
+ scnprintf(buf, size, "%" PRIu64, pair->stat.period);
+ break;
+
default:
BUG_ON(1);
- };
+ }
+}
+
+static int hpp__entry_global(struct perf_hpp_fmt *_fmt, struct perf_hpp *hpp,
+ struct hist_entry *he)
+{
+ struct diff_data__fmt *dfmt =
+ container_of(_fmt, struct diff_data__fmt, fmt);
+ struct hist_entry *pair;
+ char buf[MAX_COL_WIDTH] = " ";
+
+ pair = get_pair(he, _fmt);
+
+ if (pair)
+ hpp__entry_pair(he, pair, dfmt->idx, buf, MAX_COL_WIDTH);
+ else
+ hpp__entry_unpair(he, dfmt->idx, buf, MAX_COL_WIDTH);
+
+ if (symbol_conf.field_sep)
+ return scnprintf(hpp->buf, hpp->size, "%s", buf);
+ else
+ return scnprintf(hpp->buf, hpp->size, "%*s",
+ dfmt->header_width, buf);
+}
+
+static int diff_hpp__header(struct perf_hpp_fmt *fmt,
+ struct perf_hpp *hpp)
+{
+ struct diff_data__fmt *dfmt =
+ container_of(fmt, struct diff_data__fmt, fmt);
+
+ BUG_ON(!dfmt->header);
+
+ return scnprintf(hpp->buf, hpp->size, dfmt->header);
+}
+
+static int diff_hpp__width(struct perf_hpp_fmt *fmt,
+ struct perf_hpp *hpp __maybe_unused)
+{
+ struct diff_data__fmt *dfmt =
+ container_of(fmt, struct diff_data__fmt, fmt);
+
+ BUG_ON(dfmt->header_width <= 0);
+
+ return dfmt->header_width;
+}
- if (show_displacement)
- perf_hpp__column_enable(PERF_HPP__DISPL);
+static void init_header(struct diff_data *d, struct diff_data__fmt *dfmt)
+{
+#define MAX_HEADER_NAME 100
+ char buf_indent[MAX_HEADER_NAME];
+ char buf[MAX_HEADER_NAME];
+ const char *header = NULL;
+ int width = 0;
+
+ BUG_ON(dfmt->idx >= PERF_HPP_DIFF__MAX_INDEX);
+ header = columns[dfmt->idx].name;
+ width = columns[dfmt->idx].width;
+
+ /* Only our defined HPP fmts should appear here. */
+ BUG_ON(!header);
+
+ if (data_cnt > 2)
+ scnprintf(buf, MAX_HEADER_NAME, "%s/%d", header, d->idx);
+
+#define NAME (data_cnt > 2 ? buf : header)
+ dfmt->header_width = width;
+ width = (int) strlen(NAME);
+ if (dfmt->header_width < width)
+ dfmt->header_width = width;
+
+ scnprintf(buf_indent, MAX_HEADER_NAME, "%*s",
+ dfmt->header_width, NAME);
+
+ dfmt->header = strdup(buf_indent);
+#undef MAX_HEADER_NAME
+#undef NAME
+}
- if (show_formula)
- perf_hpp__column_enable(PERF_HPP__FORMULA);
+static void data__hpp_register(struct diff_data *d, int idx)
+{
+ struct diff_data__fmt *dfmt = &d->fmt[idx];
+ struct perf_hpp_fmt *fmt = &dfmt->fmt;
- if (show_period) {
- perf_hpp__column_enable(PERF_HPP__PERIOD);
- perf_hpp__column_enable(PERF_HPP__PERIOD_BASELINE);
+ dfmt->idx = idx;
+
+ fmt->header = diff_hpp__header;
+ fmt->width = diff_hpp__width;
+
+ switch (idx) {
+#define CASE_COLOR(c, name) \
+ case c: \
+ fmt->color = hpp__color_##name; \
+ fmt->entry = hpp__entry_##name; \
+ break;
+
+#define CASE(c, name) \
+ case c: \
+ fmt->entry = hpp__entry_##name; \
+ break;
+
+ CASE_COLOR(PERF_HPP_DIFF__BASELINE, baseline)
+ CASE(PERF_HPP_DIFF__PERIOD_BASELINE, global)
+ CASE(PERF_HPP_DIFF__DELTA, global)
+ CASE(PERF_HPP_DIFF__RATIO, global)
+ CASE(PERF_HPP_DIFF__WEIGHTED_DIFF, global)
+ CASE(PERF_HPP_DIFF__DISPL, global)
+ CASE(PERF_HPP_DIFF__FORMULA, global)
+ CASE(PERF_HPP_DIFF__PERIOD, global)
+
+ default:
+ BUG_ON(1);
}
+
+ perf_hpp__column_register(fmt);
+
+ init_header(d, dfmt);
}
-int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
+static void ui_init(void)
{
- sort_order = diff__default_sort_order;
- argc = parse_options(argc, argv, options, diff_usage, 0);
+ struct diff_data *d;
+ int i;
+
+ for_each_data(i, d) {
+
+ /*
+ * Baseline or compute realted columns:
+ *
+ * PERF_HPP_DIFF__BASELINE
+ * PERF_HPP_DIFF__DELTA
+ * PERF_HPP_DIFF__RATIO
+ * PERF_HPP_DIFF__WEIGHTED_DIFF
+ */
+ data__hpp_register(d, i ? compute_2_hpp[compute] :
+ PERF_HPP_DIFF__BASELINE);
+
+ /*
+ * And the rest:
+ *
+ * PERF_HPP_DIFF__DISPL
+ * PERF_HPP_DIFF__FORMULA
+ * PERF_HPP_DIFF__PERIOD
+ * PERF_HPP_DIFF__PERIOD_BASELINE
+ */
+ if (show_displacement && i)
+ data__hpp_register(d, PERF_HPP_DIFF__DISPL);
+
+ if (show_formula && i)
+ data__hpp_register(d, PERF_HPP_DIFF__FORMULA);
+
+ if (show_period)
+ data__hpp_register(d, i ? PERF_HPP_DIFF__PERIOD :
+ PERF_HPP_DIFF__PERIOD_BASELINE);
+ }
+}
+
+static int data_init(int argc, const char **argv)
+{
+ struct diff_data *d;
+ static const char *defaults[] = {
+ "perf.data.old",
+ "perf.data",
+ };
+ int i;
+
+ if (argc == 1)
+ usage_with_options(diff_usage, options);
+
+ data_cnt = 2;
+
if (argc) {
- if (argc > 2)
- usage_with_options(diff_usage, options);
- if (argc == 2) {
- input_old = argv[0];
- input_new = argv[1];
- } else
- input_new = argv[0];
+ data_cnt = argc;
} else if (symbol_conf.default_guest_vmlinux_name ||
symbol_conf.default_guest_kallsyms) {
- input_old = "perf.data.host";
- input_new = "perf.data.guest";
+ defaults[0] = "perf.data.host";
+ defaults[1] = "perf.data.guest";
}
+ data = zalloc(sizeof(*data) * data_cnt);
+ if (!data)
+ return -ENOMEM;
+
+ for_each_data(i, d) {
+ d->file = argc ? argv[i] : defaults[i];
+ d->idx = i;
+ }
+
+ return 0;
+}
+
+int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
+{
+ sort_order = diff__default_sort_order;
+ argc = parse_options(argc, argv, options, diff_usage, 0);
symbol_conf.exclude_other = false;
+
if (symbol__init() < 0)
return -1;
+ if (data_init(argc, argv) < 0)
+ return -1;
+
ui_init();
setup_sorting(diff_usage, options);
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index d6fdb00..a004f57 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -177,57 +177,6 @@ hpp__entry_overhead_guest_us(struct perf_hpp_fmt *_fmt __maybe_unused,
return scnprintf(hpp->buf, hpp->size, fmt, percent);
}
-static int hpp__header_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
- struct perf_hpp *hpp)
-{
- return scnprintf(hpp->buf, hpp->size, "Baseline");
-}
-
-static int hpp__width_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
- struct perf_hpp *hpp __maybe_unused)
-{
- return 8;
-}
-
-static double baseline_percent(struct hist_entry *he)
-{
- struct hist_entry *pair = hist_entry__next_pair(he);
- struct hists *pair_hists = pair ? pair->hists : NULL;
- double percent = 0.0;
-
- if (pair) {
- u64 total_period = pair_hists->stats.total_period;
- u64 base_period = pair->stat.period;
-
- percent = 100.0 * base_period / total_period;
- }
-
- return percent;
-}
-
-static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
- struct perf_hpp *hpp, struct hist_entry *he)
-{
- double percent = baseline_percent(he);
-
- if (hist_entry__has_pairs(he))
- return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
- else
- return scnprintf(hpp->buf, hpp->size, " ");
-}
-
-static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp, struct hist_entry *he)
-{
- double percent = baseline_percent(he);
- const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
-
- if (hist_entry__has_pairs(he) || symbol_conf.field_sep)
- return scnprintf(hpp->buf, hpp->size, fmt, percent);
- else
- return scnprintf(hpp->buf, hpp->size, " ");
-}
-
static int hpp__header_samples(struct perf_hpp_fmt *_fmt __maybe_unused,
struct perf_hpp *hpp)
{
@@ -272,190 +221,6 @@ static int hpp__entry_period(struct perf_hpp_fmt *_fmt __maybe_unused,
return scnprintf(hpp->buf, hpp->size, fmt, he->stat.period);
}
-static int hpp__header_period_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp)
-{
- const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
-
- return scnprintf(hpp->buf, hpp->size, fmt, "Period Base");
-}
-
-static int hpp__width_period_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
- struct perf_hpp *hpp __maybe_unused)
-{
- return 12;
-}
-
-static int hpp__entry_period_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp,
- struct hist_entry *he)
-{
- struct hist_entry *pair = hist_entry__next_pair(he);
- u64 period = pair ? pair->stat.period : 0;
- const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
-
- return scnprintf(hpp->buf, hpp->size, fmt, period);
-}
-
-static int hpp__header_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp)
-{
- const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
-
- return scnprintf(hpp->buf, hpp->size, fmt, "Delta");
-}
-
-static int hpp__width_delta(struct perf_hpp_fmt *fmt __maybe_unused,
- struct perf_hpp *hpp __maybe_unused)
-{
- return 7;
-}
-
-static int hpp__entry_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hist_entry *pair = hist_entry__next_pair(he);
- const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
- char buf[32] = " ";
- double diff = 0.0;
-
- if (pair) {
- if (he->diff.computed)
- diff = he->diff.period_ratio_delta;
- else
- diff = perf_diff__compute_delta(he, pair);
- } else
- diff = perf_diff__period_percent(he, he->stat.period);
-
- if (fabs(diff) >= 0.01)
- scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
-
- return scnprintf(hpp->buf, hpp->size, fmt, buf);
-}
-
-static int hpp__header_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp)
-{
- const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
-
- return scnprintf(hpp->buf, hpp->size, fmt, "Ratio");
-}
-
-static int hpp__width_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp __maybe_unused)
-{
- return 14;
-}
-
-static int hpp__entry_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hist_entry *pair = hist_entry__next_pair(he);
- const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
- char buf[32] = " ";
- double ratio = 0.0;
-
- if (pair) {
- if (he->diff.computed)
- ratio = he->diff.period_ratio;
- else
- ratio = perf_diff__compute_ratio(he, pair);
- }
-
- if (ratio > 0.0)
- scnprintf(buf, sizeof(buf), "%+14.6F", ratio);
-
- return scnprintf(hpp->buf, hpp->size, fmt, buf);
-}
-
-static int hpp__header_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp)
-{
- const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
-
- return scnprintf(hpp->buf, hpp->size, fmt, "Weighted diff");
-}
-
-static int hpp__width_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
- struct perf_hpp *hpp __maybe_unused)
-{
- return 14;
-}
-
-static int hpp__entry_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hist_entry *pair = hist_entry__next_pair(he);
- const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
- char buf[32] = " ";
- s64 wdiff = 0;
-
- if (pair) {
- if (he->diff.computed)
- wdiff = he->diff.wdiff;
- else
- wdiff = perf_diff__compute_wdiff(he, pair);
- }
-
- if (wdiff != 0)
- scnprintf(buf, sizeof(buf), "%14ld", wdiff);
-
- return scnprintf(hpp->buf, hpp->size, fmt, buf);
-}
-
-static int hpp__header_displ(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp)
-{
- return scnprintf(hpp->buf, hpp->size, "Displ.");
-}
-
-static int hpp__width_displ(struct perf_hpp_fmt *fmt __maybe_unused,
- struct perf_hpp *hpp __maybe_unused)
-{
- return 6;
-}
-
-static int hpp__entry_displ(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hist_entry *pair = hist_entry__next_pair(he);
- long displacement = pair ? pair->position - he->position : 0;
- const char *fmt = symbol_conf.field_sep ? "%s" : "%6.6s";
- char buf[32] = " ";
-
- if (displacement)
- scnprintf(buf, sizeof(buf), "%+4ld", displacement);
-
- return scnprintf(hpp->buf, hpp->size, fmt, buf);
-}
-
-static int hpp__header_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp)
-{
- const char *fmt = symbol_conf.field_sep ? "%s" : "%70s";
-
- return scnprintf(hpp->buf, hpp->size, fmt, "Formula");
-}
-
-static int hpp__width_formula(struct perf_hpp_fmt *fmt __maybe_unused,
- struct perf_hpp *hpp __maybe_unused)
-{
- return 70;
-}
-
-static int hpp__entry_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
- struct perf_hpp *hpp, struct hist_entry *he)
-{
- struct hist_entry *pair = hist_entry__next_pair(he);
- const char *fmt = symbol_conf.field_sep ? "%s" : "%-70s";
- char buf[96] = " ";
-
- if (pair)
- perf_diff__formula(he, pair, buf, sizeof(buf));
-
- return scnprintf(hpp->buf, hpp->size, fmt, buf);
-}
-
#define HPP__COLOR_PRINT_FNS(_name) \
{ \
.header = hpp__header_ ## _name, \
@@ -472,7 +237,6 @@ static int hpp__entry_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
}
struct perf_hpp_fmt perf_hpp__format[] = {
- HPP__COLOR_PRINT_FNS(baseline),
HPP__COLOR_PRINT_FNS(overhead),
HPP__COLOR_PRINT_FNS(overhead_sys),
HPP__COLOR_PRINT_FNS(overhead_us),
@@ -480,12 +244,6 @@ struct perf_hpp_fmt perf_hpp__format[] = {
HPP__COLOR_PRINT_FNS(overhead_guest_us),
HPP__PRINT_FNS(samples),
HPP__PRINT_FNS(period),
- HPP__PRINT_FNS(period_baseline),
- HPP__PRINT_FNS(delta),
- HPP__PRINT_FNS(ratio),
- HPP__PRINT_FNS(wdiff),
- HPP__PRINT_FNS(displ),
- HPP__PRINT_FNS(formula)
};
LIST_HEAD(perf_hpp__list);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 0c5843b..25f94a4 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -244,8 +244,6 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
he->ms.map->referenced = true;
if (symbol_conf.use_callchain)
callchain_init(he->callchain);
-
- INIT_LIST_HEAD(&he->pairs.node);
}
return he;
@@ -812,11 +810,11 @@ int hists__link(struct hists *leader, struct hists *other,
for (nd = rb_first(&other->entries); nd && !ret; nd = rb_next(nd)) {
pos = rb_entry(nd, struct hist_entry, rb_node);
- if (!hist_entry__has_pairs(pos)) {
+ if (!pos->paired) {
pair = hists__add_dummy_entry(leader, pos);
if (pair == NULL)
return -1;
- ret = cb(pos, pair, data);
+ ret = cb(pair, pos, data);
}
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index eb4dc83..d4f19eb 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -148,7 +148,7 @@ extern struct list_head perf_hpp__list;
extern struct perf_hpp_fmt perf_hpp__format[];
enum {
- PERF_HPP__BASELINE,
+ /* Matches perf_hpp__format array. */
PERF_HPP__OVERHEAD,
PERF_HPP__OVERHEAD_SYS,
PERF_HPP__OVERHEAD_US,
@@ -156,12 +156,6 @@ enum {
PERF_HPP__OVERHEAD_GUEST_US,
PERF_HPP__SAMPLES,
PERF_HPP__PERIOD,
- PERF_HPP__PERIOD_BASELINE,
- PERF_HPP__DELTA,
- PERF_HPP__RATIO,
- PERF_HPP__WEIGHTED_DIFF,
- PERF_HPP__DISPL,
- PERF_HPP__FORMULA,
PERF_HPP__MAX_INDEX
};
@@ -237,5 +231,4 @@ double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair);
s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair);
int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair,
char *buf, size_t size);
-double perf_diff__period_percent(struct hist_entry *he, u64 period);
#endif /* __PERF_HIST_H */
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index a884be2..377b144 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -74,18 +74,12 @@ struct hist_entry_diff {
struct hist_entry {
struct rb_node rb_node_in;
struct rb_node rb_node;
- union {
- struct list_head node;
- struct list_head head;
- } pairs;
struct he_stat stat;
struct map_symbol ms;
struct thread *thread;
u64 ip;
s32 cpu;
- struct hist_entry_diff diff;
-
/* XXX These two should move to some tree widget lib */
u16 row_offset;
u16 nr_rows;
@@ -98,29 +92,19 @@ struct hist_entry {
struct symbol *parent;
unsigned long position;
struct rb_root sorted_chain;
+
+ /* diff related */
+ union {
+ struct hist_entry **pairs;
+ bool paired;
+ };
+ struct hist_entry_diff diff;
+
struct branch_info *branch_info;
struct hists *hists;
struct callchain_root callchain[0];
};
-static inline bool hist_entry__has_pairs(struct hist_entry *he)
-{
- return !list_empty(&he->pairs.node);
-}
-
-static inline struct hist_entry *hist_entry__next_pair(struct hist_entry *he)
-{
- if (hist_entry__has_pairs(he))
- return list_entry(he->pairs.node.next, struct hist_entry, pairs.node);
- return NULL;
-}
-
-static inline void hist__entry_add_pair(struct hist_entry *he,
- struct hist_entry *pair)
-{
- list_add_tail(&he->pairs.head, &pair->pairs.node);
-}
-
enum sort_type {
SORT_PID,
SORT_COMM,
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 09/14] perf diff: Update perf diff documentation for multiple data comparison
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (7 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 08/14] perf diff: Change diff command to work over multiple data files Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-29 11:52 ` Namhyung Kim
2012-11-28 13:52 ` [PATCH 10/14] perf tool: Centralize default columns init in perf_hpp__init Jiri Olsa
` (5 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Updating perf diff documentation to include multiple perf data
files comparison.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-diff.txt | 73 +++++++++++++++++++++++++++++-----
1 file changed, 64 insertions(+), 9 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 194f37d..ab5b79e 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -3,17 +3,17 @@ perf-diff(1)
NAME
----
-perf-diff - Read two perf.data files and display the differential profile
+perf-diff - Read perf.data files and display the differential profile
SYNOPSIS
--------
[verse]
-'perf diff' [oldfile] [newfile]
+'perf diff' [baseline file] [data file1] [[data file2] ... ]
DESCRIPTION
-----------
-This command displays the performance difference amongst two perf.data files
-captured via perf record.
+This command displays the performance difference amongst two or more perf.data
+files captured via perf record.
If no parameters are passed it will assume perf.data.old and perf.data.
@@ -91,6 +91,59 @@ OPTIONS
--formula::
Show formula for given computation.
+COMPARISON
+----------
+The comparison is governed by the baseline file. The baseline perf.data
+file is iterated for samples. All other perf.data files specified on
+the command line are searched for the baseline sample pair. If the pair
+is found, specified computation is made and result is displayed.
+
+All samples from non-baseline perf.data files, that do not match any
+baseline entry, are displayed with empty space within baseline column
+and possible computation results (delta) in their related column.
+
+Example files samples:
+- file A with samples f1, f2, f3, f4, f6
+- file B with samples f2, f4, f5
+- file C with samples f1, f2, f5
+
+Example output:
+ x - computation takes place for pair
+ b - baseline sample percentage
+
+- perf diff A B C
+
+ baseline/A compute/B compute/C samples
+ ---------------------------------------
+ b x f1
+ b x x f2
+ b f3
+ b x f4
+ b f6
+ x x f5
+
+- perf diff B A C
+
+ baseline/B compute/A compute/C samples
+ ---------------------------------------
+ b x x f2
+ b x f4
+ b x f5
+ x x f1
+ x f3
+ x f6
+
+- perf diff C B A
+
+ baseline/C compute/B compute/A samples
+ ---------------------------------------
+ b x f1
+ b x x f2
+ b x f5
+ x f3
+ x x f4
+ x f6
+
COMPARISON METHODS
------------------
delta
@@ -100,7 +153,7 @@ 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
+ - A/B being matching hist entry from data/baseline file specified
(or perf.data/perf.data.old) respectively.
- period_percent being the % of the hist entry period value within
@@ -113,24 +166,26 @@ 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
+ - A/B being matching hist entry from data/baseline file specified
(or perf.data/perf.data.old) respectively.
- period being the hist entry period value
-wdiff
-~~~~~
+wdiff:WEIGHT-B,WEIGHT-A
+~~~~~~~~~~~~~~~~~~~~~~~
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
+ - A/B being matching hist entry from data/baseline 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'.
+ - WIEGHT-A being the weight of the data file
+ - WIEGHT-B being the weight of the baseline data file
SEE ALSO
--------
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 10/14] perf tool: Centralize default columns init in perf_hpp__init
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (8 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 09/14] perf diff: Update perf diff documentation for multiple data comparison Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-29 11:55 ` Namhyung Kim
2012-11-28 13:52 ` [PATCH 11/14] perf diff: Making compute functions static Jiri Olsa
` (4 subsequent siblings)
14 siblings, 1 reply; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Now when diff command is separated from other standard outputs,
we can use perf_hpp__init to initialize all standard columns.
Moving PERF_HPP__OVERHEAD column init back to perf_hpp__init,
and removing extra enable calls.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-report.c | 1 -
tools/perf/ui/browsers/hists.c | 2 --
tools/perf/ui/gtk/browser.c | 2 --
tools/perf/ui/hist.c | 2 ++
tools/perf/ui/setup.c | 1 -
5 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 5134acf..fc25100 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -692,7 +692,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
setup_browser(true);
else {
use_browser = 0;
- perf_hpp__column_enable(PERF_HPP__OVERHEAD);
perf_hpp__init();
}
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 4e06ba5..9463280 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -589,8 +589,6 @@ HPP__COLOR_FN(overhead_guest_us, period_guest_us)
void hist_browser__init_hpp(void)
{
- perf_hpp__column_enable(PERF_HPP__OVERHEAD);
-
perf_hpp__init();
perf_hpp__format[PERF_HPP__OVERHEAD].color =
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index c4aac7b..489a053 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -74,8 +74,6 @@ HPP__COLOR_FN(overhead_guest_us, period_guest_us)
void perf_gtk__init_hpp(void)
{
- perf_hpp__column_enable(PERF_HPP__OVERHEAD);
-
perf_hpp__init();
perf_hpp__format[PERF_HPP__OVERHEAD].color =
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index a004f57..bec5299 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -253,6 +253,8 @@ LIST_HEAD(perf_hpp__list);
void perf_hpp__init(void)
{
+ perf_hpp__column_enable(PERF_HPP__OVERHEAD);
+
if (symbol_conf.show_cpu_utilization) {
perf_hpp__column_enable(PERF_HPP__OVERHEAD_SYS);
perf_hpp__column_enable(PERF_HPP__OVERHEAD_US);
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index 166f13d..ebb4cc1 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -30,7 +30,6 @@ void setup_browser(bool fallback_to_pager)
if (fallback_to_pager)
setup_pager();
- perf_hpp__column_enable(PERF_HPP__OVERHEAD);
perf_hpp__init();
break;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 11/14] perf diff: Making compute functions static
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (9 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 10/14] perf tool: Centralize default columns init in perf_hpp__init Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-28 13:52 ` [PATCH 12/14] perf diff: Display data file info ahead of the diff output Jiri Olsa
` (3 subsequent siblings)
14 siblings, 0 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
All compute functions are now local to the diff command,
making them static.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 24 ++++++++++++------------
tools/perf/util/hist.h | 6 ------
2 files changed, 12 insertions(+), 18 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 9f4ef76..81f7529 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -233,7 +233,7 @@ static double get_period_percent(struct hist_entry *he, u64 period)
return (period * 100.0) / total;
}
-double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair)
+static double compute_delta(struct hist_entry *he, struct hist_entry *pair)
{
double old_percent = get_period_percent(he, he->stat.period);
double new_percent = get_period_percent(pair, pair->stat.period);
@@ -243,7 +243,7 @@ double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair)
return pair->diff.period_ratio_delta;
}
-double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair)
+static double compute_ratio(struct hist_entry *he, struct hist_entry *pair)
{
double old_period = he->stat.period ?: 1;
double new_period = pair->stat.period;
@@ -253,7 +253,7 @@ double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair)
return pair->diff.period_ratio;
}
-s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair)
+static s64 compute_wdiff(struct hist_entry *he, struct hist_entry *pair)
{
u64 old_period = he->stat.period;
u64 new_period = pair->stat.period;
@@ -295,8 +295,8 @@ static int formula_wdiff(struct hist_entry *he, struct hist_entry *pair,
new_period, compute_wdiff_w1, old_period, compute_wdiff_w2);
}
-int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair,
- char *buf, size_t size)
+static int formula_fprintf(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size)
{
switch (compute) {
case COMPUTE_DELTA:
@@ -457,13 +457,13 @@ static void __hists__precompute(struct hist_entry *he)
switch (compute) {
case COMPUTE_DELTA:
- perf_diff__compute_delta(he, pair);
+ compute_delta(he, pair);
break;
case COMPUTE_RATIO:
- perf_diff__compute_ratio(he, pair);
+ compute_ratio(he, pair);
break;
case COMPUTE_WEIGHTED_DIFF:
- perf_diff__compute_wdiff(he, pair);
+ compute_wdiff(he, pair);
break;
default:
BUG_ON(1);
@@ -834,7 +834,7 @@ hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
if (pair->diff.computed)
diff = pair->diff.period_ratio_delta;
else
- diff = perf_diff__compute_delta(he, pair);
+ diff = compute_delta(he, pair);
if (fabs(diff) >= 0.01)
scnprintf(buf, size, "%+4.2F%%", diff);
@@ -844,7 +844,7 @@ hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
if (pair->diff.computed)
ratio = pair->diff.period_ratio;
else
- ratio = perf_diff__compute_ratio(he, pair);
+ ratio = compute_ratio(he, pair);
if (ratio > 0.0)
scnprintf(buf, size, "%14.6F", ratio);
@@ -854,7 +854,7 @@ hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
if (pair->diff.computed)
wdiff = pair->diff.wdiff;
else
- wdiff = perf_diff__compute_wdiff(he, pair);
+ wdiff = compute_wdiff(he, pair);
if (wdiff != 0)
scnprintf(buf, size, "%14ld", wdiff);
@@ -868,7 +868,7 @@ hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
break;
case PERF_HPP_DIFF__FORMULA:
- perf_diff__formula(he, pair, buf, size);
+ formula_fprintf(he, pair, buf, size);
break;
case PERF_HPP_DIFF__PERIOD:
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index d4f19eb..e00b6b9 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -225,10 +225,4 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist __maybe_unused,
#endif
unsigned int hists__sort_list_width(struct hists *self);
-
-double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair);
-double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair);
-s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair);
-int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair,
- char *buf, size_t size);
#endif /* __PERF_HIST_H */
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 12/14] perf diff: Display data file info ahead of the diff output
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (10 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 11/14] perf diff: Making compute functions static Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-28 13:52 ` [PATCH 13/14] perf diff: Display zero calculation results Jiri Olsa
` (2 subsequent siblings)
14 siblings, 0 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Data files are referenced through the index of the file
on the command line. Adding list of data files for each
index to ease up navigation.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 81f7529..50e1ea3 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -632,6 +632,19 @@ static void hists__process(struct hists *hists)
hists__fprintf(hists, true, 0, 0, stdout);
}
+static void data_fprintf(void)
+{
+ struct diff_data *d;
+ int i;
+
+ fprintf(stdout, "# Data files:\n");
+
+ for_each_data(i, d)
+ fprintf(stdout, "# [%d] %s\n", d->idx, d->file);
+
+ fprintf(stdout, "#\n");
+}
+
static int data_process(void)
{
struct perf_evlist *evlist_base = data[0].session->evlist;
@@ -662,6 +675,9 @@ static int data_process(void)
fprintf(stdout, "%s# Event '%s'\n#\n", first ? "" : "\n",
perf_evsel__name(evsel_base));
+ if (data_cnt > 2)
+ data_fprintf();
+
hists__process(&evsel_base->hists);
first = false;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 13/14] perf diff: Display zero calculation results
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (11 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 12/14] perf diff: Display data file info ahead of the diff output Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-28 13:52 ` [PATCH 14/14] perf diff: Add generic order option for compute sorting Jiri Olsa
2012-11-28 13:56 ` [PATCH/RFC 00/14] perf, diff: Support for multiple files Jiri Olsa
14 siblings, 0 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Forcing zero calculation outputs to appear in final output,
so we can differ between zero output calculation result and
empty space for missing pair of baseline hist_entry.
Also skipping the compute and period output for dummy entries.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 24 +++++++++++++++---------
tools/perf/util/hist.c | 1 +
tools/perf/util/sort.h | 3 +++
3 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 50e1ea3..b801d0c 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -779,13 +779,15 @@ static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
container_of(fmt, struct diff_data__fmt, fmt);
char pfmt[20] = " ";
- if (percent) {
+ if (!he->dummy) {
scnprintf(pfmt, 20, "%%%d.2f%%%%", dfmt->header_width - 1);
return percent_color_snprintf(hpp->buf, hpp->size,
pfmt, percent);
} else
return scnprintf(hpp->buf, hpp->size, "%*s",
dfmt->header_width, pfmt);
+
+ return percent_color_snprintf(hpp->buf, hpp->size, pfmt, percent);
}
static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt, struct perf_hpp *hpp,
@@ -797,7 +799,7 @@ static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt, struct perf_hpp *hpp,
const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
char buf[32] = " ";
- if ((percent && he->pairs) || symbol_conf.field_sep)
+ if ((he->pairs) || symbol_conf.field_sep)
return scnprintf(hpp->buf, hpp->size, fmt, percent);
else
return scnprintf(hpp->buf, hpp->size, "%*s",
@@ -828,7 +830,8 @@ hpp__entry_unpair(struct hist_entry *he, int idx, char *buf, size_t size)
{
switch (idx) {
case PERF_HPP_DIFF__PERIOD_BASELINE:
- scnprintf(buf, size, "%" PRIu64, he->stat.period);
+ if (!he->dummy)
+ scnprintf(buf, size, "%" PRIu64, he->stat.period);
break;
default:
@@ -852,28 +855,31 @@ hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
else
diff = compute_delta(he, pair);
- if (fabs(diff) >= 0.01)
- scnprintf(buf, size, "%+4.2F%%", diff);
+ scnprintf(buf, size, "%+4.2F%%", diff);
break;
case PERF_HPP_DIFF__RATIO:
+ if (he->dummy)
+ break;
+
if (pair->diff.computed)
ratio = pair->diff.period_ratio;
else
ratio = compute_ratio(he, pair);
- if (ratio > 0.0)
- scnprintf(buf, size, "%14.6F", ratio);
+ scnprintf(buf, size, "%14.6F", ratio);
break;
case PERF_HPP_DIFF__WEIGHTED_DIFF:
+ if (he->dummy)
+ break;
+
if (pair->diff.computed)
wdiff = pair->diff.wdiff;
else
wdiff = compute_wdiff(he, pair);
- if (wdiff != 0)
- scnprintf(buf, size, "%14ld", wdiff);
+ scnprintf(buf, size, "%14ld", wdiff);
break;
case PERF_HPP_DIFF__DISPL:
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 25f94a4..531b5dc 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -745,6 +745,7 @@ static struct hist_entry *hists__add_dummy_entry(struct hists *hists,
rb_link_node(&he->rb_node, parent, p);
rb_insert_color(&he->rb_node, &hists->entries);
hists__inc_nr_entries(hists, he);
+ he->dummy = true;
}
out:
return he;
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 377b144..0e06872 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -93,6 +93,9 @@ struct hist_entry {
unsigned long position;
struct rb_root sorted_chain;
+ /* Added by hists__add_dummy_entry via hists__link */
+ bool dummy;
+
/* diff related */
union {
struct hist_entry **pairs;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 14/14] perf diff: Add generic order option for compute sorting
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (12 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 13/14] perf diff: Display zero calculation results Jiri Olsa
@ 2012-11-28 13:52 ` Jiri Olsa
2012-11-29 12:02 ` Namhyung Kim
2012-11-28 13:56 ` [PATCH/RFC 00/14] perf, diff: Support for multiple files Jiri Olsa
14 siblings, 1 reply; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:52 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
Adding option 'o' to allow sorting based on the
input file number.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
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: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-diff.txt | 4 +++
tools/perf/builtin-diff.c | 53 +++++++++++++++-------------------
2 files changed, 28 insertions(+), 29 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index ab5b79e..1402e14 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -91,6 +91,10 @@ OPTIONS
--formula::
Show formula for given computation.
+-o::
+--order::
+ Specify compute sorting column number.
+
COMPARISON
----------
The comparison is governed by the baseline file. The baseline perf.data
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index b801d0c..1f98786 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -65,7 +65,7 @@ static bool show_displacement;
static bool show_period;
static bool show_formula;
static bool show_baseline_only;
-static bool sort_compute;
+static unsigned int sort_compute;
static s64 compute_wdiff_w1;
static s64 compute_wdiff_w2;
@@ -191,13 +191,6 @@ static int setup_compute(const struct option *opt, const char *str,
return 0;
}
- if (*str == '+') {
- sort_compute = true;
- cstr = (char *) ++str;
- if (!*str)
- return 0;
- }
-
option = strchr(str, ':');
if (option) {
unsigned len = option++ - str;
@@ -533,31 +526,27 @@ static int64_t
hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
int c)
{
- int i;
+ struct hist_entry **pairs_left = left->pairs;
+ struct hist_entry **pairs_right = right->pairs;
+ struct hist_entry *p_right, *p_left;
+ static int64_t cmp;
- for (i = 0; i < data_cnt; i++) {
- struct hist_entry **pairs_left = left->pairs;
- struct hist_entry **pairs_right = right->pairs;
- struct hist_entry *p_right, *p_left;
- static int64_t cmp;
+ if (!pairs_left || !pairs_right)
+ return pairs_left ? -1 : 1;
- if (!pairs_left || !pairs_right)
- return pairs_right - pairs_left;
+ p_right = pairs_right[sort_compute];
+ p_left = pairs_left[sort_compute];
- p_right = pairs_right[i];
- p_left = pairs_left[i];
+ if (!p_left || !p_right)
+ return p_left ? -1 : 1;
- if (!p_left || !p_right)
- return p_right - p_left;
-
- /*
- * If we differ, we are done, otherwise continue until all
- * is processed or we find a difference.
- */
- cmp = __hist_entry__cmp_compute(p_left, p_right, c);
- if (cmp)
- return cmp;
- }
+ /*
+ * If we differ, we are done, otherwise continue until all
+ * is processed or we find a difference.
+ */
+ cmp = __hist_entry__cmp_compute(p_left, p_right, c);
+ if (cmp)
+ return cmp;
return 0;
}
@@ -759,6 +748,7 @@ static const struct option options[] = {
"columns '.' is reserved."),
OPT_STRING(0, "symfs", &symbol_conf.symfs, "directory",
"Look for files with symbols relative to this directory"),
+ OPT_UINTEGER('o', "order", &sort_compute, "Specify compute sorting."),
OPT_END()
};
@@ -1087,6 +1077,11 @@ static int data_init(int argc, const char **argv)
d->idx = i;
}
+ if (sort_compute >= (unsigned int) data_cnt) {
+ pr_err("Order option out of limit.\n");
+ return -EINVAL;
+ }
+
return 0;
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH/RFC 00/14] perf, diff: Support for multiple files
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
` (13 preceding siblings ...)
2012-11-28 13:52 ` [PATCH 14/14] perf diff: Add generic order option for compute sorting Jiri Olsa
@ 2012-11-28 13:56 ` Jiri Olsa
2012-11-28 15:57 ` Jiri Olsa
14 siblings, 1 reply; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 13:56 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
aargh.. fixed subject.. copy&paste error ;-)
also patches are available here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/linux.git
perf/multi2
thanks,
jirka
On Wed, Nov 28, 2012 at 02:52:35PM +0100, Jiri Olsa wrote:
> hi,
> adding support to display diff for more than 2 perf.data files.
> Sending as RFC, since the change touches lot of hists code,
> so I might be breaking something I missed.. still testing.
>
> Also it could colide with Namhyung changes for group report
> wrt patches 7 and 8, where I changed the linking of matching
> hists entries.
>
> The doc was updated with info about current perf diff processing.
>
> Attached patches:
> 01/14 perf tool: Introduce perf_hpp__list for period related columns
> 02/14 perf tool: Add struct perf_hpp_fmt into hpp callbacks
> 03/14 perf tool: Fix period symbol_conf.field_sep display
> 04/14 perf diff: Remove displacement from struct hist_entry_diff
> 05/14 perf diff: Change compute methods to work with pair directly
> 06/14 perf diff: Change formula methods to work with pair directly
> 07/14 perf diff: Add callback to hists__match/hists__link functions
> 08/14 perf diff: Change diff command to work over multiple data files
> 09/14 perf diff: Update perf diff documentation for multiple data comparison
> 10/14 perf tool: Centralize default columns init in perf_hpp__init
> 11/14 perf diff: Making compute functions static
> 12/14 perf diff: Display data file info ahead of the diff output
> 13/14 perf diff: Display zero calculation results
> 14/14 perf diff: Add generic order option for compute sorting
>
> Example of multiple perf diff output:
>
> $ perf diff perf.data.[123456]
> # Event 'cycles:u'
> #
> # Data files:
> # [0] perf.data.1
> # [1] perf.data.2
> # [2] perf.data.3
> # [3] perf.data.4
> # [4] perf.data.5
> # [5] perf.data.6
> #
> # Baseline/0 Delta/1 Delta/2 Delta/3 Delta/4 Delta/5 Shared Object Symbol
> # .......... ....... ....... ....... ....... ....... ................. .........................
> #
> +73.05% [kernel.kallsyms] [k] page_fault
> +26.16% ld-2.15.so [.] _dl_next_ld_env_entry
> +15.40% ld-2.15.so [.] _dl_sysdep_start
> 71.48% -26.52% libc-2.15.so [.] __strcmp_sse2
> 27.69% -1.65% -4.83% -4.68% ld-2.15.so [.] dl_main
> 0.82% -0.05% -0.34% -0.15% -0.13% -0.03% ld-2.15.so [.] _start
> +39.15% libc-2.15.so [.] error_tail
> +73.18% libc-2.15.so [.] __strcasecmp_l_sse2
> +76.30% libc-2.15.so [.] __stpcpy_sse2
> +76.46% libc-2.15.so [.] _IO_getline_info
>
> thanks,
> jirka
>
> 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: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/Documentation/perf-diff.txt | 77 ++++++++--
> tools/perf/builtin-diff.c | 747 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------
> tools/perf/ui/browsers/hists.c | 22 +--
> tools/perf/ui/gtk/browser.c | 26 +---
> tools/perf/ui/hist.c | 392 ++++++++++++++---------------------------------
> tools/perf/ui/stdio/hist.c | 17 +--
> tools/perf/util/hist.c | 29 ++--
> tools/perf/util/hist.h | 42 ++---
> tools/perf/util/sort.h | 38 ++---
> 9 files changed, 855 insertions(+), 535 deletions(-)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH/RFC 00/14] perf, diff: Support for multiple files
2012-11-28 13:56 ` [PATCH/RFC 00/14] perf, diff: Support for multiple files Jiri Olsa
@ 2012-11-28 15:57 ` Jiri Olsa
0 siblings, 0 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-28 15:57 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim
On Wed, Nov 28, 2012 at 02:56:55PM +0100, Jiri Olsa wrote:
> aargh.. fixed subject.. copy&paste error ;-)
>
> also patches are available here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/linux.git
> perf/multi2
worked out some fixies with Arnaldo, new version is here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/linux.git
perf/multi3
thanks,
jirka
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 01/14] perf tool: Introduce perf_hpp__list for period related columns
2012-11-28 13:52 ` [PATCH 01/14] perf tool: Introduce perf_hpp__list for period related columns Jiri Olsa
@ 2012-11-29 7:44 ` Namhyung Kim
2012-11-29 10:52 ` Namhyung Kim
0 siblings, 1 reply; 39+ messages in thread
From: Namhyung Kim @ 2012-11-29 7:44 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
Hi Jiri,
On Wed, 28 Nov 2012 14:52:36 +0100, Jiri Olsa wrote:
> Adding perf_hpp__list list to register and contain all period
> related columns the command is interested in.
>
> This way we get rid of static array holding all possible
> columns and enable commands to register their own columns.
But it seems you didn't get rid of the array? :)
>
> It'll be handy for diff command in future to process and display
> data for multiple files.
[snip]
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 8b091a5..a935a60 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -126,13 +126,19 @@ struct perf_hpp {
> };
>
> struct perf_hpp_fmt {
> - bool cond;
> int (*header)(struct perf_hpp *hpp);
> int (*width)(struct perf_hpp *hpp);
> int (*color)(struct perf_hpp *hpp, struct hist_entry *he);
> int (*entry)(struct perf_hpp *hpp, struct hist_entry *he);
> +
> + struct list_head list;
> };
>
> +extern struct list_head perf_hpp__list;
> +
> +#define perf_hpp__for_each_format(format) \
> + list_for_each_entry(format, &perf_hpp__list, list)
> +
> extern struct perf_hpp_fmt perf_hpp__format[];
Instead of using new perf_hpp__list, how about this?
#define perf_hpp__for_each_format(fmt) \
for (fmt = &perf_hpp__format[0]; fmt < &perf_hpp__format[PERF_HPP__MAX_INDEX]; fmt++) \
if (fmt->cond)
Thanks,
Namhyung
>
> enum {
> @@ -155,7 +161,8 @@ enum {
> };
>
> void perf_hpp__init(void);
> -void perf_hpp__column_enable(unsigned col, bool enable);
> +void perf_hpp__column_register(struct perf_hpp_fmt *format);
> +void perf_hpp__column_enable(unsigned col);
> int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
> bool color);
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 03/14] perf tool: Fix period symbol_conf.field_sep display
2012-11-28 13:52 ` [PATCH 03/14] perf tool: Fix period symbol_conf.field_sep display Jiri Olsa
@ 2012-11-29 7:56 ` Namhyung Kim
2012-11-29 11:53 ` Jiri Olsa
0 siblings, 1 reply; 39+ messages in thread
From: Namhyung Kim @ 2012-11-29 7:56 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Wed, 28 Nov 2012 14:52:38 +0100, Jiri Olsa wrote:
> Currently we don't properly display hist data with
> symbol_conf.field_sep separator. We need to display
> either space or separator.
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> 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: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/ui/hist.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index 5452960..20a4392 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c
> @@ -523,17 +523,13 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
> struct perf_hpp_fmt *fmt;
> char *start = hpp->buf;
> int ret;
> - bool first = true;
>
> if (symbol_conf.exclude_other && !he->parent)
> return 0;
>
> perf_hpp__for_each_format(fmt) {
> - if (!sep || !first) {
> - ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
> - advance_hpp(hpp, ret);
> - first = false;
> - }
> + ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
> + advance_hpp(hpp, ret);
It will display the separator even before the first column so that the
output can be messed up with this. I can see that there's a bug setting
'first' to false - the line should be moved out of the block otherwise
it's pointless since we cannot enter the block.
Thanks,
Namhyung
>
> if (color && fmt->color)
> ret = fmt->color(fmt, hpp, he);
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/14] perf diff: Remove displacement from struct hist_entry_diff
2012-11-28 13:52 ` [PATCH 04/14] perf diff: Remove displacement from struct hist_entry_diff Jiri Olsa
@ 2012-11-29 8:00 ` Namhyung Kim
2013-01-24 18:50 ` [tip:perf/core] " tip-bot for Jiri Olsa
1 sibling, 0 replies; 39+ messages in thread
From: Namhyung Kim @ 2012-11-29 8:00 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Wed, 28 Nov 2012 14:52:39 +0100, Jiri Olsa wrote:
> Removing displacement from struct hist_entry_diff, because
> it's not used. Displacement is not used for sorting, so
> there's no reason to pre-calculate it.
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> 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>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 01/14] perf tool: Introduce perf_hpp__list for period related columns
2012-11-29 7:44 ` Namhyung Kim
@ 2012-11-29 10:52 ` Namhyung Kim
0 siblings, 0 replies; 39+ messages in thread
From: Namhyung Kim @ 2012-11-29 10:52 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
Hi,
On Thu, 29 Nov 2012 16:44:59 +0900, Namhyung Kim wrote:
> Hi Jiri,
>
> On Wed, 28 Nov 2012 14:52:36 +0100, Jiri Olsa wrote:
>> Adding perf_hpp__list list to register and contain all period
>> related columns the command is interested in.
>>
>> This way we get rid of static array holding all possible
>> columns and enable commands to register their own columns.
>
> But it seems you didn't get rid of the array? :)
Ah, understood. You wanted to add new hpp columns dynamically!
Looks good to me then.
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 05/14] perf diff: Change compute methods to work with pair directly
2012-11-28 13:52 ` [PATCH 05/14] perf diff: Change compute methods to work with pair directly Jiri Olsa
@ 2012-11-29 11:15 ` Namhyung Kim
2013-01-24 18:51 ` [tip:perf/core] " tip-bot for Jiri Olsa
1 sibling, 0 replies; 39+ messages in thread
From: Namhyung Kim @ 2012-11-29 11:15 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Wed, 28 Nov 2012 14:52:40 +0100, Jiri Olsa wrote:
> Changing compute methods to operate over hist entry and its
> pair directly. This makes the code more obvious and readable,
> instead of all time checking for pair being != NULL.
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> 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>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 06/14] perf diff: Change formula methods to work with pair directly
2012-11-28 13:52 ` [PATCH 06/14] perf diff: Change formula " Jiri Olsa
@ 2012-11-29 11:15 ` Namhyung Kim
2013-01-24 18:52 ` [tip:perf/core] " tip-bot for Jiri Olsa
1 sibling, 0 replies; 39+ messages in thread
From: Namhyung Kim @ 2012-11-29 11:15 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Wed, 28 Nov 2012 14:52:41 +0100, Jiri Olsa wrote:
> Changing formula methods to operate over hist entry and its
> pair directly. This makes the code more obvious and readable,
> instead of all time checking for pair being != NULL.
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> 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>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 08/14] perf diff: Change diff command to work over multiple data files
2012-11-28 13:52 ` [PATCH 08/14] perf diff: Change diff command to work over multiple data files Jiri Olsa
@ 2012-11-29 11:37 ` Namhyung Kim
2012-11-29 11:53 ` Jiri Olsa
0 siblings, 1 reply; 39+ messages in thread
From: Namhyung Kim @ 2012-11-29 11:37 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Wed, 28 Nov 2012 14:52:43 +0100, Jiri Olsa wrote:
> Adding diff command the flexibility to specify multiple data
> files on input. If not input file is given the standard behaviour
> stands and diff inspects 'perf.data' and 'perf.data.old' files.
>
> Also changing the processing and output display for data files.
>
> For 'perf diff A B' command:
>
> - the current way is to iterate over B data and display
> A (baseline) data (symbol samples) only if found in B
>
> - this patch iterates over A (baseline) data and display
> B data (symbol samples) only if found in A
>
> For 'perf diff A B C' command, the diff now iterates the
> A (baseline) data and display B and C data (symbol samples)
> only if they found in A (baseline)
>
> All other standard diff command computation features
> (delta,ratio,wdiff) stays.
This is quite a large change and IMHO can be separated into 3 patches at
least.
1) change he->pairs from list to array
2) introduce diff_data__fmt
3) convert to diff_data__fmt
Thanks,
Namhyung
> 5 files changed, 560 insertions(+), 392 deletions(-)
>
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index 6361b55..9f4ef76 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -18,11 +18,49 @@
> #include "util/util.h"
>
> #include <stdlib.h>
> +#include <math.h>
>
> -static char const *input_old = "perf.data.old",
> - *input_new = "perf.data";
> -static char diff__default_sort_order[] = "dso,symbol";
> -static bool force;
> +/* Diff command specific HPP columns. */
> +enum {
> + PERF_HPP_DIFF__BASELINE,
> + PERF_HPP_DIFF__PERIOD,
> + PERF_HPP_DIFF__PERIOD_BASELINE,
> + PERF_HPP_DIFF__DELTA,
> + PERF_HPP_DIFF__RATIO,
> + PERF_HPP_DIFF__WEIGHTED_DIFF,
> + PERF_HPP_DIFF__DISPL,
> + PERF_HPP_DIFF__FORMULA,
> +
> + PERF_HPP_DIFF__MAX_INDEX
> +};
> +
> +struct diff_data__fmt {
> + struct perf_hpp_fmt fmt;
> + int idx;
> +
> + char *header;
> + int header_width;
> +};
> +
> +struct diff_data {
> + struct perf_session *session;
> + const char *file;
> + int idx;
> +
> + struct diff_data__fmt fmt[PERF_HPP_DIFF__MAX_INDEX];
> +};
> +
> +static struct diff_data *data;
> +static int data_cnt;
> +
> +#define for_each_data(i, d) \
> + for (i = 0, d = &data[0]; i < data_cnt; i++, d = &data[i])
> +
> +#define for_each_data_new(i, d) \
> + for (i = 1, d = &data[1]; i < data_cnt; i++, d = &data[i])
> +
> +static const char diff__default_sort_order[] = "dso,symbol";
> +static bool force;
> static bool show_displacement;
> static bool show_period;
> static bool show_formula;
> @@ -39,12 +77,55 @@ enum {
> COMPUTE_MAX,
> };
>
> +static int compute_2_hpp[COMPUTE_MAX] = {
> + [COMPUTE_DELTA] = PERF_HPP_DIFF__DELTA,
> + [COMPUTE_RATIO] = PERF_HPP_DIFF__RATIO,
> + [COMPUTE_WEIGHTED_DIFF] = PERF_HPP_DIFF__WEIGHTED_DIFF,
> +};
> +
> +
> const char *compute_names[COMPUTE_MAX] = {
> [COMPUTE_DELTA] = "delta",
> [COMPUTE_RATIO] = "ratio",
> [COMPUTE_WEIGHTED_DIFF] = "wdiff",
> };
>
> +static struct header_column {
> + const char *name;
> + int width;
> +} columns[PERF_HPP_DIFF__MAX_INDEX] = {
> + [PERF_HPP_DIFF__BASELINE] = {
> + .name = "Baseline",
> + },
> + [PERF_HPP_DIFF__PERIOD] = {
> + .name = "Period",
> + },
> + [PERF_HPP_DIFF__PERIOD_BASELINE] = {
> + .name = "Base period",
> + },
> + [PERF_HPP_DIFF__DELTA] = {
> + .name = "Delta",
> + .width = 7,
> + },
> + [PERF_HPP_DIFF__RATIO] = {
> + .name = "Ratio",
> + .width = 14,
> + },
> + [PERF_HPP_DIFF__WEIGHTED_DIFF] = {
> + .name = "Weighted diff",
> + .width = 14,
> + },
> + [PERF_HPP_DIFF__DISPL] = {
> + .name = "Displ",
> + },
> + [PERF_HPP_DIFF__FORMULA] = {
> + .name = "Formula",
> + .width = 70,
> + }
> +};
> +
> +#define MAX_COL_WIDTH 70
> +
> static int compute;
>
> static int setup_compute_opt_wdiff(char *opt)
> @@ -146,7 +227,7 @@ static int setup_compute(const struct option *opt, const char *str,
> return -EINVAL;
> }
>
> -double perf_diff__period_percent(struct hist_entry *he, u64 period)
> +static double get_period_percent(struct hist_entry *he, u64 period)
> {
> u64 total = he->hists->stats.total_period;
> return (period * 100.0) / total;
> @@ -154,34 +235,34 @@ double perf_diff__period_percent(struct hist_entry *he, u64 period)
>
> double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair)
> {
> - double new_percent = perf_diff__period_percent(he, he->stat.period);
> - double old_percent = perf_diff__period_percent(pair, pair->stat.period);
> + double old_percent = get_period_percent(he, he->stat.period);
> + double new_percent = get_period_percent(pair, pair->stat.period);
>
> - he->diff.period_ratio_delta = new_percent - old_percent;
> - he->diff.computed = true;
> - return he->diff.period_ratio_delta;
> + pair->diff.period_ratio_delta = new_percent - old_percent;
> + pair->diff.computed = true;
> + return pair->diff.period_ratio_delta;
> }
>
> double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair)
> {
> - double new_period = he->stat.period;
> - double old_period = pair->stat.period;
> + double old_period = he->stat.period ?: 1;
> + double new_period = pair->stat.period;
>
> - he->diff.computed = true;
> - he->diff.period_ratio = new_period / old_period;
> - return he->diff.period_ratio;
> + pair->diff.computed = true;
> + pair->diff.period_ratio = (new_period / old_period);
> + return pair->diff.period_ratio;
> }
>
> s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair)
> {
> - u64 new_period = he->stat.period;
> - u64 old_period = pair->stat.period;
> + u64 old_period = he->stat.period;
> + u64 new_period = pair->stat.period;
>
> - he->diff.computed = true;
> - he->diff.wdiff = new_period * compute_wdiff_w2 -
> - old_period * compute_wdiff_w1;
> + pair->diff.computed = true;
> + pair->diff.wdiff = new_period * compute_wdiff_w1 -
> + old_period * compute_wdiff_w2;
>
> - return he->diff.wdiff;
> + return pair->diff.wdiff;
> }
>
> static int formula_delta(struct hist_entry *he, struct hist_entry *pair,
> @@ -197,8 +278,8 @@ static int formula_delta(struct hist_entry *he, struct hist_entry *pair,
> static int formula_ratio(struct hist_entry *he, struct hist_entry *pair,
> char *buf, size_t size)
> {
> - double new_period = he->stat.period;
> - double old_period = pair->stat.period;
> + double old_period = he->stat.period;
> + double new_period = pair->stat.period;
>
> return scnprintf(buf, size, "%.0F / %.0F", new_period, old_period);
> }
> @@ -206,12 +287,12 @@ static int formula_ratio(struct hist_entry *he, struct hist_entry *pair,
> static int formula_wdiff(struct hist_entry *he, struct hist_entry *pair,
> char *buf, size_t size)
> {
> - u64 new_period = he->stat.period;
> - u64 old_period = pair->stat.period;
> + u64 old_period = he->stat.period;
> + u64 new_period = pair->stat.period;
>
> return scnprintf(buf, size,
> "(%" PRIu64 " * " "%" PRId64 ") - (%" PRIu64 " * " "%" PRId64 ")",
> - new_period, compute_wdiff_w2, old_period, compute_wdiff_w1);
> + new_period, compute_wdiff_w1, old_period, compute_wdiff_w2);
> }
>
> int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair,
> @@ -356,22 +437,21 @@ static void hists__baseline_only(struct hists *hists)
> struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
>
> next = rb_next(&he->rb_node);
> - if (!hist_entry__next_pair(he)) {
> +
> + if (!he->pairs) {
> rb_erase(&he->rb_node, &hists->entries);
> hist_entry__free(he);
> }
> }
> }
>
> -static void hists__precompute(struct hists *hists)
> +static void __hists__precompute(struct hist_entry *he)
> {
> - struct rb_node *next = rb_first(&hists->entries);
> + int i;
>
> - while (next != NULL) {
> - struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
> - struct hist_entry *pair = hist_entry__next_pair(he);
> + for (i = 0; i < data_cnt; i++) {
> + struct hist_entry *pair = he->pairs[i];
>
> - next = rb_next(&he->rb_node);
> if (!pair)
> continue;
>
> @@ -391,6 +471,21 @@ static void hists__precompute(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->pairs)
> + __hists__precompute(he);
> + }
> +}
> +
> static int64_t cmp_doubles(double l, double r)
> {
> if (l > r)
> @@ -402,8 +497,8 @@ static int64_t cmp_doubles(double l, double r)
> }
>
> static int64_t
> -hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
> - int c)
> +__hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
> + int c)
> {
> switch (c) {
> case COMPUTE_DELTA:
> @@ -434,6 +529,39 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
> return 0;
> }
>
> +static int64_t
> +hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
> + int c)
> +{
> + int i;
> +
> + for (i = 0; i < data_cnt; i++) {
> + struct hist_entry **pairs_left = left->pairs;
> + struct hist_entry **pairs_right = right->pairs;
> + struct hist_entry *p_right, *p_left;
> + static int64_t cmp;
> +
> + if (!pairs_left || !pairs_right)
> + return pairs_right - pairs_left;
> +
> + p_right = pairs_right[i];
> + p_left = pairs_left[i];
> +
> + if (!p_left || !p_right)
> + return p_right - p_left;
> +
> + /*
> + * If we differ, we are done, otherwise continue until all
> + * is processed or we find a difference.
> + */
> + cmp = __hist_entry__cmp_compute(p_left, p_right, c);
> + if (cmp)
> + return cmp;
> + }
> +
> + return 0;
> +}
> +
> static void insert_hist_entry_by_compute(struct rb_root *root,
> struct hist_entry *he,
> int c)
> @@ -473,83 +601,112 @@ static void hists__compute_resort(struct hists *hists)
> }
>
> static int match_cb(struct hist_entry *a, struct hist_entry *b,
> - void *data __maybe_unused)
> + void *_data)
> {
> - hist__entry_add_pair(a, b);
> + intptr_t i = (intptr_t) _data;
> + struct hist_entry **p;
> +
> + if (!a->pairs) {
> + p = zalloc(sizeof(p) * data_cnt);
> + if (!p)
> + return -ENOMEM;
> +
> + a->pairs = p;
> + }
> +
> + a->pairs[i] = b;
> + b->paired = true;
> return 0;
> }
>
> -static void hists__process(struct hists *old, struct hists *new)
> +static void hists__process(struct hists *hists)
> {
> - hists__match(new, old, match_cb, NULL);
> -
> if (show_baseline_only)
> - hists__baseline_only(new);
> - else
> - hists__link(new, old, match_cb, NULL);
> + hists__baseline_only(hists);
>
> if (sort_compute) {
> - hists__precompute(new);
> - hists__compute_resort(new);
> + hists__precompute(hists);
> + hists__compute_resort(hists);
> }
>
> - hists__fprintf(new, true, 0, 0, stdout);
> + hists__fprintf(hists, true, 0, 0, stdout);
> }
>
> -static int __cmd_diff(void)
> +static int data_process(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;
> + struct perf_evlist *evlist_base = data[0].session->evlist;
> + struct perf_evsel *evsel_base;
> bool first = true;
>
> - older = perf_session__new(input_old, O_RDONLY, force, false,
> - &tool);
> - newer = perf_session__new(input_new, O_RDONLY, force, false,
> - &tool);
> - if (session[0] == NULL || session[1] == NULL)
> - return -ENOMEM;
> + list_for_each_entry(evsel_base, &evlist_base->entries, node) {
> + struct diff_data *d;
> + int i;
>
> - for (i = 0; i < 2; ++i) {
> - ret = perf_session__process_events(session[i], &tool);
> - if (ret)
> - goto out_delete;
> - }
> -
> - evlist_old = older->evlist;
> - evlist_new = newer->evlist;
> + for_each_data_new(i, d) {
> + struct perf_evlist *evlist = d->session->evlist;
> + struct perf_evsel *evsel_new;
>
> - perf_evlist__resort_hists(evlist_old, true);
> - perf_evlist__resort_hists(evlist_new, false);
> + evsel_new = evsel_match(evsel_base, evlist);
> + if (!evsel_new)
> + continue;
>
> - list_for_each_entry(evsel, &evlist_new->entries, node) {
> - struct perf_evsel *evsel_old;
> + hists__match(&evsel_base->hists, &evsel_new->hists,
> + match_cb, (void *) (intptr_t) i);
>
> - evsel_old = evsel_match(evsel, evlist_old);
> - if (!evsel_old)
> - continue;
> + if (!show_baseline_only)
> + hists__link(&evsel_base->hists,
> + &evsel_new->hists,
> + match_cb, (void *) (intptr_t) i);
> + }
>
> fprintf(stdout, "%s# Event '%s'\n#\n", first ? "" : "\n",
> - perf_evsel__name(evsel));
> + perf_evsel__name(evsel_base));
> +
> + hists__process(&evsel_base->hists);
>
> first = false;
> + }
> +
> + return 0;
> +}
> +
> +static int __cmd_diff(void)
> +{
> + struct diff_data *d;
> + int ret = -EINVAL, i;
> +
> + for_each_data(i, d) {
> + d->session = perf_session__new(d->file, O_RDONLY, force,
> + false, &tool);
> + if (!d->session) {
> + pr_err("Failed to open %s\n", d->file);
> + ret = -ENOMEM;
> + goto out_delete;
> + }
>
> - hists__process(&evsel_old->hists, &evsel->hists);
> + ret = perf_session__process_events(d->session, &tool);
> + if (ret) {
> + pr_err("Failed to process %s\n", d->file);
> + goto out_delete;
> + }
> +
> + /* Sort by name all but baseline. */
> + perf_evlist__resort_hists(d->session->evlist, i != 0);
> }
>
> + ret = data_process();
> +
> out_delete:
> - for (i = 0; i < 2; ++i)
> - perf_session__delete(session[i]);
> + for_each_data(i, d) {
> + if (d->session)
> + perf_session__delete(d->session);
> + }
> +
> return ret;
> -#undef older
> -#undef newer
> }
>
> static const char * const diff_usage[] = {
> - "perf diff [<options>] [old_file] [new_file]",
> + "perf diff [<options>] <base file> <file1> [file2] ...",
> NULL,
> };
>
> @@ -589,62 +746,340 @@ static const struct option options[] = {
> OPT_END()
> };
>
> -static void ui_init(void)
> +static double baseline_percent(struct hist_entry *he)
> {
> - /*
> - * Display baseline/delta/ratio/displacement/
> - * formula/periods columns.
> - */
> - perf_hpp__column_enable(PERF_HPP__BASELINE);
> + struct hists *hists = he->hists;
> + u64 total_period = hists->stats.total_period;
> + u64 base_period = he->stat.period;
>
> - switch (compute) {
> - case COMPUTE_DELTA:
> - perf_hpp__column_enable(PERF_HPP__DELTA);
> + return 100.0 * base_period / total_period;
> +}
> +
> +static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
> + struct perf_hpp *hpp, struct hist_entry *he)
> +{
> + double percent = baseline_percent(he);
> + struct diff_data__fmt *dfmt =
> + container_of(fmt, struct diff_data__fmt, fmt);
> + char pfmt[20] = " ";
> +
> + if (percent) {
> + scnprintf(pfmt, 20, "%%%d.2f%%%%", dfmt->header_width - 1);
> + return percent_color_snprintf(hpp->buf, hpp->size,
> + pfmt, percent);
> + } else
> + return scnprintf(hpp->buf, hpp->size, "%*s",
> + dfmt->header_width, pfmt);
> +}
> +
> +static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt, struct perf_hpp *hpp,
> + struct hist_entry *he)
> +{
> + struct diff_data__fmt *dfmt =
> + container_of(_fmt, struct diff_data__fmt, fmt);
> + double percent = baseline_percent(he);
> + const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
> + char buf[32] = " ";
> +
> + if ((percent && he->pairs) || symbol_conf.field_sep)
> + return scnprintf(hpp->buf, hpp->size, fmt, percent);
> + else
> + return scnprintf(hpp->buf, hpp->size, "%*s",
> + dfmt->header_width, buf);
> +}
> +
> +static struct hist_entry *get_pair(struct hist_entry *he,
> + struct perf_hpp_fmt *fmt)
> +{
> + struct hist_entry *pair = NULL;
> +
> + if (he->pairs) {
> + struct diff_data *d;
> + struct diff_data__fmt *dfmt;
> + void *ptr;
> +
> + dfmt = container_of(fmt, struct diff_data__fmt, fmt);
> + ptr = dfmt - dfmt->idx;
> + d = container_of(ptr, struct diff_data, fmt);
> + pair = he->pairs[d->idx];
> + }
> +
> + return pair;
> +}
> +
> +static void
> +hpp__entry_unpair(struct hist_entry *he, int idx, char *buf, size_t size)
> +{
> + switch (idx) {
> + case PERF_HPP_DIFF__PERIOD_BASELINE:
> + scnprintf(buf, size, "%" PRIu64, he->stat.period);
> break;
> - case COMPUTE_RATIO:
> - perf_hpp__column_enable(PERF_HPP__RATIO);
> +
> + default:
> break;
> - case COMPUTE_WEIGHTED_DIFF:
> - perf_hpp__column_enable(PERF_HPP__WEIGHTED_DIFF);
> + }
> +}
> +
> +static void
> +hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
> + int idx, char *buf, size_t size)
> +{
> + long displacement;
> + double diff;
> + double ratio;
> + s64 wdiff;
> +
> + switch (idx) {
> + case PERF_HPP_DIFF__DELTA:
> + if (pair->diff.computed)
> + diff = pair->diff.period_ratio_delta;
> + else
> + diff = perf_diff__compute_delta(he, pair);
> +
> + if (fabs(diff) >= 0.01)
> + scnprintf(buf, size, "%+4.2F%%", diff);
> + break;
> +
> + case PERF_HPP_DIFF__RATIO:
> + if (pair->diff.computed)
> + ratio = pair->diff.period_ratio;
> + else
> + ratio = perf_diff__compute_ratio(he, pair);
> +
> + if (ratio > 0.0)
> + scnprintf(buf, size, "%14.6F", ratio);
> + break;
> +
> + case PERF_HPP_DIFF__WEIGHTED_DIFF:
> + if (pair->diff.computed)
> + wdiff = pair->diff.wdiff;
> + else
> + wdiff = perf_diff__compute_wdiff(he, pair);
> +
> + if (wdiff != 0)
> + scnprintf(buf, size, "%14ld", wdiff);
> + break;
> +
> + case PERF_HPP_DIFF__DISPL:
> + displacement = pair->position - he->position;
> +
> + if (displacement)
> + scnprintf(buf, size, "%+4ld", displacement);
> break;
> +
> + case PERF_HPP_DIFF__FORMULA:
> + perf_diff__formula(he, pair, buf, size);
> + break;
> +
> + case PERF_HPP_DIFF__PERIOD:
> + scnprintf(buf, size, "%" PRIu64, pair->stat.period);
> + break;
> +
> default:
> BUG_ON(1);
> - };
> + }
> +}
> +
> +static int hpp__entry_global(struct perf_hpp_fmt *_fmt, struct perf_hpp *hpp,
> + struct hist_entry *he)
> +{
> + struct diff_data__fmt *dfmt =
> + container_of(_fmt, struct diff_data__fmt, fmt);
> + struct hist_entry *pair;
> + char buf[MAX_COL_WIDTH] = " ";
> +
> + pair = get_pair(he, _fmt);
> +
> + if (pair)
> + hpp__entry_pair(he, pair, dfmt->idx, buf, MAX_COL_WIDTH);
> + else
> + hpp__entry_unpair(he, dfmt->idx, buf, MAX_COL_WIDTH);
> +
> + if (symbol_conf.field_sep)
> + return scnprintf(hpp->buf, hpp->size, "%s", buf);
> + else
> + return scnprintf(hpp->buf, hpp->size, "%*s",
> + dfmt->header_width, buf);
> +}
> +
> +static int diff_hpp__header(struct perf_hpp_fmt *fmt,
> + struct perf_hpp *hpp)
> +{
> + struct diff_data__fmt *dfmt =
> + container_of(fmt, struct diff_data__fmt, fmt);
> +
> + BUG_ON(!dfmt->header);
> +
> + return scnprintf(hpp->buf, hpp->size, dfmt->header);
> +}
> +
> +static int diff_hpp__width(struct perf_hpp_fmt *fmt,
> + struct perf_hpp *hpp __maybe_unused)
> +{
> + struct diff_data__fmt *dfmt =
> + container_of(fmt, struct diff_data__fmt, fmt);
> +
> + BUG_ON(dfmt->header_width <= 0);
> +
> + return dfmt->header_width;
> +}
>
> - if (show_displacement)
> - perf_hpp__column_enable(PERF_HPP__DISPL);
> +static void init_header(struct diff_data *d, struct diff_data__fmt *dfmt)
> +{
> +#define MAX_HEADER_NAME 100
> + char buf_indent[MAX_HEADER_NAME];
> + char buf[MAX_HEADER_NAME];
> + const char *header = NULL;
> + int width = 0;
> +
> + BUG_ON(dfmt->idx >= PERF_HPP_DIFF__MAX_INDEX);
> + header = columns[dfmt->idx].name;
> + width = columns[dfmt->idx].width;
> +
> + /* Only our defined HPP fmts should appear here. */
> + BUG_ON(!header);
> +
> + if (data_cnt > 2)
> + scnprintf(buf, MAX_HEADER_NAME, "%s/%d", header, d->idx);
> +
> +#define NAME (data_cnt > 2 ? buf : header)
> + dfmt->header_width = width;
> + width = (int) strlen(NAME);
> + if (dfmt->header_width < width)
> + dfmt->header_width = width;
> +
> + scnprintf(buf_indent, MAX_HEADER_NAME, "%*s",
> + dfmt->header_width, NAME);
> +
> + dfmt->header = strdup(buf_indent);
> +#undef MAX_HEADER_NAME
> +#undef NAME
> +}
>
> - if (show_formula)
> - perf_hpp__column_enable(PERF_HPP__FORMULA);
> +static void data__hpp_register(struct diff_data *d, int idx)
> +{
> + struct diff_data__fmt *dfmt = &d->fmt[idx];
> + struct perf_hpp_fmt *fmt = &dfmt->fmt;
>
> - if (show_period) {
> - perf_hpp__column_enable(PERF_HPP__PERIOD);
> - perf_hpp__column_enable(PERF_HPP__PERIOD_BASELINE);
> + dfmt->idx = idx;
> +
> + fmt->header = diff_hpp__header;
> + fmt->width = diff_hpp__width;
> +
> + switch (idx) {
> +#define CASE_COLOR(c, name) \
> + case c: \
> + fmt->color = hpp__color_##name; \
> + fmt->entry = hpp__entry_##name; \
> + break;
> +
> +#define CASE(c, name) \
> + case c: \
> + fmt->entry = hpp__entry_##name; \
> + break;
> +
> + CASE_COLOR(PERF_HPP_DIFF__BASELINE, baseline)
> + CASE(PERF_HPP_DIFF__PERIOD_BASELINE, global)
> + CASE(PERF_HPP_DIFF__DELTA, global)
> + CASE(PERF_HPP_DIFF__RATIO, global)
> + CASE(PERF_HPP_DIFF__WEIGHTED_DIFF, global)
> + CASE(PERF_HPP_DIFF__DISPL, global)
> + CASE(PERF_HPP_DIFF__FORMULA, global)
> + CASE(PERF_HPP_DIFF__PERIOD, global)
> +
> + default:
> + BUG_ON(1);
> }
> +
> + perf_hpp__column_register(fmt);
> +
> + init_header(d, dfmt);
> }
>
> -int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
> +static void ui_init(void)
> {
> - sort_order = diff__default_sort_order;
> - argc = parse_options(argc, argv, options, diff_usage, 0);
> + struct diff_data *d;
> + int i;
> +
> + for_each_data(i, d) {
> +
> + /*
> + * Baseline or compute realted columns:
> + *
> + * PERF_HPP_DIFF__BASELINE
> + * PERF_HPP_DIFF__DELTA
> + * PERF_HPP_DIFF__RATIO
> + * PERF_HPP_DIFF__WEIGHTED_DIFF
> + */
> + data__hpp_register(d, i ? compute_2_hpp[compute] :
> + PERF_HPP_DIFF__BASELINE);
> +
> + /*
> + * And the rest:
> + *
> + * PERF_HPP_DIFF__DISPL
> + * PERF_HPP_DIFF__FORMULA
> + * PERF_HPP_DIFF__PERIOD
> + * PERF_HPP_DIFF__PERIOD_BASELINE
> + */
> + if (show_displacement && i)
> + data__hpp_register(d, PERF_HPP_DIFF__DISPL);
> +
> + if (show_formula && i)
> + data__hpp_register(d, PERF_HPP_DIFF__FORMULA);
> +
> + if (show_period)
> + data__hpp_register(d, i ? PERF_HPP_DIFF__PERIOD :
> + PERF_HPP_DIFF__PERIOD_BASELINE);
> + }
> +}
> +
> +static int data_init(int argc, const char **argv)
> +{
> + struct diff_data *d;
> + static const char *defaults[] = {
> + "perf.data.old",
> + "perf.data",
> + };
> + int i;
> +
> + if (argc == 1)
> + usage_with_options(diff_usage, options);
> +
> + data_cnt = 2;
> +
> if (argc) {
> - if (argc > 2)
> - usage_with_options(diff_usage, options);
> - if (argc == 2) {
> - input_old = argv[0];
> - input_new = argv[1];
> - } else
> - input_new = argv[0];
> + data_cnt = argc;
> } else if (symbol_conf.default_guest_vmlinux_name ||
> symbol_conf.default_guest_kallsyms) {
> - input_old = "perf.data.host";
> - input_new = "perf.data.guest";
> + defaults[0] = "perf.data.host";
> + defaults[1] = "perf.data.guest";
> }
>
> + data = zalloc(sizeof(*data) * data_cnt);
> + if (!data)
> + return -ENOMEM;
> +
> + for_each_data(i, d) {
> + d->file = argc ? argv[i] : defaults[i];
> + d->idx = i;
> + }
> +
> + return 0;
> +}
> +
> +int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
> +{
> + sort_order = diff__default_sort_order;
> + argc = parse_options(argc, argv, options, diff_usage, 0);
> symbol_conf.exclude_other = false;
> +
> if (symbol__init() < 0)
> return -1;
>
> + if (data_init(argc, argv) < 0)
> + return -1;
> +
> ui_init();
>
> setup_sorting(diff_usage, options);
> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index d6fdb00..a004f57 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c
> @@ -177,57 +177,6 @@ hpp__entry_overhead_guest_us(struct perf_hpp_fmt *_fmt __maybe_unused,
> return scnprintf(hpp->buf, hpp->size, fmt, percent);
> }
>
> -static int hpp__header_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
> - struct perf_hpp *hpp)
> -{
> - return scnprintf(hpp->buf, hpp->size, "Baseline");
> -}
> -
> -static int hpp__width_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
> - struct perf_hpp *hpp __maybe_unused)
> -{
> - return 8;
> -}
> -
> -static double baseline_percent(struct hist_entry *he)
> -{
> - struct hist_entry *pair = hist_entry__next_pair(he);
> - struct hists *pair_hists = pair ? pair->hists : NULL;
> - double percent = 0.0;
> -
> - if (pair) {
> - u64 total_period = pair_hists->stats.total_period;
> - u64 base_period = pair->stat.period;
> -
> - percent = 100.0 * base_period / total_period;
> - }
> -
> - return percent;
> -}
> -
> -static int hpp__color_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
> - struct perf_hpp *hpp, struct hist_entry *he)
> -{
> - double percent = baseline_percent(he);
> -
> - if (hist_entry__has_pairs(he))
> - return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
> - else
> - return scnprintf(hpp->buf, hpp->size, " ");
> -}
> -
> -static int hpp__entry_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp, struct hist_entry *he)
> -{
> - double percent = baseline_percent(he);
> - const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
> -
> - if (hist_entry__has_pairs(he) || symbol_conf.field_sep)
> - return scnprintf(hpp->buf, hpp->size, fmt, percent);
> - else
> - return scnprintf(hpp->buf, hpp->size, " ");
> -}
> -
> static int hpp__header_samples(struct perf_hpp_fmt *_fmt __maybe_unused,
> struct perf_hpp *hpp)
> {
> @@ -272,190 +221,6 @@ static int hpp__entry_period(struct perf_hpp_fmt *_fmt __maybe_unused,
> return scnprintf(hpp->buf, hpp->size, fmt, he->stat.period);
> }
>
> -static int hpp__header_period_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp)
> -{
> - const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, "Period Base");
> -}
> -
> -static int hpp__width_period_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
> - struct perf_hpp *hpp __maybe_unused)
> -{
> - return 12;
> -}
> -
> -static int hpp__entry_period_baseline(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp,
> - struct hist_entry *he)
> -{
> - struct hist_entry *pair = hist_entry__next_pair(he);
> - u64 period = pair ? pair->stat.period : 0;
> - const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, period);
> -}
> -
> -static int hpp__header_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp)
> -{
> - const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, "Delta");
> -}
> -
> -static int hpp__width_delta(struct perf_hpp_fmt *fmt __maybe_unused,
> - struct perf_hpp *hpp __maybe_unused)
> -{
> - return 7;
> -}
> -
> -static int hpp__entry_delta(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp, struct hist_entry *he)
> -{
> - struct hist_entry *pair = hist_entry__next_pair(he);
> - const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
> - char buf[32] = " ";
> - double diff = 0.0;
> -
> - if (pair) {
> - if (he->diff.computed)
> - diff = he->diff.period_ratio_delta;
> - else
> - diff = perf_diff__compute_delta(he, pair);
> - } else
> - diff = perf_diff__period_percent(he, he->stat.period);
> -
> - if (fabs(diff) >= 0.01)
> - scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, buf);
> -}
> -
> -static int hpp__header_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp)
> -{
> - const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, "Ratio");
> -}
> -
> -static int hpp__width_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp __maybe_unused)
> -{
> - return 14;
> -}
> -
> -static int hpp__entry_ratio(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp, struct hist_entry *he)
> -{
> - struct hist_entry *pair = hist_entry__next_pair(he);
> - const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
> - char buf[32] = " ";
> - double ratio = 0.0;
> -
> - if (pair) {
> - if (he->diff.computed)
> - ratio = he->diff.period_ratio;
> - else
> - ratio = perf_diff__compute_ratio(he, pair);
> - }
> -
> - if (ratio > 0.0)
> - scnprintf(buf, sizeof(buf), "%+14.6F", ratio);
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, buf);
> -}
> -
> -static int hpp__header_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp)
> -{
> - const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, "Weighted diff");
> -}
> -
> -static int hpp__width_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
> - struct perf_hpp *hpp __maybe_unused)
> -{
> - return 14;
> -}
> -
> -static int hpp__entry_wdiff(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp, struct hist_entry *he)
> -{
> - struct hist_entry *pair = hist_entry__next_pair(he);
> - const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
> - char buf[32] = " ";
> - s64 wdiff = 0;
> -
> - if (pair) {
> - if (he->diff.computed)
> - wdiff = he->diff.wdiff;
> - else
> - wdiff = perf_diff__compute_wdiff(he, pair);
> - }
> -
> - if (wdiff != 0)
> - scnprintf(buf, sizeof(buf), "%14ld", wdiff);
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, buf);
> -}
> -
> -static int hpp__header_displ(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp)
> -{
> - return scnprintf(hpp->buf, hpp->size, "Displ.");
> -}
> -
> -static int hpp__width_displ(struct perf_hpp_fmt *fmt __maybe_unused,
> - struct perf_hpp *hpp __maybe_unused)
> -{
> - return 6;
> -}
> -
> -static int hpp__entry_displ(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp, struct hist_entry *he)
> -{
> - struct hist_entry *pair = hist_entry__next_pair(he);
> - long displacement = pair ? pair->position - he->position : 0;
> - const char *fmt = symbol_conf.field_sep ? "%s" : "%6.6s";
> - char buf[32] = " ";
> -
> - if (displacement)
> - scnprintf(buf, sizeof(buf), "%+4ld", displacement);
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, buf);
> -}
> -
> -static int hpp__header_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp)
> -{
> - const char *fmt = symbol_conf.field_sep ? "%s" : "%70s";
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, "Formula");
> -}
> -
> -static int hpp__width_formula(struct perf_hpp_fmt *fmt __maybe_unused,
> - struct perf_hpp *hpp __maybe_unused)
> -{
> - return 70;
> -}
> -
> -static int hpp__entry_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
> - struct perf_hpp *hpp, struct hist_entry *he)
> -{
> - struct hist_entry *pair = hist_entry__next_pair(he);
> - const char *fmt = symbol_conf.field_sep ? "%s" : "%-70s";
> - char buf[96] = " ";
> -
> - if (pair)
> - perf_diff__formula(he, pair, buf, sizeof(buf));
> -
> - return scnprintf(hpp->buf, hpp->size, fmt, buf);
> -}
> -
> #define HPP__COLOR_PRINT_FNS(_name) \
> { \
> .header = hpp__header_ ## _name, \
> @@ -472,7 +237,6 @@ static int hpp__entry_formula(struct perf_hpp_fmt *_fmt __maybe_unused,
> }
>
> struct perf_hpp_fmt perf_hpp__format[] = {
> - HPP__COLOR_PRINT_FNS(baseline),
> HPP__COLOR_PRINT_FNS(overhead),
> HPP__COLOR_PRINT_FNS(overhead_sys),
> HPP__COLOR_PRINT_FNS(overhead_us),
> @@ -480,12 +244,6 @@ struct perf_hpp_fmt perf_hpp__format[] = {
> HPP__COLOR_PRINT_FNS(overhead_guest_us),
> HPP__PRINT_FNS(samples),
> HPP__PRINT_FNS(period),
> - HPP__PRINT_FNS(period_baseline),
> - HPP__PRINT_FNS(delta),
> - HPP__PRINT_FNS(ratio),
> - HPP__PRINT_FNS(wdiff),
> - HPP__PRINT_FNS(displ),
> - HPP__PRINT_FNS(formula)
> };
>
> LIST_HEAD(perf_hpp__list);
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 0c5843b..25f94a4 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -244,8 +244,6 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
> he->ms.map->referenced = true;
> if (symbol_conf.use_callchain)
> callchain_init(he->callchain);
> -
> - INIT_LIST_HEAD(&he->pairs.node);
> }
>
> return he;
> @@ -812,11 +810,11 @@ int hists__link(struct hists *leader, struct hists *other,
> for (nd = rb_first(&other->entries); nd && !ret; nd = rb_next(nd)) {
> pos = rb_entry(nd, struct hist_entry, rb_node);
>
> - if (!hist_entry__has_pairs(pos)) {
> + if (!pos->paired) {
> pair = hists__add_dummy_entry(leader, pos);
> if (pair == NULL)
> return -1;
> - ret = cb(pos, pair, data);
> + ret = cb(pair, pos, data);
> }
> }
>
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index eb4dc83..d4f19eb 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -148,7 +148,7 @@ extern struct list_head perf_hpp__list;
> extern struct perf_hpp_fmt perf_hpp__format[];
>
> enum {
> - PERF_HPP__BASELINE,
> + /* Matches perf_hpp__format array. */
> PERF_HPP__OVERHEAD,
> PERF_HPP__OVERHEAD_SYS,
> PERF_HPP__OVERHEAD_US,
> @@ -156,12 +156,6 @@ enum {
> PERF_HPP__OVERHEAD_GUEST_US,
> PERF_HPP__SAMPLES,
> PERF_HPP__PERIOD,
> - PERF_HPP__PERIOD_BASELINE,
> - PERF_HPP__DELTA,
> - PERF_HPP__RATIO,
> - PERF_HPP__WEIGHTED_DIFF,
> - PERF_HPP__DISPL,
> - PERF_HPP__FORMULA,
>
> PERF_HPP__MAX_INDEX
> };
> @@ -237,5 +231,4 @@ double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair);
> s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair);
> int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair,
> char *buf, size_t size);
> -double perf_diff__period_percent(struct hist_entry *he, u64 period);
> #endif /* __PERF_HIST_H */
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index a884be2..377b144 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -74,18 +74,12 @@ struct hist_entry_diff {
> struct hist_entry {
> struct rb_node rb_node_in;
> struct rb_node rb_node;
> - union {
> - struct list_head node;
> - struct list_head head;
> - } pairs;
> struct he_stat stat;
> struct map_symbol ms;
> struct thread *thread;
> u64 ip;
> s32 cpu;
>
> - struct hist_entry_diff diff;
> -
> /* XXX These two should move to some tree widget lib */
> u16 row_offset;
> u16 nr_rows;
> @@ -98,29 +92,19 @@ struct hist_entry {
> struct symbol *parent;
> unsigned long position;
> struct rb_root sorted_chain;
> +
> + /* diff related */
> + union {
> + struct hist_entry **pairs;
> + bool paired;
> + };
> + struct hist_entry_diff diff;
> +
> struct branch_info *branch_info;
> struct hists *hists;
> struct callchain_root callchain[0];
> };
>
> -static inline bool hist_entry__has_pairs(struct hist_entry *he)
> -{
> - return !list_empty(&he->pairs.node);
> -}
> -
> -static inline struct hist_entry *hist_entry__next_pair(struct hist_entry *he)
> -{
> - if (hist_entry__has_pairs(he))
> - return list_entry(he->pairs.node.next, struct hist_entry, pairs.node);
> - return NULL;
> -}
> -
> -static inline void hist__entry_add_pair(struct hist_entry *he,
> - struct hist_entry *pair)
> -{
> - list_add_tail(&he->pairs.head, &pair->pairs.node);
> -}
> -
> enum sort_type {
> SORT_PID,
> SORT_COMM,
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 09/14] perf diff: Update perf diff documentation for multiple data comparison
2012-11-28 13:52 ` [PATCH 09/14] perf diff: Update perf diff documentation for multiple data comparison Jiri Olsa
@ 2012-11-29 11:52 ` Namhyung Kim
2012-11-29 12:06 ` Jiri Olsa
0 siblings, 1 reply; 39+ messages in thread
From: Namhyung Kim @ 2012-11-29 11:52 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Wed, 28 Nov 2012 14:52:44 +0100, Jiri Olsa wrote:
> +COMPARISON
> +----------
> +The comparison is governed by the baseline file. The baseline perf.data
> +file is iterated for samples. All other perf.data files specified on
> +the command line are searched for the baseline sample pair. If the pair
> +is found, specified computation is made and result is displayed.
> +
> +All samples from non-baseline perf.data files, that do not match any
> +baseline entry, are displayed with empty space within baseline column
> +and possible computation results (delta) in their related column.
> +
> +Example files samples:
> +- file A with samples f1, f2, f3, f4, f6
> +- file B with samples f2, f4, f5
> +- file C with samples f1, f2, f5
> +
> +Example output:
> + x - computation takes place for pair
> + b - baseline sample percentage
> +
> +- perf diff A B C
> +
> + baseline/A compute/B compute/C samples
> + ---------------------------------------
> + b x f1
> + b x x f2
> + b f3
> + b x f4
> + b f6
> + x x f5
> +
> +- perf diff B A C
> +
> + baseline/B compute/A compute/C samples
> + ---------------------------------------
> + b x x f2
> + b x f4
> + b x f5
> + x x f1
> + x f3
> + x f6
> +
> +- perf diff C B A
> +
> + baseline/C compute/B compute/A samples
> + ---------------------------------------
> + b x f1
> + b x x f2
> + b x f5
> + x f3
> + x x f4
> + x f6
> +
It seems not work this way. When I test multiple diff, I didn't get
consecutive 'b's on the baseline column like your example in the cover
letter of this series.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 08/14] perf diff: Change diff command to work over multiple data files
2012-11-29 11:37 ` Namhyung Kim
@ 2012-11-29 11:53 ` Jiri Olsa
0 siblings, 0 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-29 11:53 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Thu, Nov 29, 2012 at 08:37:50PM +0900, Namhyung Kim wrote:
> On Wed, 28 Nov 2012 14:52:43 +0100, Jiri Olsa wrote:
> > Adding diff command the flexibility to specify multiple data
> > files on input. If not input file is given the standard behaviour
> > stands and diff inspects 'perf.data' and 'perf.data.old' files.
> >
> > Also changing the processing and output display for data files.
> >
> > For 'perf diff A B' command:
> >
> > - the current way is to iterate over B data and display
> > A (baseline) data (symbol samples) only if found in B
> >
> > - this patch iterates over A (baseline) data and display
> > B data (symbol samples) only if found in A
> >
> > For 'perf diff A B C' command, the diff now iterates the
> > A (baseline) data and display B and C data (symbol samples)
> > only if they found in A (baseline)
> >
> > All other standard diff command computation features
> > (delta,ratio,wdiff) stays.
>
> This is quite a large change and IMHO can be separated into 3 patches at
> least.
you should have seen this patch before ;-)
>
> 1) change he->pairs from list to array
ok
> 2) introduce diff_data__fmt
> 3) convert to diff_data__fmt
will try ;)
thanks,
jirka
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 03/14] perf tool: Fix period symbol_conf.field_sep display
2012-11-29 7:56 ` Namhyung Kim
@ 2012-11-29 11:53 ` Jiri Olsa
2012-11-29 13:40 ` [PATCH] perf hists: Fix period symbol_conf.field_sep display, again Jiri Olsa
0 siblings, 1 reply; 39+ messages in thread
From: Jiri Olsa @ 2012-11-29 11:53 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Thu, Nov 29, 2012 at 04:56:04PM +0900, Namhyung Kim wrote:
> On Wed, 28 Nov 2012 14:52:38 +0100, Jiri Olsa wrote:
> > Currently we don't properly display hist data with
> > symbol_conf.field_sep separator. We need to display
> > either space or separator.
> >
> > Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> > 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: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/ui/hist.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> > index 5452960..20a4392 100644
> > --- a/tools/perf/ui/hist.c
> > +++ b/tools/perf/ui/hist.c
> > @@ -523,17 +523,13 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
> > struct perf_hpp_fmt *fmt;
> > char *start = hpp->buf;
> > int ret;
> > - bool first = true;
> >
> > if (symbol_conf.exclude_other && !he->parent)
> > return 0;
> >
> > perf_hpp__for_each_format(fmt) {
> > - if (!sep || !first) {
> > - ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
> > - advance_hpp(hpp, ret);
> > - first = false;
> > - }
> > + ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
> > + advance_hpp(hpp, ret);
>
> It will display the separator even before the first column so that the
> output can be messed up with this. I can see that there's a bug setting
> 'first' to false - the line should be moved out of the block otherwise
> it's pointless since we cannot enter the block.
hum, I'll retest
thanks,
jirka
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 10/14] perf tool: Centralize default columns init in perf_hpp__init
2012-11-28 13:52 ` [PATCH 10/14] perf tool: Centralize default columns init in perf_hpp__init Jiri Olsa
@ 2012-11-29 11:55 ` Namhyung Kim
2012-11-29 12:13 ` Jiri Olsa
0 siblings, 1 reply; 39+ messages in thread
From: Namhyung Kim @ 2012-11-29 11:55 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Wed, 28 Nov 2012 14:52:45 +0100, Jiri Olsa wrote:
> Now when diff command is separated from other standard outputs,
> we can use perf_hpp__init to initialize all standard columns.
>
> Moving PERF_HPP__OVERHEAD column init back to perf_hpp__init,
> and removing extra enable calls.
Why was this needed in the first place? AFAIK it's already there and
didn't used only for perf diff.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 14/14] perf diff: Add generic order option for compute sorting
2012-11-28 13:52 ` [PATCH 14/14] perf diff: Add generic order option for compute sorting Jiri Olsa
@ 2012-11-29 12:02 ` Namhyung Kim
2012-11-29 12:19 ` Jiri Olsa
0 siblings, 1 reply; 39+ messages in thread
From: Namhyung Kim @ 2012-11-29 12:02 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Wed, 28 Nov 2012 14:52:49 +0100, Jiri Olsa wrote:
> Adding option 'o' to allow sorting based on the
> input file number.
[snip]
> hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
> int c)
> {
> - int i;
> + struct hist_entry **pairs_left = left->pairs;
> + struct hist_entry **pairs_right = right->pairs;
> + struct hist_entry *p_right, *p_left;
> + static int64_t cmp;
>
> - for (i = 0; i < data_cnt; i++) {
> - struct hist_entry **pairs_left = left->pairs;
> - struct hist_entry **pairs_right = right->pairs;
> - struct hist_entry *p_right, *p_left;
> - static int64_t cmp;
> + if (!pairs_left || !pairs_right)
> + return pairs_left ? -1 : 1;
>
> - if (!pairs_left || !pairs_right)
> - return pairs_right - pairs_left;
> + p_right = pairs_right[sort_compute];
> + p_left = pairs_left[sort_compute];
>
> - p_right = pairs_right[i];
> - p_left = pairs_left[i];
> + if (!p_left || !p_right)
> + return p_left ? -1 : 1;
What if both p_left and p_right are NULL? Shouldn't it be move to the
next pairs?
>
> - if (!p_left || !p_right)
> - return p_right - p_left;
> -
> - /*
> - * If we differ, we are done, otherwise continue until all
> - * is processed or we find a difference.
> - */
> - cmp = __hist_entry__cmp_compute(p_left, p_right, c);
> - if (cmp)
> - return cmp;
> - }
> + /*
> + * If we differ, we are done, otherwise continue until all
> + * is processed or we find a difference.
> + */
I guess this comment is not applied anymore. Or we need a loop after
checking sort_compute column, right?
Thanks,
Namhyung
> + cmp = __hist_entry__cmp_compute(p_left, p_right, c);
> + if (cmp)
> + return cmp;
>
> return 0;
> }
> @@ -759,6 +748,7 @@ static const struct option options[] = {
> "columns '.' is reserved."),
> OPT_STRING(0, "symfs", &symbol_conf.symfs, "directory",
> "Look for files with symbols relative to this directory"),
> + OPT_UINTEGER('o', "order", &sort_compute, "Specify compute sorting."),
> OPT_END()
> };
>
> @@ -1087,6 +1077,11 @@ static int data_init(int argc, const char **argv)
> d->idx = i;
> }
>
> + if (sort_compute >= (unsigned int) data_cnt) {
> + pr_err("Order option out of limit.\n");
> + return -EINVAL;
> + }
> +
> return 0;
> }
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 09/14] perf diff: Update perf diff documentation for multiple data comparison
2012-11-29 11:52 ` Namhyung Kim
@ 2012-11-29 12:06 ` Jiri Olsa
0 siblings, 0 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-29 12:06 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Thu, Nov 29, 2012 at 08:52:18PM +0900, Namhyung Kim wrote:
> On Wed, 28 Nov 2012 14:52:44 +0100, Jiri Olsa wrote:
> > +COMPARISON
> > +----------
> > +The comparison is governed by the baseline file. The baseline perf.data
> > +file is iterated for samples. All other perf.data files specified on
> > +the command line are searched for the baseline sample pair. If the pair
> > +is found, specified computation is made and result is displayed.
> > +
> > +All samples from non-baseline perf.data files, that do not match any
> > +baseline entry, are displayed with empty space within baseline column
> > +and possible computation results (delta) in their related column.
> > +
> > +Example files samples:
> > +- file A with samples f1, f2, f3, f4, f6
> > +- file B with samples f2, f4, f5
> > +- file C with samples f1, f2, f5
> > +
> > +Example output:
> > + x - computation takes place for pair
> > + b - baseline sample percentage
> > +
> > +- perf diff A B C
> > +
> > + baseline/A compute/B compute/C samples
> > + ---------------------------------------
> > + b x f1
> > + b x x f2
> > + b f3
> > + b x f4
> > + b f6
> > + x x f5
> > +
> > +- perf diff B A C
> > +
> > + baseline/B compute/A compute/C samples
> > + ---------------------------------------
> > + b x x f2
> > + b x f4
> > + b x f5
> > + x x f1
> > + x f3
> > + x f6
> > +
> > +- perf diff C B A
> > +
> > + baseline/C compute/B compute/A samples
> > + ---------------------------------------
> > + b x f1
> > + b x x f2
> > + b x f5
> > + x f3
> > + x x f4
> > + x f6
> > +
>
> It seems not work this way. When I test multiple diff, I didn't get
> consecutive 'b's on the baseline column like your example in the cover
> letter of this series.
the baseline consecutiveness is just for sake of the doc readibility
it just presents logical output to get user some idea
final output could be sorted by column data using -o <column number>
the initial diff output (no options) is sort of sorted by baseline ;)
you're right, that should be fixed.. by default sorted by baseline
jirka
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 10/14] perf tool: Centralize default columns init in perf_hpp__init
2012-11-29 11:55 ` Namhyung Kim
@ 2012-11-29 12:13 ` Jiri Olsa
2012-11-30 5:06 ` Namhyung Kim
0 siblings, 1 reply; 39+ messages in thread
From: Jiri Olsa @ 2012-11-29 12:13 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Thu, Nov 29, 2012 at 08:55:47PM +0900, Namhyung Kim wrote:
> On Wed, 28 Nov 2012 14:52:45 +0100, Jiri Olsa wrote:
> > Now when diff command is separated from other standard outputs,
> > we can use perf_hpp__init to initialize all standard columns.
> >
> > Moving PERF_HPP__OVERHEAD column init back to perf_hpp__init,
> > and removing extra enable calls.
>
> Why was this needed in the first place? AFAIK it's already there and
> didn't used only for perf diff.
hm, I think PERF_HPP__OVERHEAD wasn't part of perf_hpp__init and every
report except for diff command is using it.. so I think it makes sense
to move it to perf_hpp__init.. maybe I'm missing something.
The diff command is not using perf_hpp__init, it's building its own
PERF_HPP columns
jirka
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 14/14] perf diff: Add generic order option for compute sorting
2012-11-29 12:02 ` Namhyung Kim
@ 2012-11-29 12:19 ` Jiri Olsa
0 siblings, 0 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-29 12:19 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Thu, Nov 29, 2012 at 09:02:01PM +0900, Namhyung Kim wrote:
> On Wed, 28 Nov 2012 14:52:49 +0100, Jiri Olsa wrote:
> > Adding option 'o' to allow sorting based on the
> > input file number.
> [snip]
> > hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
> > int c)
> > {
> > - int i;
> > + struct hist_entry **pairs_left = left->pairs;
> > + struct hist_entry **pairs_right = right->pairs;
> > + struct hist_entry *p_right, *p_left;
> > + static int64_t cmp;
> >
> > - for (i = 0; i < data_cnt; i++) {
> > - struct hist_entry **pairs_left = left->pairs;
> > - struct hist_entry **pairs_right = right->pairs;
> > - struct hist_entry *p_right, *p_left;
> > - static int64_t cmp;
> > + if (!pairs_left || !pairs_right)
> > + return pairs_left ? -1 : 1;
> >
> > - if (!pairs_left || !pairs_right)
> > - return pairs_right - pairs_left;
> > + p_right = pairs_right[sort_compute];
> > + p_left = pairs_left[sort_compute];
> >
> > - p_right = pairs_right[i];
> > - p_left = pairs_left[i];
> > + if (!p_left || !p_right)
> > + return p_left ? -1 : 1;
>
> What if both p_left and p_right are NULL? Shouldn't it be move to the
> next pairs?
hm, right.. we bail out, but not sure what to return.. 0,-1 or 1
>
> >
> > - if (!p_left || !p_right)
> > - return p_right - p_left;
> > -
> > - /*
> > - * If we differ, we are done, otherwise continue until all
> > - * is processed or we find a difference.
> > - */
> > - cmp = __hist_entry__cmp_compute(p_left, p_right, c);
> > - if (cmp)
> > - return cmp;
> > - }
> > + /*
> > + * If we differ, we are done, otherwise continue until all
> > + * is processed or we find a difference.
> > + */
>
> I guess this comment is not applied anymore. Or we need a loop after
> checking sort_compute column, right?
unfortunatelly formated.. if we get this far, we have numbers to compare..
meaning: we have 2 lines that have same, non empty column to compare
thanks,
jirka
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] perf hists: Fix period symbol_conf.field_sep display, again
2012-11-29 11:53 ` Jiri Olsa
@ 2012-11-29 13:40 ` Jiri Olsa
2012-11-29 17:35 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 39+ messages in thread
From: Jiri Olsa @ 2012-11-29 13:40 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Thu, Nov 29, 2012 at 12:53:44PM +0100, Jiri Olsa wrote:
> On Thu, Nov 29, 2012 at 04:56:04PM +0900, Namhyung Kim wrote:
SNIP
> > > - if (!sep || !first) {
> > > - ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
> > > - advance_hpp(hpp, ret);
> > > - first = false;
> > > - }
> > > + ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
> > > + advance_hpp(hpp, ret);
> >
> > It will display the separator even before the first column so that the
> > output can be messed up with this. I can see that there's a bug setting
> > 'first' to false - the line should be moved out of the block otherwise
> > it's pointless since we cannot enter the block.
>
> hum, I'll retest
you're right, fix is attached
Arnaldo, I know you've already pulled this one.. I can resend v2
if needed, which is shorter (just that 'first = false' move as
Namhyyung said)
thanks,
jirka
---
Last fix of this place was wrong and caused the separator
field to be displayed even before period column.
Should be fixed properly this time by introducing 'first' bool
like on other places doing the same stuff.
Screwed-up-by: Jiri Olsa <jolsa@redhat.com>
Reported-by: Namhyung Kim <namhyung@kernel.org>
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>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/ui/hist.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 2447e0c..6e639b5 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -466,13 +466,21 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
struct perf_hpp_fmt *fmt;
char *start = hpp->buf;
int ret;
+ bool first = true;
if (symbol_conf.exclude_other && !he->parent)
return 0;
perf_hpp__for_each_format(fmt) {
- ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
- advance_hpp(hpp, ret);
+ /*
+ * If there's no field_sep, we still need
+ * to display initial ' '.
+ */
+ if (!sep || !first) {
+ ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
+ advance_hpp(hpp, ret);
+ } else
+ first = false;
if (color && fmt->color)
ret = fmt->color(hpp, he);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH] perf hists: Fix period symbol_conf.field_sep display, again
2012-11-29 13:40 ` [PATCH] perf hists: Fix period symbol_conf.field_sep display, again Jiri Olsa
@ 2012-11-29 17:35 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 39+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-11-29 17:35 UTC (permalink / raw)
To: Jiri Olsa
Cc: Namhyung Kim, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker
Em Thu, Nov 29, 2012 at 02:40:02PM +0100, Jiri Olsa escreveu:
> On Thu, Nov 29, 2012 at 12:53:44PM +0100, Jiri Olsa wrote:
> > On Thu, Nov 29, 2012 at 04:56:04PM +0900, Namhyung Kim wrote:
>
> SNIP
>
> > > > - if (!sep || !first) {
> > > > - ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
> > > > - advance_hpp(hpp, ret);
> > > > - first = false;
> > > > - }
> > > > + ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
> > > > + advance_hpp(hpp, ret);
> > >
> > > It will display the separator even before the first column so that the
> > > output can be messed up with this. I can see that there's a bug setting
> > > 'first' to false - the line should be moved out of the block otherwise
> > > it's pointless since we cannot enter the block.
> >
> > hum, I'll retest
>
> you're right, fix is attached
>
> Arnaldo, I know you've already pulled this one.. I can resend v2
> if needed, which is shorter (just that 'first = false' move as
> Namhyyung said)
I just folded it, was the tip of the tree, will go on processing the
others now.
> thanks,
> jirka
>
>
> ---
> Last fix of this place was wrong and caused the separator
> field to be displayed even before period column.
>
> Should be fixed properly this time by introducing 'first' bool
> like on other places doing the same stuff.
>
> Screwed-up-by: Jiri Olsa <jolsa@redhat.com>
> Reported-by: Namhyung Kim <namhyung@kernel.org>
> 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>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
> tools/perf/ui/hist.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
> index 2447e0c..6e639b5 100644
> --- a/tools/perf/ui/hist.c
> +++ b/tools/perf/ui/hist.c
> @@ -466,13 +466,21 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
> struct perf_hpp_fmt *fmt;
> char *start = hpp->buf;
> int ret;
> + bool first = true;
>
> if (symbol_conf.exclude_other && !he->parent)
> return 0;
>
> perf_hpp__for_each_format(fmt) {
> - ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
> - advance_hpp(hpp, ret);
> + /*
> + * If there's no field_sep, we still need
> + * to display initial ' '.
> + */
> + if (!sep || !first) {
> + ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
> + advance_hpp(hpp, ret);
> + } else
> + first = false;
>
> if (color && fmt->color)
> ret = fmt->color(hpp, he);
> --
> 1.7.11.7
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 10/14] perf tool: Centralize default columns init in perf_hpp__init
2012-11-29 12:13 ` Jiri Olsa
@ 2012-11-30 5:06 ` Namhyung Kim
2012-11-30 11:56 ` Jiri Olsa
0 siblings, 1 reply; 39+ messages in thread
From: Namhyung Kim @ 2012-11-30 5:06 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
Hi Jiri,
On Thu, 29 Nov 2012 13:13:19 +0100, Jiri Olsa wrote:
> On Thu, Nov 29, 2012 at 08:55:47PM +0900, Namhyung Kim wrote:
>> On Wed, 28 Nov 2012 14:52:45 +0100, Jiri Olsa wrote:
>> > Now when diff command is separated from other standard outputs,
>> > we can use perf_hpp__init to initialize all standard columns.
>> >
>> > Moving PERF_HPP__OVERHEAD column init back to perf_hpp__init,
>> > and removing extra enable calls.
>>
>> Why was this needed in the first place? AFAIK it's already there and
>> didn't used only for perf diff.
>
> hm, I think PERF_HPP__OVERHEAD wasn't part of perf_hpp__init and every
> report except for diff command is using it.. so I think it makes sense
> to move it to perf_hpp__init.. maybe I'm missing something.
You're right. The _OVERHEAD column was enabled by default but wasn't
part of the _init function - sorry for the confusion. But what I try to
say was that it can be folded into the patch 1.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 10/14] perf tool: Centralize default columns init in perf_hpp__init
2012-11-30 5:06 ` Namhyung Kim
@ 2012-11-30 11:56 ` Jiri Olsa
0 siblings, 0 replies; 39+ messages in thread
From: Jiri Olsa @ 2012-11-30 11:56 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Fri, Nov 30, 2012 at 02:06:51PM +0900, Namhyung Kim wrote:
> Hi Jiri,
>
> On Thu, 29 Nov 2012 13:13:19 +0100, Jiri Olsa wrote:
> > On Thu, Nov 29, 2012 at 08:55:47PM +0900, Namhyung Kim wrote:
> >> On Wed, 28 Nov 2012 14:52:45 +0100, Jiri Olsa wrote:
> >> > Now when diff command is separated from other standard outputs,
> >> > we can use perf_hpp__init to initialize all standard columns.
> >> >
> >> > Moving PERF_HPP__OVERHEAD column init back to perf_hpp__init,
> >> > and removing extra enable calls.
> >>
> >> Why was this needed in the first place? AFAIK it's already there and
> >> didn't used only for perf diff.
> >
> > hm, I think PERF_HPP__OVERHEAD wasn't part of perf_hpp__init and every
> > report except for diff command is using it.. so I think it makes sense
> > to move it to perf_hpp__init.. maybe I'm missing something.
>
> You're right. The _OVERHEAD column was enabled by default but wasn't
> part of the _init function - sorry for the confusion. But what I try to
> say was that it can be folded into the patch 1.
hm, probably yes.. I'll check
thanks,
jirka
^ permalink raw reply [flat|nested] 39+ messages in thread
* [tip:perf/core] perf diff: Remove displacement from struct hist_entry_diff
2012-11-28 13:52 ` [PATCH 04/14] perf diff: Remove displacement from struct hist_entry_diff Jiri Olsa
2012-11-29 8:00 ` Namhyung Kim
@ 2013-01-24 18:50 ` tip-bot for Jiri Olsa
1 sibling, 0 replies; 39+ messages in thread
From: tip-bot for Jiri Olsa @ 2013-01-24 18:50 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, namhyung,
jolsa, fweisbec, tglx, cjashfor, mingo
Commit-ID: fa283ada1606e687641dc2b157d66ef0f9c9aa8a
Gitweb: http://git.kernel.org/tip/fa283ada1606e687641dc2b157d66ef0f9c9aa8a
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Wed, 28 Nov 2012 14:52:39 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sun, 9 Dec 2012 08:46:06 -0300
perf diff: Remove displacement from struct hist_entry_diff
Removing displacement from struct hist_entry_diff, because it's not
used. Displacement is not used for sorting, so there's no reason to
pre-calculate it.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1354110769-2998-5-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/sort.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 91ae274..a1c0d56 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -55,9 +55,6 @@ struct he_stat {
struct hist_entry_diff {
bool computed;
- /* PERF_HPP__DISPL */
- int displacement;
-
/* PERF_HPP__DELTA */
double period_ratio_delta;
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [tip:perf/core] perf diff: Change compute methods to work with pair directly
2012-11-28 13:52 ` [PATCH 05/14] perf diff: Change compute methods to work with pair directly Jiri Olsa
2012-11-29 11:15 ` Namhyung Kim
@ 2013-01-24 18:51 ` tip-bot for Jiri Olsa
1 sibling, 0 replies; 39+ messages in thread
From: tip-bot for Jiri Olsa @ 2013-01-24 18:51 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, namhyung,
jolsa, fweisbec, tglx, cjashfor, mingo
Commit-ID: 05472daa4d8ab88a071bfcaa3bb47473e4071848
Gitweb: http://git.kernel.org/tip/05472daa4d8ab88a071bfcaa3bb47473e4071848
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Wed, 28 Nov 2012 14:52:40 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sun, 9 Dec 2012 08:46:06 -0300
perf diff: Change compute methods to work with pair directly
Changing compute methods to operate over hist entry and its pair
directly. This makes the code more obvious and readable, instead of all
time checking for pair being != NULL.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1354110769-2998-6-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-diff.c | 38 +++++++++++++++++---------------------
tools/perf/ui/hist.c | 40 +++++++++++++++++++++++++---------------
tools/perf/util/hist.h | 7 ++++---
3 files changed, 46 insertions(+), 39 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 9fbbc01..342085a 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -146,47 +146,40 @@ static int setup_compute(const struct option *opt, const char *str,
return -EINVAL;
}
-static double get_period_percent(struct hist_entry *he, u64 period)
+double perf_diff__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)
+double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
- double new_percent = get_period_percent(he, he->stat.period);
- double old_percent = pair ? get_period_percent(pair, pair->stat.period) : 0.0;
+ double new_percent = perf_diff__period_percent(he, he->stat.period);
+ double old_percent = perf_diff__period_percent(pair, pair->stat.period);
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)
+double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
double new_period = he->stat.period;
- double old_period = pair ? pair->stat.period : 0;
+ double old_period = pair->stat.period;
he->diff.computed = true;
- he->diff.period_ratio = pair ? (new_period / old_period) : 0;
+ he->diff.period_ratio = new_period / old_period;
return he->diff.period_ratio;
}
-s64 perf_diff__compute_wdiff(struct hist_entry *he)
+s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
u64 new_period = he->stat.period;
- u64 old_period = pair ? pair->stat.period : 0;
+ u64 old_period = pair->stat.period;
he->diff.computed = true;
-
- if (!pair)
- he->diff.wdiff = 0;
- else
- he->diff.wdiff = new_period * compute_wdiff_w2 -
- old_period * compute_wdiff_w1;
+ he->diff.wdiff = new_period * compute_wdiff_w2 -
+ old_period * compute_wdiff_w1;
return he->diff.wdiff;
}
@@ -385,18 +378,21 @@ static void hists__precompute(struct hists *hists)
while (next != NULL) {
struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
+ struct hist_entry *pair = hist_entry__next_pair(he);
next = rb_next(&he->rb_node);
+ if (!pair)
+ continue;
switch (compute) {
case COMPUTE_DELTA:
- perf_diff__compute_delta(he);
+ perf_diff__compute_delta(he, pair);
break;
case COMPUTE_RATIO:
- perf_diff__compute_ratio(he);
+ perf_diff__compute_ratio(he, pair);
break;
case COMPUTE_WEIGHTED_DIFF:
- perf_diff__compute_wdiff(he);
+ perf_diff__compute_wdiff(he, pair);
break;
default:
BUG_ON(1);
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 6e639b5..108e5ed 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -268,14 +268,18 @@ static int hpp__width_delta(struct perf_hpp *hpp __maybe_unused)
static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
{
+ struct hist_entry *pair = hist_entry__next_pair(he);
const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
char buf[32] = " ";
- double diff;
+ double diff = 0.0;
- if (he->diff.computed)
- diff = he->diff.period_ratio_delta;
- else
- diff = perf_diff__compute_delta(he);
+ if (pair) {
+ if (he->diff.computed)
+ diff = he->diff.period_ratio_delta;
+ else
+ diff = perf_diff__compute_delta(he, pair);
+ } else
+ diff = perf_diff__period_percent(he, he->stat.period);
if (fabs(diff) >= 0.01)
scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
@@ -297,14 +301,17 @@ static int hpp__width_ratio(struct perf_hpp *hpp __maybe_unused)
static int hpp__entry_ratio(struct perf_hpp *hpp, struct hist_entry *he)
{
+ struct hist_entry *pair = hist_entry__next_pair(he);
const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
char buf[32] = " ";
- double ratio;
+ double ratio = 0.0;
- if (he->diff.computed)
- ratio = he->diff.period_ratio;
- else
- ratio = perf_diff__compute_ratio(he);
+ if (pair) {
+ if (he->diff.computed)
+ ratio = he->diff.period_ratio;
+ else
+ ratio = perf_diff__compute_ratio(he, pair);
+ }
if (ratio > 0.0)
scnprintf(buf, sizeof(buf), "%+14.6F", ratio);
@@ -326,14 +333,17 @@ static int hpp__width_wdiff(struct perf_hpp *hpp __maybe_unused)
static int hpp__entry_wdiff(struct perf_hpp *hpp, struct hist_entry *he)
{
+ struct hist_entry *pair = hist_entry__next_pair(he);
const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
char buf[32] = " ";
- s64 wdiff;
+ s64 wdiff = 0;
- if (he->diff.computed)
- wdiff = he->diff.wdiff;
- else
- wdiff = perf_diff__compute_wdiff(he);
+ if (pair) {
+ if (he->diff.computed)
+ wdiff = he->diff.wdiff;
+ else
+ wdiff = perf_diff__compute_wdiff(he, pair);
+ }
if (wdiff != 0)
scnprintf(buf, sizeof(buf), "%14ld", wdiff);
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index a935a60..235503a 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -226,8 +226,9 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist __maybe_unused,
unsigned int hists__sort_list_width(struct hists *self);
-double perf_diff__compute_delta(struct hist_entry *he);
-double perf_diff__compute_ratio(struct hist_entry *he);
-s64 perf_diff__compute_wdiff(struct hist_entry *he);
+double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair);
+double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair);
+s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair);
int perf_diff__formula(char *buf, size_t size, struct hist_entry *he);
+double perf_diff__period_percent(struct hist_entry *he, u64 period);
#endif /* __PERF_HIST_H */
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [tip:perf/core] perf diff: Change formula methods to work with pair directly
2012-11-28 13:52 ` [PATCH 06/14] perf diff: Change formula " Jiri Olsa
2012-11-29 11:15 ` Namhyung Kim
@ 2013-01-24 18:52 ` tip-bot for Jiri Olsa
1 sibling, 0 replies; 39+ messages in thread
From: tip-bot for Jiri Olsa @ 2013-01-24 18:52 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, namhyung,
jolsa, fweisbec, tglx, cjashfor, mingo
Commit-ID: f4c8bae1920c459b7b9c12363d11e8a588862e42
Gitweb: http://git.kernel.org/tip/f4c8bae1920c459b7b9c12363d11e8a588862e42
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Wed, 28 Nov 2012 14:52:41 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sun, 9 Dec 2012 08:46:07 -0300
perf diff: Change formula methods to work with pair directly
Changing formula methods to operate over hist entry and its pair
directly. This makes the code more obvious and readable, instead of all
time checking for pair being != NULL.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1354110769-2998-7-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-diff.c | 35 +++++++++++++----------------------
tools/perf/ui/hist.c | 5 ++++-
tools/perf/util/hist.h | 3 ++-
3 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 342085a..d869029 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -184,13 +184,9 @@ s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair)
return he->diff.wdiff;
}
-static int formula_delta(struct hist_entry *he, char *buf, size_t size)
+static int formula_delta(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
-
- if (!pair)
- return -1;
-
return scnprintf(buf, size,
"(%" PRIu64 " * 100 / %" PRIu64 ") - "
"(%" PRIu64 " * 100 / %" PRIu64 ")",
@@ -198,41 +194,36 @@ static int formula_delta(struct hist_entry *he, char *buf, size_t size)
pair->stat.period, pair->hists->stats.total_period);
}
-static int formula_ratio(struct hist_entry *he, char *buf, size_t size)
+static int formula_ratio(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
double new_period = he->stat.period;
- double old_period = pair ? pair->stat.period : 0;
-
- if (!pair)
- return -1;
+ double old_period = pair->stat.period;
return scnprintf(buf, size, "%.0F / %.0F", new_period, old_period);
}
-static int formula_wdiff(struct hist_entry *he, char *buf, size_t size)
+static int formula_wdiff(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size)
{
- struct hist_entry *pair = hist_entry__next_pair(he);
u64 new_period = he->stat.period;
- u64 old_period = pair ? pair->stat.period : 0;
-
- if (!pair)
- return -1;
+ u64 old_period = pair->stat.period;
return scnprintf(buf, size,
"(%" PRIu64 " * " "%" PRId64 ") - (%" PRIu64 " * " "%" PRId64 ")",
new_period, compute_wdiff_w2, old_period, compute_wdiff_w1);
}
-int perf_diff__formula(char *buf, size_t size, struct hist_entry *he)
+int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size)
{
switch (compute) {
case COMPUTE_DELTA:
- return formula_delta(he, buf, size);
+ return formula_delta(he, pair, buf, size);
case COMPUTE_RATIO:
- return formula_ratio(he, buf, size);
+ return formula_ratio(he, pair, buf, size);
case COMPUTE_WEIGHTED_DIFF:
- return formula_wdiff(he, buf, size);
+ return formula_wdiff(he, pair, buf, size);
default:
BUG_ON(1);
}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 108e5ed..1785bab 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -389,10 +389,13 @@ static int hpp__width_formula(struct perf_hpp *hpp __maybe_unused)
static int hpp__entry_formula(struct perf_hpp *hpp, struct hist_entry *he)
{
+ struct hist_entry *pair = hist_entry__next_pair(he);
const char *fmt = symbol_conf.field_sep ? "%s" : "%-70s";
char buf[96] = " ";
- perf_diff__formula(buf, sizeof(buf), he);
+ if (pair)
+ perf_diff__formula(he, pair, buf, sizeof(buf));
+
return scnprintf(hpp->buf, hpp->size, fmt, buf);
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 235503a..c1b2fad 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -229,6 +229,7 @@ unsigned int hists__sort_list_width(struct hists *self);
double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair);
double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair);
s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair);
-int perf_diff__formula(char *buf, size_t size, struct hist_entry *he);
+int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair,
+ char *buf, size_t size);
double perf_diff__period_percent(struct hist_entry *he, u64 period);
#endif /* __PERF_HIST_H */
^ permalink raw reply related [flat|nested] 39+ messages in thread
end of thread, other threads:[~2013-01-24 18:52 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-28 13:52 [PATCH/RFC 00/14] perf, tool: Fix endian issues Jiri Olsa
2012-11-28 13:52 ` [PATCH 01/14] perf tool: Introduce perf_hpp__list for period related columns Jiri Olsa
2012-11-29 7:44 ` Namhyung Kim
2012-11-29 10:52 ` Namhyung Kim
2012-11-28 13:52 ` [PATCH 02/14] perf tool: Add struct perf_hpp_fmt into hpp callbacks Jiri Olsa
2012-11-28 13:52 ` [PATCH 03/14] perf tool: Fix period symbol_conf.field_sep display Jiri Olsa
2012-11-29 7:56 ` Namhyung Kim
2012-11-29 11:53 ` Jiri Olsa
2012-11-29 13:40 ` [PATCH] perf hists: Fix period symbol_conf.field_sep display, again Jiri Olsa
2012-11-29 17:35 ` Arnaldo Carvalho de Melo
2012-11-28 13:52 ` [PATCH 04/14] perf diff: Remove displacement from struct hist_entry_diff Jiri Olsa
2012-11-29 8:00 ` Namhyung Kim
2013-01-24 18:50 ` [tip:perf/core] " tip-bot for Jiri Olsa
2012-11-28 13:52 ` [PATCH 05/14] perf diff: Change compute methods to work with pair directly Jiri Olsa
2012-11-29 11:15 ` Namhyung Kim
2013-01-24 18:51 ` [tip:perf/core] " tip-bot for Jiri Olsa
2012-11-28 13:52 ` [PATCH 06/14] perf diff: Change formula " Jiri Olsa
2012-11-29 11:15 ` Namhyung Kim
2013-01-24 18:52 ` [tip:perf/core] " tip-bot for Jiri Olsa
2012-11-28 13:52 ` [PATCH 07/14] perf diff: Add callback to hists__match/hists__link functions Jiri Olsa
2012-11-28 13:52 ` [PATCH 08/14] perf diff: Change diff command to work over multiple data files Jiri Olsa
2012-11-29 11:37 ` Namhyung Kim
2012-11-29 11:53 ` Jiri Olsa
2012-11-28 13:52 ` [PATCH 09/14] perf diff: Update perf diff documentation for multiple data comparison Jiri Olsa
2012-11-29 11:52 ` Namhyung Kim
2012-11-29 12:06 ` Jiri Olsa
2012-11-28 13:52 ` [PATCH 10/14] perf tool: Centralize default columns init in perf_hpp__init Jiri Olsa
2012-11-29 11:55 ` Namhyung Kim
2012-11-29 12:13 ` Jiri Olsa
2012-11-30 5:06 ` Namhyung Kim
2012-11-30 11:56 ` Jiri Olsa
2012-11-28 13:52 ` [PATCH 11/14] perf diff: Making compute functions static Jiri Olsa
2012-11-28 13:52 ` [PATCH 12/14] perf diff: Display data file info ahead of the diff output Jiri Olsa
2012-11-28 13:52 ` [PATCH 13/14] perf diff: Display zero calculation results Jiri Olsa
2012-11-28 13:52 ` [PATCH 14/14] perf diff: Add generic order option for compute sorting Jiri Olsa
2012-11-29 12:02 ` Namhyung Kim
2012-11-29 12:19 ` Jiri Olsa
2012-11-28 13:56 ` [PATCH/RFC 00/14] perf, diff: Support for multiple files Jiri Olsa
2012-11-28 15:57 ` Jiri Olsa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).