* [PATCH 01/14] perf hists: Add struct hists pointer to struct hist_entry
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 02/14] perf diff: Refactor diff displacement possition info Jiri Olsa
` (13 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Adding pointer back to the parent struct hists for struct hists_entry.
This will be useful in future for any hist_entry's data computation,
that depends on total data of its parent hists.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/util/hist.c | 2 ++
tools/perf/util/sort.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 236bc9d..040f34c 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -325,6 +325,7 @@ struct hist_entry *__hists__add_branch_entry(struct hists *self,
.parent = sym_parent,
.filtered = symbol__parent_filter(sym_parent),
.branch_info = bi,
+ .hists = self,
};
return add_hist_entry(self, &entry, al, period);
@@ -346,6 +347,7 @@ struct hist_entry *__hists__add_entry(struct hists *self,
.period = period,
.parent = sym_parent,
.filtered = symbol__parent_filter(sym_parent),
+ .hists = self,
};
return add_hist_entry(self, &entry, al, period);
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 12d6347..eb3959b 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -79,6 +79,7 @@ struct hist_entry {
struct rb_root sorted_chain;
};
struct branch_info *branch_info;
+ struct hists *hists;
struct callchain_root callchain[0];
};
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 02/14] perf diff: Refactor diff displacement possition info
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
2012-09-27 11:09 ` [PATCH 01/14] perf hists: Add struct hists pointer to struct hist_entry Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 03/14] perf hists: Separate overhead and baseline columns Jiri Olsa
` (12 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Moving the position calculation into the diff command, so the position
is prepared inside struct hist_entry data and there's no need to compute
in the output display path.
Removing 'displacement' from struct perf_hpp as it is no longer needed.
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>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/builtin-diff.c | 49 +++++++++++++++++++++++++++++----------------
tools/perf/builtin-report.c | 2 +-
tools/perf/builtin-top.c | 2 +-
tools/perf/ui/hist.c | 8 +++++---
tools/perf/ui/stdio/hist.c | 17 +++-------------
tools/perf/util/hist.h | 4 +---
tools/perf/util/sort.h | 2 +-
7 files changed, 44 insertions(+), 40 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 761f419..5cb577a 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -70,8 +70,8 @@ static struct perf_tool tool = {
.ordering_requires_timestamps = true,
};
-static void perf_session__insert_hist_entry_by_name(struct rb_root *root,
- struct hist_entry *he)
+static void insert_hist_entry_by_name(struct rb_root *root,
+ struct hist_entry *he)
{
struct rb_node **p = &root->rb_node;
struct rb_node *parent = NULL;
@@ -90,7 +90,7 @@ static void perf_session__insert_hist_entry_by_name(struct rb_root *root,
rb_insert_color(&he->rb_node, root);
}
-static void hists__resort_entries(struct hists *self)
+static void hists__name_resort(struct hists *self, bool sort)
{
unsigned long position = 1;
struct rb_root tmp = RB_ROOT;
@@ -100,12 +100,16 @@ static void hists__resort_entries(struct hists *self)
struct hist_entry *n = rb_entry(next, struct hist_entry, rb_node);
next = rb_next(&n->rb_node);
- rb_erase(&n->rb_node, &self->entries);
n->position = position++;
- perf_session__insert_hist_entry_by_name(&tmp, n);
+
+ if (sort) {
+ rb_erase(&n->rb_node, &self->entries);
+ insert_hist_entry_by_name(&tmp, n);
+ }
}
- self->entries = tmp;
+ if (sort)
+ self->entries = tmp;
}
static struct hist_entry *hists__find_entry(struct hists *self,
@@ -121,7 +125,7 @@ static struct hist_entry *hists__find_entry(struct hists *self,
n = n->rb_left;
else if (cmp > 0)
n = n->rb_right;
- else
+ else
return iter;
}
@@ -150,6 +154,24 @@ static struct perf_evsel *evsel_match(struct perf_evsel *evsel,
return NULL;
}
+static void perf_evlist__resort_hists(struct perf_evlist *evlist, bool name)
+{
+ struct perf_evsel *evsel;
+
+ list_for_each_entry(evsel, &evlist->entries, node) {
+ struct hists *hists = &evsel->hists;
+
+ hists__output_resort(hists);
+
+ /*
+ * The hists__name_resort only sets possition
+ * if name is false.
+ */
+ if (name || ((!name) && show_displacement))
+ hists__name_resort(hists, name);
+ }
+}
+
static int __cmd_diff(void)
{
int ret, i;
@@ -176,15 +198,8 @@ static int __cmd_diff(void)
evlist_old = older->evlist;
evlist_new = newer->evlist;
- list_for_each_entry(evsel, &evlist_new->entries, node)
- hists__output_resort(&evsel->hists);
-
- list_for_each_entry(evsel, &evlist_old->entries, node) {
- hists__output_resort(&evsel->hists);
-
- if (show_displacement)
- hists__resort_entries(&evsel->hists);
- }
+ perf_evlist__resort_hists(evlist_old, true);
+ perf_evlist__resort_hists(evlist_new, false);
list_for_each_entry(evsel, &evlist_new->entries, node) {
struct perf_evsel *evsel_old;
@@ -200,7 +215,7 @@ static int __cmd_diff(void)
hists__match(&evsel_old->hists, &evsel->hists);
hists__fprintf(&evsel->hists, &evsel_old->hists,
- show_displacement, true, 0, 0, stdout);
+ true, 0, 0, stdout);
}
out_delete:
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 1da243d..6748cac 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -320,7 +320,7 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
const char *evname = perf_evsel__name(pos);
hists__fprintf_nr_sample_events(hists, evname, stdout);
- hists__fprintf(hists, NULL, false, true, 0, 0, stdout);
+ hists__fprintf(hists, NULL, true, 0, 0, stdout);
fprintf(stdout, "\n\n");
}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index e434a16..6b8c629 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -316,7 +316,7 @@ static void perf_top__print_sym_table(struct perf_top *top)
hists__output_recalc_col_len(&top->sym_evsel->hists,
top->winsize.ws_row - 3);
putchar('\n');
- hists__fprintf(&top->sym_evsel->hists, NULL, false, false,
+ hists__fprintf(&top->sym_evsel->hists, NULL, false,
top->winsize.ws_row - 4 - printed, win_width, stdout);
}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index e3f8cd4..55b9ca8 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -244,13 +244,15 @@ static int hpp__width_displ(struct perf_hpp *hpp __maybe_unused)
}
static int hpp__entry_displ(struct perf_hpp *hpp,
- struct hist_entry *he __maybe_unused)
+ struct hist_entry *he)
{
+ struct hist_entry *pair = he->pair;
+ long displacement = pair ? pair->position - he->position : 0;
const char *fmt = symbol_conf.field_sep ? "%s" : "%6.6s";
char buf[32] = " ";
- if (hpp->displacement)
- scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
+ if (displacement)
+ scnprintf(buf, sizeof(buf), "%+4ld", displacement);
return scnprintf(hpp->buf, hpp->size, fmt, buf);
}
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 882461a..d7405f0 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -308,7 +308,7 @@ static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
static int hist_entry__fprintf(struct hist_entry *he, size_t size,
struct hists *hists, struct hists *pair_hists,
- long displacement, u64 total_period, FILE *fp)
+ u64 total_period, FILE *fp)
{
char bf[512];
int ret;
@@ -316,7 +316,6 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
.buf = bf,
.size = size,
.total_period = total_period,
- .displacement = displacement,
.ptr = pair_hists,
};
bool color = !symbol_conf.field_sep;
@@ -337,15 +336,13 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
}
size_t hists__fprintf(struct hists *hists, struct hists *pair,
- bool show_displacement, bool show_header, int max_rows,
+ bool show_header, int max_rows,
int max_cols, FILE *fp)
{
struct sort_entry *se;
struct rb_node *nd;
size_t ret = 0;
u64 total_period;
- unsigned long position = 1;
- long displacement = 0;
unsigned int width;
const char *sep = symbol_conf.field_sep;
const char *col_width = symbol_conf.col_width_list_str;
@@ -449,15 +446,7 @@ print_entries:
if (h->filtered)
continue;
- if (show_displacement) {
- if (h->pair != NULL)
- displacement = ((long)h->pair->position -
- (long)position);
- else
- displacement = 0;
- ++position;
- }
- ret += hist_entry__fprintf(h, max_cols, hists, pair, displacement,
+ ret += hist_entry__fprintf(h, max_cols, hists, pair,
total_period, fp);
if (max_rows && ++nr_rows >= max_rows)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index f011ad4..6d07f1a 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -99,8 +99,7 @@ void hists__inc_nr_events(struct hists *self, u32 type);
size_t hists__fprintf_nr_events(struct hists *self, FILE *fp);
size_t hists__fprintf(struct hists *self, struct hists *pair,
- bool show_displacement, bool show_header,
- int max_rows, int max_cols, FILE *fp);
+ bool show_header, int max_rows, int max_cols, FILE *fp);
int hist_entry__inc_addr_samples(struct hist_entry *self, int evidx, u64 addr);
int hist_entry__annotate(struct hist_entry *self, size_t privsize);
@@ -120,7 +119,6 @@ struct perf_hpp {
size_t size;
u64 total_period;
const char *sep;
- long displacement;
void *ptr;
};
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index eb3959b..f070b52 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -73,8 +73,8 @@ struct hist_entry {
u8 filtered;
char *srcline;
struct symbol *parent;
+ unsigned long position;
union {
- unsigned long position;
struct hist_entry *pair;
struct rb_root sorted_chain;
};
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 03/14] perf hists: Separate overhead and baseline columns
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
2012-09-27 11:09 ` [PATCH 01/14] perf hists: Add struct hists pointer to struct hist_entry Jiri Olsa
2012-09-27 11:09 ` [PATCH 02/14] perf diff: Refactor diff displacement possition info Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-28 5:56 ` Namhyung Kim
2012-09-27 11:09 ` [PATCH 04/14] perf tools: Removing hists pair argument from output path Jiri Olsa
` (11 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Currently the overhead and baseline columns are handled within
single function and the distinction is made by 'baseline hists'
pointer passed by 'struct perf_hpp::ptr'.
Since hists pointer is now part of each hist_entry, it's possible
to locate paired hists pointer directly from the passed struct
hist_entry pointer.
Also separating those 2 columns makes the code more obvious.
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>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/ui/hist.c | 74 ++++++++++++++++++++++++++++++----------------
tools/perf/ui/stdio/hist.c | 11 +++++--
tools/perf/util/hist.h | 1 +
3 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 55b9ca8..920001d 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -8,9 +8,7 @@
/* hist period print (hpp) functions */
static int hpp__header_overhead(struct perf_hpp *hpp)
{
- const char *fmt = hpp->ptr ? "Baseline" : "Overhead";
-
- return scnprintf(hpp->buf, hpp->size, fmt);
+ return scnprintf(hpp->buf, hpp->size, "Overhead");
}
static int hpp__width_overhead(struct perf_hpp *hpp __maybe_unused)
@@ -22,17 +20,6 @@ static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
{
double percent = 100.0 * he->period / hpp->total_period;
- if (hpp->ptr) {
- struct hists *old_hists = hpp->ptr;
- u64 total_period = old_hists->stats.total_period;
- u64 base_period = he->pair ? he->pair->period : 0;
-
- if (total_period)
- percent = 100.0 * base_period / total_period;
- else
- percent = 0.0;
- }
-
return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
}
@@ -41,17 +28,6 @@ static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
double percent = 100.0 * he->period / hpp->total_period;
const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
- if (hpp->ptr) {
- struct hists *old_hists = hpp->ptr;
- u64 total_period = old_hists->stats.total_period;
- u64 base_period = he->pair ? he->pair->period : 0;
-
- if (total_period)
- percent = 100.0 * base_period / total_period;
- else
- percent = 0.0;
- }
-
return scnprintf(hpp->buf, hpp->size, fmt, percent);
}
@@ -159,6 +135,47 @@ 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)
+{
+ return scnprintf(hpp->buf, hpp->size, "Baseline");
+}
+
+static int hpp__width_baseline(struct perf_hpp *hpp __maybe_unused)
+{
+ return 8;
+}
+
+static double baseline_percent(struct hist_entry *he)
+{
+ struct hist_entry *pair = he->pair;
+ 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->period;
+
+ percent = 100.0 * base_period / total_period;
+ }
+
+ return percent;
+}
+
+static int hpp__color_baseline(struct perf_hpp *hpp, struct hist_entry *he)
+{
+ double percent = baseline_percent(he);
+
+ return percent_color_snprintf(hpp->buf, hpp->size, " %5.2f%%", percent);
+}
+
+static int hpp__entry_baseline(struct perf_hpp *hpp, struct hist_entry *he)
+{
+ double percent = baseline_percent(he);
+ const char *fmt = symbol_conf.field_sep ? "%.2f" : " %5.2f%%";
+
+ return scnprintf(hpp->buf, hpp->size, fmt, percent);
+}
+
static int hpp__header_samples(struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%11s";
@@ -269,6 +286,7 @@ static int hpp__entry_displ(struct perf_hpp *hpp,
.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) },
@@ -302,6 +320,8 @@ void perf_hpp__init(bool need_pair, bool show_displacement)
perf_hpp__format[PERF_HPP__PERIOD].cond = true;
if (need_pair) {
+ perf_hpp__format[PERF_HPP__OVERHEAD].cond = false;
+ perf_hpp__format[PERF_HPP__BASELINE].cond = true;
perf_hpp__format[PERF_HPP__DELTA].cond = true;
if (show_displacement)
@@ -321,6 +341,7 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
const char *sep = symbol_conf.field_sep;
char *start = hpp->buf;
int i, ret;
+ bool first = true;
if (symbol_conf.exclude_other && !he->parent)
return 0;
@@ -329,9 +350,10 @@ int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
if (!perf_hpp__format[i].cond)
continue;
- if (!sep || i > 0) {
+ if (!sep || !first) {
ret = scnprintf(hpp->buf, hpp->size, "%s", sep ?: " ");
advance_hpp(hpp, ret);
+ first = false;
}
if (color && perf_hpp__format[i].color)
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index d7405f0..0aa6776 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -353,6 +353,7 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
.size = sizeof(bf),
.ptr = pair,
};
+ bool first = true;
init_rem_hits();
@@ -364,8 +365,10 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
if (!perf_hpp__format[idx].cond)
continue;
- if (idx)
+ if (!first)
fprintf(fp, "%s", sep ?: " ");
+ else
+ first = false;
perf_hpp__format[idx].header(&dummy_hpp);
fprintf(fp, "%s", bf);
@@ -400,6 +403,8 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
if (sep)
goto print_entries;
+ first = true;
+
fprintf(fp, "# ");
for (idx = 0; idx < PERF_HPP__MAX_INDEX; idx++) {
unsigned int i;
@@ -407,8 +412,10 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
if (!perf_hpp__format[idx].cond)
continue;
- if (idx)
+ if (!first)
fprintf(fp, "%s", sep ?: " ");
+ else
+ first = false;
width = perf_hpp__format[idx].width(&dummy_hpp);
for (i = 0; i < width; i++)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 6d07f1a..4fd8ddb 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -133,6 +133,7 @@ struct perf_hpp_fmt {
extern struct perf_hpp_fmt perf_hpp__format[];
enum {
+ PERF_HPP__BASELINE,
PERF_HPP__OVERHEAD,
PERF_HPP__OVERHEAD_SYS,
PERF_HPP__OVERHEAD_US,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 03/14] perf hists: Separate overhead and baseline columns
2012-09-27 11:09 ` [PATCH 03/14] perf hists: Separate overhead and baseline columns Jiri Olsa
@ 2012-09-28 5:56 ` Namhyung Kim
2012-10-02 13:32 ` Jiri Olsa
0 siblings, 1 reply; 28+ messages in thread
From: Namhyung Kim @ 2012-09-28 5: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 Thu, 27 Sep 2012 13:09:24 +0200, Jiri Olsa wrote:
> Currently the overhead and baseline columns are handled within
> single function and the distinction is made by 'baseline hists'
> pointer passed by 'struct perf_hpp::ptr'.
>
> Since hists pointer is now part of each hist_entry, it's possible
> to locate paired hists pointer directly from the passed struct
> hist_entry pointer.
>
> Also separating those 2 columns makes the code more obvious.
Yes, it was thinking about something like this.
>
> 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>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
[snip]
> +static int hpp__color_baseline(struct perf_hpp *hpp, struct hist_entry *he)
> +{
> + double percent = baseline_percent(he);
> +
> + return percent_color_snprintf(hpp->buf, hpp->size, " %5.2f%%", percent);
Is it possible to have a baseline value over 100%? I changed 'overhead'
colum format from '2 spaces + %5.2f + %' to '1 space + %6.2f + %' for
the case. Probably it'd better using it here too for consistency.
> +}
> +
> +static int hpp__entry_baseline(struct perf_hpp *hpp, struct hist_entry *he)
> +{
> + double percent = baseline_percent(he);
> + const char *fmt = symbol_conf.field_sep ? "%.2f" : " %5.2f%%";
Ditto.
Thanks,
Namhyung
> +
> + return scnprintf(hpp->buf, hpp->size, fmt, percent);
> +}
> +
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 03/14] perf hists: Separate overhead and baseline columns
2012-09-28 5:56 ` Namhyung Kim
@ 2012-10-02 13:32 ` Jiri Olsa
0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-10-02 13:32 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, Sep 28, 2012 at 02:56:01PM +0900, Namhyung Kim wrote:
> On Thu, 27 Sep 2012 13:09:24 +0200, Jiri Olsa wrote:
> > Currently the overhead and baseline columns are handled within
> > single function and the distinction is made by 'baseline hists'
> > pointer passed by 'struct perf_hpp::ptr'.
> >
> > Since hists pointer is now part of each hist_entry, it's possible
> > to locate paired hists pointer directly from the passed struct
> > hist_entry pointer.
> >
> > Also separating those 2 columns makes the code more obvious.
>
> Yes, it was thinking about something like this.
>
>
> >
> > 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>
> > Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> > ---
> [snip]
> > +static int hpp__color_baseline(struct perf_hpp *hpp, struct hist_entry *he)
> > +{
> > + double percent = baseline_percent(he);
> > +
> > + return percent_color_snprintf(hpp->buf, hpp->size, " %5.2f%%", percent);
>
> Is it possible to have a baseline value over 100%? I changed 'overhead'
> colum format from '2 spaces + %5.2f + %' to '1 space + %6.2f + %' for
> the case. Probably it'd better using it here too for consistency.
I think you're right, I'll make the change
thanks,
jirka
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 04/14] perf tools: Removing hists pair argument from output path
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (2 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 03/14] perf hists: Separate overhead and baseline columns Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 05/14] perf diff: Add -b option for perf diff to display paired entries only Jiri Olsa
` (10 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
The hists pointer is now part of the 'struct hist_entry'.
And since the overhead and baseline columns are split now,
there's no reason to pass it through the output path.
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>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/builtin-diff.c | 3 +--
tools/perf/builtin-report.c | 2 +-
tools/perf/builtin-top.c | 2 +-
tools/perf/ui/hist.c | 9 +++++----
tools/perf/ui/stdio/hist.c | 10 +++-------
tools/perf/util/hist.h | 4 ++--
6 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 5cb577a..413c65a1b 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -214,8 +214,7 @@ static int __cmd_diff(void)
first = false;
hists__match(&evsel_old->hists, &evsel->hists);
- hists__fprintf(&evsel->hists, &evsel_old->hists,
- true, 0, 0, stdout);
+ hists__fprintf(&evsel->hists, true, 0, 0, stdout);
}
out_delete:
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 6748cac..95e7ea8 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -320,7 +320,7 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
const char *evname = perf_evsel__name(pos);
hists__fprintf_nr_sample_events(hists, evname, stdout);
- hists__fprintf(hists, NULL, true, 0, 0, stdout);
+ hists__fprintf(hists, true, 0, 0, stdout);
fprintf(stdout, "\n\n");
}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 6b8c629..dd7ff2b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -316,7 +316,7 @@ static void perf_top__print_sym_table(struct perf_top *top)
hists__output_recalc_col_len(&top->sym_evsel->hists,
top->winsize.ws_row - 3);
putchar('\n');
- hists__fprintf(&top->sym_evsel->hists, NULL, false,
+ hists__fprintf(&top->sym_evsel->hists, false,
top->winsize.ws_row - 4 - printed, win_width, stdout);
}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 920001d..2dbec2d 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -228,16 +228,17 @@ 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 hists *pair_hists = hpp->ptr;
+ struct hist_entry *pair = he->pair;
+ struct hists *pair_hists = pair ? pair->hists : NULL;
u64 old_total, new_total;
double old_percent = 0, new_percent = 0;
double diff;
const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
char buf[32] = " ";
- old_total = pair_hists->stats.total_period;
- if (old_total > 0 && he->pair)
- old_percent = 100.0 * he->pair->period / old_total;
+ old_total = pair_hists ? pair_hists->stats.total_period : 0;
+ if (old_total > 0 && pair)
+ old_percent = 100.0 * pair->period / old_total;
new_total = hpp->total_period;
if (new_total > 0)
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 0aa6776..1340c93 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -307,8 +307,7 @@ static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
}
static int hist_entry__fprintf(struct hist_entry *he, size_t size,
- struct hists *hists, struct hists *pair_hists,
- u64 total_period, FILE *fp)
+ struct hists *hists, u64 total_period, FILE *fp)
{
char bf[512];
int ret;
@@ -316,7 +315,6 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
.buf = bf,
.size = size,
.total_period = total_period,
- .ptr = pair_hists,
};
bool color = !symbol_conf.field_sep;
@@ -335,8 +333,7 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
return ret;
}
-size_t hists__fprintf(struct hists *hists, struct hists *pair,
- bool show_header, int max_rows,
+size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
int max_cols, FILE *fp)
{
struct sort_entry *se;
@@ -351,7 +348,6 @@ size_t hists__fprintf(struct hists *hists, struct hists *pair,
struct perf_hpp dummy_hpp = {
.buf = bf,
.size = sizeof(bf),
- .ptr = pair,
};
bool first = true;
@@ -453,7 +449,7 @@ print_entries:
if (h->filtered)
continue;
- ret += hist_entry__fprintf(h, max_cols, hists, pair,
+ ret += hist_entry__fprintf(h, max_cols, hists,
total_period, fp);
if (max_rows && ++nr_rows >= max_rows)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 4fd8ddb..6cd7535 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -98,8 +98,8 @@ void hists__output_recalc_col_len(struct hists *hists, int max_rows);
void hists__inc_nr_events(struct hists *self, u32 type);
size_t hists__fprintf_nr_events(struct hists *self, FILE *fp);
-size_t hists__fprintf(struct hists *self, struct hists *pair,
- bool show_header, int max_rows, int max_cols, FILE *fp);
+size_t hists__fprintf(struct hists *self, bool show_header, int max_rows,
+ int max_cols, FILE *fp);
int hist_entry__inc_addr_samples(struct hist_entry *self, int evidx, u64 addr);
int hist_entry__annotate(struct hist_entry *self, size_t privsize);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 05/14] perf diff: Add -b option for perf diff to display paired entries only
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (3 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 04/14] perf tools: Removing hists pair argument from output path Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 06/14] perf tool: Add hpp interface to enable/disable hpp column Jiri Olsa
` (9 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Adding -b option to perf diff command to display only entries
with match in the baseline.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 4 ++++
tools/perf/builtin-diff.c | 31 +++++++++++++++++++++++++++++--
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index ab7f667..6ce9585 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -72,6 +72,10 @@ OPTIONS
--symfs=<directory>::
Look for files with symbols relative to this directory.
+-b::
+--baseline-only::
+ Show only items with match in baseline.
+
SEE ALSO
--------
linkperf:perf-record[1]
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 413c65a1b..1c36673 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -24,6 +24,7 @@ static char const *input_old = "perf.data.old",
static char diff__default_sort_order[] = "dso,symbol";
static bool force;
static bool show_displacement;
+static bool show_baseline_only;
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
@@ -172,6 +173,31 @@ static void perf_evlist__resort_hists(struct perf_evlist *evlist, bool name)
}
}
+static void hists__baseline_only(struct hists *hists)
+{
+ struct rb_node *next = rb_first(&hists->entries);
+
+ while (next != NULL) {
+ struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
+
+ next = rb_next(&he->rb_node);
+ if (!he->pair) {
+ rb_erase(&he->rb_node, &hists->entries);
+ hist_entry__free(he);
+ }
+ }
+}
+
+static void hists__process(struct hists *old, struct hists *new)
+{
+ hists__match(old, new);
+
+ if (show_baseline_only)
+ hists__baseline_only(new);
+
+ hists__fprintf(new, true, 0, 0, stdout);
+}
+
static int __cmd_diff(void)
{
int ret, i;
@@ -213,8 +239,7 @@ static int __cmd_diff(void)
first = false;
- hists__match(&evsel_old->hists, &evsel->hists);
- hists__fprintf(&evsel->hists, true, 0, 0, stdout);
+ hists__process(&evsel_old->hists, &evsel->hists);
}
out_delete:
@@ -235,6 +260,8 @@ static const struct option options[] = {
"be more verbose (show symbol address, etc)"),
OPT_BOOLEAN('M', "displacement", &show_displacement,
"Show position displacement relative to baseline"),
+ OPT_BOOLEAN('b', "baseline-only", &show_baseline_only,
+ "Show only items with match in baseline"),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
"dump raw trace in ASCII"),
OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 06/14] perf tool: Add hpp interface to enable/disable hpp column
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (4 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 05/14] perf diff: Add -b option for perf diff to display paired entries only Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-28 6:02 ` Namhyung Kim
2012-09-27 11:09 ` [PATCH 07/14] perf diff: Add ratio computation way to compare hist entries Jiri Olsa
` (8 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Adding perf_hpp__column_enable function to enable/disable hists
column and removing diff command specific stuff 'need_pair and
show_displacement' from hpp code.
The diff command now enables/disables columns separately according
to the user arguments. This will be helpful in future patches where
more columns are added into diff output.
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>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/builtin-diff.c | 18 +++++++++++++++++-
tools/perf/builtin-report.c | 2 +-
tools/perf/ui/browsers/hists.c | 2 +-
tools/perf/ui/gtk/browser.c | 2 +-
tools/perf/ui/hist.c | 15 ++++++---------
tools/perf/ui/setup.c | 2 +-
tools/perf/util/hist.h | 3 ++-
7 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 1c36673..1063c31 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -283,6 +283,21 @@ static const struct option options[] = {
OPT_END()
};
+static void ui_init(void)
+{
+ perf_hpp__init();
+
+ /* No overhead column. */
+ perf_hpp__column_enable(PERF_HPP__OVERHEAD, false);
+
+ /* Display baseline/delta/displacement columns. */
+ perf_hpp__column_enable(PERF_HPP__BASELINE, true);
+ perf_hpp__column_enable(PERF_HPP__DELTA, true);
+
+ if (show_displacement)
+ perf_hpp__column_enable(PERF_HPP__DISPL, true);
+}
+
int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
{
sort_order = diff__default_sort_order;
@@ -305,7 +320,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
if (symbol__init() < 0)
return -1;
- perf_hpp__init(true, show_displacement);
+ ui_init();
+
setup_sorting(diff_usage, options);
setup_pager();
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 95e7ea8..a61725d 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -691,7 +691,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
setup_browser(true);
else {
use_browser = 0;
- perf_hpp__init(false, false);
+ perf_hpp__init();
}
setup_sorting(report_usage, options);
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index a21f40b..bbd11c2 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -584,7 +584,7 @@ HPP__COLOR_FN(overhead_guest_us, period_guest_us)
void hist_browser__init_hpp(void)
{
- perf_hpp__init(false, false);
+ perf_hpp__init();
perf_hpp__format[PERF_HPP__OVERHEAD].color =
hist_browser__hpp_color_overhead;
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 7ff99ec..2bc08f6 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -73,7 +73,7 @@ HPP__COLOR_FN(overhead_guest_us, period_guest_us)
void perf_gtk__init_hpp(void)
{
- perf_hpp__init(false, false);
+ perf_hpp__init();
perf_hpp__format[PERF_HPP__OVERHEAD].color =
perf_gtk__hpp_color_overhead;
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 2dbec2d..81bb03e 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -302,7 +302,7 @@ struct perf_hpp_fmt perf_hpp__format[] = {
#undef HPP__COLOR_PRINT_FNS
#undef HPP__PRINT_FNS
-void perf_hpp__init(bool need_pair, bool show_displacement)
+void perf_hpp__init(void)
{
if (symbol_conf.show_cpu_utilization) {
perf_hpp__format[PERF_HPP__OVERHEAD_SYS].cond = true;
@@ -319,15 +319,12 @@ void perf_hpp__init(bool need_pair, bool show_displacement)
if (symbol_conf.show_total_period)
perf_hpp__format[PERF_HPP__PERIOD].cond = true;
+}
- if (need_pair) {
- perf_hpp__format[PERF_HPP__OVERHEAD].cond = false;
- perf_hpp__format[PERF_HPP__BASELINE].cond = true;
- perf_hpp__format[PERF_HPP__DELTA].cond = true;
-
- if (show_displacement)
- perf_hpp__format[PERF_HPP__DISPL].cond = true;
- }
+void perf_hpp__column_enable(unsigned col, bool enable)
+{
+ BUG_ON(col >= PERF_HPP__MAX_INDEX);
+ perf_hpp__format[col].cond = enable;
}
static inline void advance_hpp(struct perf_hpp *hpp, int inc)
diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index bd7d460..ebb4cc1 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -30,7 +30,7 @@ void setup_browser(bool fallback_to_pager)
if (fallback_to_pager)
setup_pager();
- perf_hpp__init(false, false);
+ perf_hpp__init();
break;
}
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 6cd7535..c99317f 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -147,7 +147,8 @@ enum {
PERF_HPP__MAX_INDEX
};
-void perf_hpp__init(bool need_pair, bool show_displacement);
+void perf_hpp__init(void);
+void perf_hpp__column_enable(unsigned col, bool enable);
int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
bool color);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 06/14] perf tool: Add hpp interface to enable/disable hpp column
2012-09-27 11:09 ` [PATCH 06/14] perf tool: Add hpp interface to enable/disable hpp column Jiri Olsa
@ 2012-09-28 6:02 ` Namhyung Kim
0 siblings, 0 replies; 28+ messages in thread
From: Namhyung Kim @ 2012-09-28 6:02 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Thu, 27 Sep 2012 13:09:27 +0200, Jiri Olsa wrote:
> Adding perf_hpp__column_enable function to enable/disable hists
> column and removing diff command specific stuff 'need_pair and
> show_displacement' from hpp code.
>
> The diff command now enables/disables columns separately according
> to the user arguments. This will be helpful in future patches where
> more columns are added into diff output.
This is what I wanted to do. :)
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 07/14] perf diff: Add ratio computation way to compare hist entries
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (5 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 06/14] perf tool: Add hpp interface to enable/disable hpp column Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 08/14] perf diff: Removing the total_period argument from output code Jiri Olsa
` (7 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Adding -c option to select computation method with the current
'Delta' computation as default. Current possible values are of
this option are: 'delta' and 'ratio'.
Adding 'ratio' as new computation way to compare hist entries.
If specified the 'Ratio' column is displayed with value 'r'
computed as:
r = A->period / B->period
with:
- A/B being matching hist entry from first/second file specified
(or perf.data/perf.data.old) respectively.
- period being the hist entry period value
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 33 ++++++++++++++++++++++
tools/perf/builtin-diff.c | 51 ++++++++++++++++++++++++++++++++--
tools/perf/ui/hist.c | 28 +++++++++++++++++++
tools/perf/util/hist.h | 1 +
4 files changed, 111 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 6ce9585..8fff061 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -76,6 +76,39 @@ OPTIONS
--baseline-only::
Show only items with match in baseline.
+-c::
+--compute::
+ Differential computation selection - delta,ratio (default is delta).
+ See COMPARISON METHODS section for more info.
+
+COMPARISON METHODS
+------------------
+delta
+~~~~~
+If specified the 'Delta' column is displayed with value 'd' computed as:
+
+ d = A->period_percent - B->period_percent
+
+with:
+ - A/B being matching hist entry from first/second file specified
+ (or perf.data/perf.data.old) respectively.
+
+ - period_percent being the % of the hist entry period value within
+ single data file
+
+ratio
+~~~~~
+If specified the 'Ratio' column is displayed with value 'r' computed as:
+
+ r = A->period / B->period
+
+with:
+ - A/B being matching hist entry from first/second file specified
+ (or perf.data/perf.data.old) respectively.
+
+ - period being the hist entry period value
+
+
SEE ALSO
--------
linkperf:perf-record[1]
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 1063c31..b5c15f3 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -26,6 +26,40 @@ static bool force;
static bool show_displacement;
static bool show_baseline_only;
+enum {
+ COMPUTE_DELTA,
+ COMPUTE_RATIO,
+ COMPUTE_MAX,
+};
+
+const char *compute_names[COMPUTE_MAX] = {
+ [COMPUTE_DELTA] = "delta",
+ [COMPUTE_RATIO] = "ratio",
+};
+
+static int compute;
+
+static int setup_compute(const struct option *opt, const char *str,
+ int unset __maybe_unused)
+{
+ int *cp = (int *) opt->value;
+ unsigned i;
+
+ if (!str) {
+ *cp = COMPUTE_DELTA;
+ return 0;
+ }
+
+ for (i = 0; i < COMPUTE_MAX; i++)
+ if (!strcmp(str, compute_names[i])) {
+ *cp = i;
+ return 0;
+ }
+
+ pr_err("Failed to find valid compute string\n");
+ return -EINVAL;
+}
+
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
{
@@ -262,6 +296,9 @@ static const struct option options[] = {
"Show position displacement relative to baseline"),
OPT_BOOLEAN('b', "baseline-only", &show_baseline_only,
"Show only items with match in baseline"),
+ OPT_CALLBACK('c', "compute", &compute, "delta,ratio (default delta)",
+ "Entries differential computation selection",
+ setup_compute),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
"dump raw trace in ASCII"),
OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
@@ -290,9 +327,19 @@ static void ui_init(void)
/* No overhead column. */
perf_hpp__column_enable(PERF_HPP__OVERHEAD, false);
- /* Display baseline/delta/displacement columns. */
+ /* Display baseline/delta/ratio/displacement columns. */
perf_hpp__column_enable(PERF_HPP__BASELINE, true);
- perf_hpp__column_enable(PERF_HPP__DELTA, true);
+
+ switch (compute) {
+ case COMPUTE_DELTA:
+ perf_hpp__column_enable(PERF_HPP__DELTA, true);
+ break;
+ case COMPUTE_RATIO:
+ perf_hpp__column_enable(PERF_HPP__RATIO, true);
+ break;
+ default:
+ BUG_ON(1);
+ };
if (show_displacement)
perf_hpp__column_enable(PERF_HPP__DISPL, true);
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 81bb03e..21aabc6 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -251,6 +251,33 @@ 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)
+{
+ 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)
+{
+ return 14;
+}
+
+static int hpp__entry_ratio(struct perf_hpp *hpp, struct hist_entry *he)
+{
+ struct hist_entry *pair = he->pair;
+ double new_period = he->period;
+ double old_period = pair ? pair->period : 0;
+ double ratio = pair ? new_period / old_period : 0;
+ const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
+ char buf[32] = " ";
+
+ if (ratio > 0.0)
+ scnprintf(buf, sizeof(buf), "%+14.6F", ratio);
+
+ return scnprintf(hpp->buf, hpp->size, fmt, buf);
+}
+
static int hpp__header_displ(struct perf_hpp *hpp)
{
return scnprintf(hpp->buf, hpp->size, "Displ.");
@@ -296,6 +323,7 @@ struct perf_hpp_fmt perf_hpp__format[] = {
{ .cond = false, HPP__PRINT_FNS(samples) },
{ .cond = false, HPP__PRINT_FNS(period) },
{ .cond = false, HPP__PRINT_FNS(delta) },
+ { .cond = false, HPP__PRINT_FNS(ratio) },
{ .cond = false, HPP__PRINT_FNS(displ) }
};
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index c99317f..2826531 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -142,6 +142,7 @@ enum {
PERF_HPP__SAMPLES,
PERF_HPP__PERIOD,
PERF_HPP__DELTA,
+ PERF_HPP__RATIO,
PERF_HPP__DISPL,
PERF_HPP__MAX_INDEX
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 08/14] perf diff: Removing the total_period argument from output code
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (6 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 07/14] perf diff: Add ratio computation way to compare hist entries Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 09/14] perf diff: Add option to sort entries based on diff computation Jiri Olsa
` (6 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
The total_period is available in struct hists data via the 'struct
hist_entry::hists' pointer. There's no need to carry it through the
output code path.
Removing 'struct perf_hpp::total_period' pointer, because it's no
longer needed.
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>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/ui/browsers/hists.c | 4 ++--
tools/perf/ui/gtk/browser.c | 4 ++--
tools/perf/ui/hist.c | 37 ++++++++++++++++++++++++++-----------
tools/perf/ui/stdio/hist.c | 15 +++++----------
tools/perf/util/hist.h | 1 -
5 files changed, 35 insertions(+), 26 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index bbd11c2..d359795 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -569,7 +569,8 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
static int hist_browser__hpp_color_ ## _name(struct perf_hpp *hpp, \
struct hist_entry *he) \
{ \
- double percent = 100.0 * he->_field / hpp->total_period; \
+ struct hists *hists = he->hists; \
+ double percent = 100.0 * he->_field / hists->stats.total_period;\
*(double *)hpp->ptr = percent; \
return scnprintf(hpp->buf, hpp->size, "%6.2f%%", percent); \
}
@@ -624,7 +625,6 @@ static int hist_browser__show_entry(struct hist_browser *browser,
struct perf_hpp hpp = {
.buf = s,
.size = sizeof(s),
- .total_period = browser->hists->stats.total_period,
};
ui_browser__gotorc(&browser->b, row, 0);
diff --git a/tools/perf/ui/gtk/browser.c b/tools/perf/ui/gtk/browser.c
index 2bc08f6..3cbb1d6 100644
--- a/tools/perf/ui/gtk/browser.c
+++ b/tools/perf/ui/gtk/browser.c
@@ -49,7 +49,8 @@ static const char *perf_gtk__get_percent_color(double percent)
static int perf_gtk__hpp_color_ ## _name(struct perf_hpp *hpp, \
struct hist_entry *he) \
{ \
- double percent = 100.0 * he->_field / hpp->total_period; \
+ struct hists *hists = he->hists; \
+ double percent = 100.0 * he->_field / hists->stats.total_period; \
const char *markup; \
int ret = 0; \
\
@@ -102,7 +103,6 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
struct perf_hpp hpp = {
.buf = s,
.size = sizeof(s),
- .total_period = hists->stats.total_period,
};
nr_cols = 0;
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 21aabc6..81f3b4f 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -18,14 +18,16 @@ static int hpp__width_overhead(struct perf_hpp *hpp __maybe_unused)
static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
{
- double percent = 100.0 * he->period / hpp->total_period;
+ struct hists *hists = he->hists;
+ double percent = 100.0 * he->period / hists->stats.total_period;
return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
}
static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
{
- double percent = 100.0 * he->period / hpp->total_period;
+ struct hists *hists = he->hists;
+ double percent = 100.0 * he->period / hists->stats.total_period;
const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";
return scnprintf(hpp->buf, hpp->size, fmt, percent);
@@ -45,13 +47,16 @@ static int hpp__width_overhead_sys(struct perf_hpp *hpp __maybe_unused)
static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
{
- double percent = 100.0 * he->period_sys / hpp->total_period;
+ struct hists *hists = he->hists;
+ double percent = 100.0 * he->period_sys / hists->stats.total_period;
+
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)
{
- double percent = 100.0 * he->period_sys / hpp->total_period;
+ struct hists *hists = he->hists;
+ double percent = 100.0 * he->period_sys / hists->stats.total_period;
const char *fmt = symbol_conf.field_sep ? "%.2f" : "%6.2f%%";
return scnprintf(hpp->buf, hpp->size, fmt, percent);
@@ -71,13 +76,16 @@ static int hpp__width_overhead_us(struct perf_hpp *hpp __maybe_unused)
static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
{
- double percent = 100.0 * he->period_us / hpp->total_period;
+ struct hists *hists = he->hists;
+ double percent = 100.0 * he->period_us / hists->stats.total_period;
+
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)
{
- double percent = 100.0 * he->period_us / hpp->total_period;
+ struct hists *hists = he->hists;
+ double percent = 100.0 * he->period_us / hists->stats.total_period;
const char *fmt = symbol_conf.field_sep ? "%.2f" : "%6.2f%%";
return scnprintf(hpp->buf, hpp->size, fmt, percent);
@@ -96,14 +104,17 @@ static int hpp__width_overhead_guest_sys(struct perf_hpp *hpp __maybe_unused)
static int hpp__color_overhead_guest_sys(struct perf_hpp *hpp,
struct hist_entry *he)
{
- double percent = 100.0 * he->period_guest_sys / hpp->total_period;
+ struct hists *hists = he->hists;
+ double percent = 100.0 * he->period_guest_sys / hists->stats.total_period;
+
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)
{
- double percent = 100.0 * he->period_guest_sys / hpp->total_period;
+ struct hists *hists = he->hists;
+ double percent = 100.0 * he->period_guest_sys / hists->stats.total_period;
const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%% ";
return scnprintf(hpp->buf, hpp->size, fmt, percent);
@@ -122,14 +133,17 @@ static int hpp__width_overhead_guest_us(struct perf_hpp *hpp __maybe_unused)
static int hpp__color_overhead_guest_us(struct perf_hpp *hpp,
struct hist_entry *he)
{
- double percent = 100.0 * he->period_guest_us / hpp->total_period;
+ struct hists *hists = he->hists;
+ double percent = 100.0 * he->period_guest_us / hists->stats.total_period;
+
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)
{
- double percent = 100.0 * he->period_guest_us / hpp->total_period;
+ struct hists *hists = he->hists;
+ double percent = 100.0 * he->period_guest_us / hists->stats.total_period;
const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%% ";
return scnprintf(hpp->buf, hpp->size, fmt, percent);
@@ -230,6 +244,7 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
{
struct hist_entry *pair = he->pair;
struct hists *pair_hists = pair ? pair->hists : NULL;
+ struct hists *hists = he->hists;
u64 old_total, new_total;
double old_percent = 0, new_percent = 0;
double diff;
@@ -240,7 +255,7 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
if (old_total > 0 && pair)
old_percent = 100.0 * pair->period / old_total;
- new_total = hpp->total_period;
+ new_total = hists->stats.total_period;
if (new_total > 0)
new_percent = 100.0 * he->period / new_total;
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 1340c93..850c6d2 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -292,9 +292,10 @@ static size_t hist_entry_callchain__fprintf(struct hist_entry *he,
static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
struct hists *hists,
- u64 total_period, FILE *fp)
+ FILE *fp)
{
int left_margin = 0;
+ u64 total_period = hists->stats.total_period;
if (sort__first_dimension == SORT_COMM) {
struct sort_entry *se = list_first_entry(&hist_entry__sort_list,
@@ -307,14 +308,13 @@ static size_t hist_entry__callchain_fprintf(struct hist_entry *he,
}
static int hist_entry__fprintf(struct hist_entry *he, size_t size,
- struct hists *hists, u64 total_period, FILE *fp)
+ struct hists *hists, FILE *fp)
{
char bf[512];
int ret;
struct perf_hpp hpp = {
.buf = bf,
.size = size,
- .total_period = total_period,
};
bool color = !symbol_conf.field_sep;
@@ -327,8 +327,7 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
ret = fprintf(fp, "%s\n", bf);
if (symbol_conf.use_callchain)
- ret += hist_entry__callchain_fprintf(he, hists,
- total_period, fp);
+ ret += hist_entry__callchain_fprintf(he, hists, fp);
return ret;
}
@@ -339,7 +338,6 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
struct sort_entry *se;
struct rb_node *nd;
size_t ret = 0;
- u64 total_period;
unsigned int width;
const char *sep = symbol_conf.field_sep;
const char *col_width = symbol_conf.col_width_list_str;
@@ -441,16 +439,13 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
goto out;
print_entries:
- total_period = hists->stats.total_period;
-
for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
if (h->filtered)
continue;
- ret += hist_entry__fprintf(h, max_cols, hists,
- total_period, fp);
+ ret += hist_entry__fprintf(h, max_cols, hists, fp);
if (max_rows && ++nr_rows >= max_rows)
goto out;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2826531..b239504 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -117,7 +117,6 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *he);
struct perf_hpp {
char *buf;
size_t size;
- u64 total_period;
const char *sep;
void *ptr;
};
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 09/14] perf diff: Add option to sort entries based on diff computation
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (7 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 08/14] perf diff: Removing the total_period argument from output code Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 10/14] perf diff: Add weighted diff computation way to compare hist entries Jiri Olsa
` (5 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Adding support to sort hist entries based on the outcome of selected
computation. It's now possible to specify '+' as a first character
of '-c' option value to make such sort.
Example:
$ ./perf diff -cratio -b
# Event 'cache-misses'
#
# Baseline Ratio Shared Object Symbol
# ........ .............. ................. ................................
#
19.64% 0.69 [kernel.kallsyms] [k] clear_page
0.30% 0.17 [kernel.kallsyms] [k] mm_alloc
0.04% 0.20 [kernel.kallsyms] [k] kmem_cache_alloc
$ ./perf diff -c+ratio -b
# Event 'cache-misses'
#
# Baseline Ratio Shared Object Symbol
# ........ .............. ................. ................................
#
19.64% 0.69 [kernel.kallsyms] [k] clear_page
0.04% 0.20 [kernel.kallsyms] [k] kmem_cache_alloc
0.30% 0.17 [kernel.kallsyms] [k] mm_alloc
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 2 +
tools/perf/builtin-diff.c | 137 +++++++++++++++++++++++++++++++++
tools/perf/ui/hist.c | 29 +++----
tools/perf/util/hist.h | 2 +
tools/perf/util/sort.h | 15 ++++
5 files changed, 167 insertions(+), 18 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 8fff061..cff3d9b 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -79,6 +79,8 @@ OPTIONS
-c::
--compute::
Differential computation selection - delta,ratio (default is delta).
+ If '+' is specified as a first character, the output is sorted based
+ on the computation results.
See COMPARISON METHODS section for more info.
COMPARISON METHODS
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index b5c15f3..14bc244 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -25,6 +25,7 @@ static char diff__default_sort_order[] = "dso,symbol";
static bool force;
static bool show_displacement;
static bool show_baseline_only;
+static bool sort_compute;
enum {
COMPUTE_DELTA,
@@ -50,6 +51,13 @@ static int setup_compute(const struct option *opt, const char *str,
return 0;
}
+ if (*str == '+') {
+ sort_compute = true;
+ str++;
+ if (!*str)
+ return 0;
+ }
+
for (i = 0; i < COMPUTE_MAX; i++)
if (!strcmp(str, compute_names[i])) {
*cp = i;
@@ -60,6 +68,34 @@ static int setup_compute(const struct option *opt, const char *str,
return -EINVAL;
}
+static double get_period_percent(struct hist_entry *he, u64 period)
+{
+ u64 total = he->hists->stats.total_period;
+ return (period * 100.0) / total;
+}
+
+double perf_diff__compute_delta(struct hist_entry *he)
+{
+ struct hist_entry *pair = he->pair;
+ double new_percent = get_period_percent(he, he->period);
+ double old_percent = pair ? get_period_percent(pair, pair->period) : 0.0;
+
+ he->diff.period_ratio_delta = new_percent - old_percent;
+ he->diff.computed = true;
+ return he->diff.period_ratio_delta;
+}
+
+double perf_diff__compute_ratio(struct hist_entry *he)
+{
+ struct hist_entry *pair = he->pair;
+ double new_period = he->period;
+ double old_period = pair ? pair->period : 0;
+
+ he->diff.computed = true;
+ he->diff.period_ratio = pair ? (new_period / old_period) : 0;
+ return he->diff.period_ratio;
+}
+
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
{
@@ -222,6 +258,102 @@ static void hists__baseline_only(struct hists *hists)
}
}
+static void hists__precompute(struct hists *hists)
+{
+ struct rb_node *next = rb_first(&hists->entries);
+
+ while (next != NULL) {
+ struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
+
+ next = rb_next(&he->rb_node);
+
+ switch (compute) {
+ case COMPUTE_DELTA:
+ perf_diff__compute_delta(he);
+ break;
+ case COMPUTE_RATIO:
+ perf_diff__compute_ratio(he);
+ break;
+ default:
+ BUG_ON(1);
+ }
+ }
+}
+
+static int64_t cmp_doubles(double l, double r)
+{
+ if (l > r)
+ return -1;
+ else if (l < r)
+ return 1;
+ else
+ return 0;
+}
+
+static int64_t
+hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
+ int c)
+{
+ switch (c) {
+ case COMPUTE_DELTA:
+ {
+ double l = left->diff.period_ratio_delta;
+ double r = right->diff.period_ratio_delta;
+
+ return cmp_doubles(l, r);
+ }
+ case COMPUTE_RATIO:
+ {
+ double l = left->diff.period_ratio;
+ double r = right->diff.period_ratio;
+
+ return cmp_doubles(l, r);
+ }
+ default:
+ BUG_ON(1);
+ }
+
+ return 0;
+}
+
+static void insert_hist_entry_by_compute(struct rb_root *root,
+ struct hist_entry *he,
+ int c)
+{
+ struct rb_node **p = &root->rb_node;
+ struct rb_node *parent = NULL;
+ struct hist_entry *iter;
+
+ while (*p != NULL) {
+ parent = *p;
+ iter = rb_entry(parent, struct hist_entry, rb_node);
+ if (hist_entry__cmp_compute(he, iter, c) < 0)
+ p = &(*p)->rb_left;
+ else
+ p = &(*p)->rb_right;
+ }
+
+ rb_link_node(&he->rb_node, parent, p);
+ rb_insert_color(&he->rb_node, root);
+}
+
+static void hists__compute_resort(struct hists *hists)
+{
+ struct rb_root tmp = RB_ROOT;
+ struct rb_node *next = rb_first(&hists->entries);
+
+ while (next != NULL) {
+ struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);
+
+ next = rb_next(&he->rb_node);
+
+ rb_erase(&he->rb_node, &hists->entries);
+ insert_hist_entry_by_compute(&tmp, he, compute);
+ }
+
+ hists->entries = tmp;
+}
+
static void hists__process(struct hists *old, struct hists *new)
{
hists__match(old, new);
@@ -229,6 +361,11 @@ static void hists__process(struct hists *old, struct hists *new)
if (show_baseline_only)
hists__baseline_only(new);
+ if (sort_compute) {
+ hists__precompute(new);
+ hists__compute_resort(new);
+ }
+
hists__fprintf(new, true, 0, 0, stdout);
}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 81f3b4f..21b57a2 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -242,24 +242,15 @@ 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 = he->pair;
- struct hists *pair_hists = pair ? pair->hists : NULL;
- struct hists *hists = he->hists;
- u64 old_total, new_total;
- double old_percent = 0, new_percent = 0;
- double diff;
const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
char buf[32] = " ";
+ double diff;
- old_total = pair_hists ? pair_hists->stats.total_period : 0;
- if (old_total > 0 && pair)
- old_percent = 100.0 * pair->period / old_total;
-
- new_total = hists->stats.total_period;
- if (new_total > 0)
- new_percent = 100.0 * he->period / new_total;
+ if (he->diff.computed)
+ diff = he->diff.period_ratio_delta;
+ else
+ diff = perf_diff__compute_delta(he);
- diff = new_percent - old_percent;
if (fabs(diff) >= 0.01)
scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
@@ -280,12 +271,14 @@ 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 = he->pair;
- double new_period = he->period;
- double old_period = pair ? pair->period : 0;
- double ratio = pair ? new_period / old_period : 0;
const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
char buf[32] = " ";
+ double ratio;
+
+ if (he->diff.computed)
+ ratio = he->diff.period_ratio;
+ else
+ ratio = perf_diff__compute_ratio(he);
if (ratio > 0.0)
scnprintf(buf, sizeof(buf), "%+14.6F", ratio);
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index b239504..2dbb340 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -206,4 +206,6 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist, const char *help,
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);
#endif /* __PERF_HIST_H */
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index f070b52..2090670 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -43,6 +43,19 @@ extern struct sort_entry sort_sym_from;
extern struct sort_entry sort_sym_to;
extern enum sort_type sort__first_dimension;
+struct hist_entry_diff {
+ bool computed;
+
+ /* PERF_HPP__DISPL */
+ int displacement;
+
+ /* PERF_HPP__DELTA */
+ double period_ratio_delta;
+
+ /* PERF_HPP__RATIO */
+ double period_ratio;
+};
+
/**
* struct hist_entry - histogram entry
*
@@ -63,6 +76,8 @@ struct hist_entry {
s32 cpu;
u32 nr_events;
+ struct hist_entry_diff diff;
+
/* XXX These two should move to some tree widget lib */
u16 row_offset;
u16 nr_rows;
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 10/14] perf diff: Add weighted diff computation way to compare hist entries
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (8 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 09/14] perf diff: Add option to sort entries based on diff computation Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 11/14] perf diff: Add -p option to display period values for " Jiri Olsa
` (4 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Adding 'wdiff' as new computation way to compare hist entries.
If specified the 'Weighted diff' column is displayed with value 'd'
computed as:
d = B->period * WEIGHT-A - A->period * WEIGHT-B
- A/B being matching hist entry from first/second file specified
(or perf.data/perf.data.old) respectively.
- period being the hist entry period value
- WEIGHT-A/WEIGHT-B being user suplied weights in the the '-c' option
behind ':' separator like '-c wdiff:1,2'.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 15 ++++-
tools/perf/builtin-diff.c | 114 +++++++++++++++++++++++++++++++--
tools/perf/ui/hist.c | 30 +++++++++
tools/perf/util/hist.h | 2 +
tools/perf/util/sort.h | 3 +
5 files changed, 159 insertions(+), 5 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index cff3d9b..fa413ac 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -78,7 +78,7 @@ OPTIONS
-c::
--compute::
- Differential computation selection - delta,ratio (default is delta).
+ Differential computation selection - delta,ratio,wdiff (default is delta).
If '+' is specified as a first character, the output is sorted based
on the computation results.
See COMPARISON METHODS section for more info.
@@ -110,6 +110,19 @@ with:
- period being the hist entry period value
+wdiff
+~~~~~
+If specified the 'Weighted diff' column is displayed with value 'd' computed as:
+
+ d = B->period * WEIGHT-A - A->period * WEIGHT-B
+
+ - A/B being matching hist entry from first/second file specified
+ (or perf.data/perf.data.old) respectively.
+
+ - period being the hist entry period value
+
+ - WEIGHT-A/WEIGHT-B being user suplied weights in the the '-c' option
+ behind ':' separator like '-c wdiff:1,2'.
SEE ALSO
--------
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 14bc244..b0a08aa 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -27,24 +27,81 @@ static bool show_displacement;
static bool show_baseline_only;
static bool sort_compute;
+static s64 compute_wdiff_w1;
+static s64 compute_wdiff_w2;
+
enum {
COMPUTE_DELTA,
COMPUTE_RATIO,
+ COMPUTE_WEIGHTED_DIFF,
COMPUTE_MAX,
};
const char *compute_names[COMPUTE_MAX] = {
[COMPUTE_DELTA] = "delta",
[COMPUTE_RATIO] = "ratio",
+ [COMPUTE_WEIGHTED_DIFF] = "wdiff",
};
static int compute;
+static int setup_compute_opt_wdiff(char *opt)
+{
+ char *w1_str = opt;
+ char *w2_str;
+
+ int ret = -EINVAL;
+
+ if (!opt)
+ goto out;
+
+ w2_str = strchr(opt, ',');
+ if (!w2_str)
+ goto out;
+
+ *w2_str++ = 0x0;
+ if (!*w2_str)
+ goto out;
+
+ compute_wdiff_w1 = strtol(w1_str, NULL, 10);
+ compute_wdiff_w2 = strtol(w2_str, NULL, 10);
+
+ if (!compute_wdiff_w1 || !compute_wdiff_w2)
+ goto out;
+
+ pr_debug("compute wdiff w1(%" PRId64 ") w2(%" PRId64 ")\n",
+ compute_wdiff_w1, compute_wdiff_w2);
+
+ ret = 0;
+
+ out:
+ if (ret)
+ pr_err("Weight parsing failed.");
+
+ return ret;
+}
+
+static int setup_compute_opt(char *opt)
+{
+ if (compute == COMPUTE_WEIGHTED_DIFF)
+ return setup_compute_opt_wdiff(opt);
+
+ if (opt) {
+ pr_err("Extra option specified.");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int setup_compute(const struct option *opt, const char *str,
int unset __maybe_unused)
{
int *cp = (int *) opt->value;
+ char *cstr = (char *) str;
+ char buf[50];
unsigned i;
+ char *option;
if (!str) {
*cp = COMPUTE_DELTA;
@@ -53,15 +110,33 @@ static int setup_compute(const struct option *opt, const char *str,
if (*str == '+') {
sort_compute = true;
- str++;
+ cstr = (char *) ++str;
if (!*str)
return 0;
}
+ option = strchr(str, ':');
+ if (option) {
+ unsigned len = option++ - str;
+
+ /*
+ * The str data are not writeable, so we need
+ * to use another buffer.
+ */
+
+ /* No option value is longer. */
+ if (len >= sizeof(buf))
+ return -EINVAL;
+
+ strncpy(buf, str, len);
+ buf[len] = 0x0;
+ cstr = buf;
+ }
+
for (i = 0; i < COMPUTE_MAX; i++)
- if (!strcmp(str, compute_names[i])) {
+ if (!strcmp(cstr, compute_names[i])) {
*cp = i;
- return 0;
+ return setup_compute_opt(option);
}
pr_err("Failed to find valid compute string\n");
@@ -96,6 +171,23 @@ double perf_diff__compute_ratio(struct hist_entry *he)
return he->diff.period_ratio;
}
+double perf_diff__compute_wdiff(struct hist_entry *he)
+{
+ struct hist_entry *pair = he->pair;
+ u64 new_period = he->period;
+ u64 old_period = pair ? pair->period : 0;
+
+ he->diff.computed = true;
+
+ if (!pair)
+ he->diff.wdiff = 0;
+ else
+ he->diff.wdiff = new_period * compute_wdiff_w2 -
+ old_period * compute_wdiff_w1;
+
+ return he->diff.wdiff;
+}
+
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
{
@@ -274,6 +366,9 @@ static void hists__precompute(struct hists *hists)
case COMPUTE_RATIO:
perf_diff__compute_ratio(he);
break;
+ case COMPUTE_WEIGHTED_DIFF:
+ perf_diff__compute_wdiff(he);
+ break;
default:
BUG_ON(1);
}
@@ -309,6 +404,13 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
return cmp_doubles(l, r);
}
+ case COMPUTE_WEIGHTED_DIFF:
+ {
+ s64 l = left->diff.wdiff;
+ s64 r = right->diff.wdiff;
+
+ return r - l;
+ }
default:
BUG_ON(1);
}
@@ -433,7 +535,8 @@ static const struct option options[] = {
"Show position displacement relative to baseline"),
OPT_BOOLEAN('b', "baseline-only", &show_baseline_only,
"Show only items with match in baseline"),
- OPT_CALLBACK('c', "compute", &compute, "delta,ratio (default delta)",
+ OPT_CALLBACK('c', "compute", &compute,
+ "delta,ratio,wdiff:w1,w2 (default delta)",
"Entries differential computation selection",
setup_compute),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
@@ -474,6 +577,9 @@ static void ui_init(void)
case COMPUTE_RATIO:
perf_hpp__column_enable(PERF_HPP__RATIO, true);
break;
+ case COMPUTE_WEIGHTED_DIFF:
+ perf_hpp__column_enable(PERF_HPP__WEIGHTED_DIFF, true);
+ break;
default:
BUG_ON(1);
};
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 21b57a2..5980ca5 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -286,6 +286,35 @@ 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)
+{
+ 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)
+{
+ return 14;
+}
+
+static int hpp__entry_wdiff(struct perf_hpp *hpp, struct hist_entry *he)
+{
+ const char *fmt = symbol_conf.field_sep ? "%s" : "%14s";
+ char buf[32] = " ";
+ u64 wdiff;
+
+ if (he->diff.computed)
+ wdiff = he->diff.wdiff;
+ else
+ wdiff = perf_diff__compute_wdiff(he);
+
+ 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 *hpp)
{
return scnprintf(hpp->buf, hpp->size, "Displ.");
@@ -332,6 +361,7 @@ struct perf_hpp_fmt perf_hpp__format[] = {
{ .cond = false, HPP__PRINT_FNS(period) },
{ .cond = false, HPP__PRINT_FNS(delta) },
{ .cond = false, HPP__PRINT_FNS(ratio) },
+ { .cond = false, HPP__PRINT_FNS(wdiff) },
{ .cond = false, HPP__PRINT_FNS(displ) }
};
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2dbb340..27866b5 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -142,6 +142,7 @@ enum {
PERF_HPP__PERIOD,
PERF_HPP__DELTA,
PERF_HPP__RATIO,
+ PERF_HPP__WEIGHTED_DIFF,
PERF_HPP__DISPL,
PERF_HPP__MAX_INDEX
@@ -208,4 +209,5 @@ 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);
+double perf_diff__compute_wdiff(struct hist_entry *he);
#endif /* __PERF_HIST_H */
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 2090670..19d68ee 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -54,6 +54,9 @@ struct hist_entry_diff {
/* PERF_HPP__RATIO */
double period_ratio;
+
+ /* HISTC_WEIGHTED_DIFF */
+ s64 wdiff;
};
/**
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 11/14] perf diff: Add -p option to display period values for hist entries
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (9 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 10/14] perf diff: Add weighted diff computation way to compare hist entries Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 12/14] perf diff: Add -F option to display formula for computation Jiri Olsa
` (3 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Adding -p option to show period values for both compared hist
entries. Showing hist column PERF_HPP__PERIOD and newly added
hist column PERF_HPP__PERIOD_BASELINE.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 4 ++++
tools/perf/builtin-diff.c | 10 +++++++++-
tools/perf/ui/hist.c | 21 +++++++++++++++++++++
tools/perf/util/hist.h | 1 +
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index fa413ac..21cc2ef 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -83,6 +83,10 @@ OPTIONS
on the computation results.
See COMPARISON METHODS section for more info.
+-p::
+--period::
+ Show period values for both compared hist entries.
+
COMPARISON METHODS
------------------
delta
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index b0a08aa..799f395 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -24,6 +24,7 @@ static char const *input_old = "perf.data.old",
static char diff__default_sort_order[] = "dso,symbol";
static bool force;
static bool show_displacement;
+static bool show_period;
static bool show_baseline_only;
static bool sort_compute;
@@ -539,6 +540,8 @@ static const struct option options[] = {
"delta,ratio,wdiff:w1,w2 (default delta)",
"Entries differential computation selection",
setup_compute),
+ OPT_BOOLEAN('p', "period", &show_period,
+ "Show period values."),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
"dump raw trace in ASCII"),
OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
@@ -567,7 +570,7 @@ static void ui_init(void)
/* No overhead column. */
perf_hpp__column_enable(PERF_HPP__OVERHEAD, false);
- /* Display baseline/delta/ratio/displacement columns. */
+ /* Display baseline/delta/ratio/displacement/periods columns. */
perf_hpp__column_enable(PERF_HPP__BASELINE, true);
switch (compute) {
@@ -586,6 +589,11 @@ static void ui_init(void)
if (show_displacement)
perf_hpp__column_enable(PERF_HPP__DISPL, true);
+
+ if (show_period) {
+ perf_hpp__column_enable(PERF_HPP__PERIOD, true);
+ perf_hpp__column_enable(PERF_HPP__PERIOD_BASELINE, true);
+ }
}
int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 5980ca5..f97c16e 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -228,6 +228,26 @@ static int hpp__entry_period(struct perf_hpp *hpp, struct hist_entry *he)
return scnprintf(hpp->buf, hpp->size, fmt, he->period);
}
+static int hpp__header_period_baseline(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)
+{
+ return 12;
+}
+
+static int hpp__entry_period_baseline(struct perf_hpp *hpp, struct hist_entry *he)
+{
+ struct hist_entry *pair = he->pair;
+ u64 period = pair ? pair->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 *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
@@ -359,6 +379,7 @@ struct perf_hpp_fmt perf_hpp__format[] = {
{ .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) },
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 27866b5..ff307bf 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -140,6 +140,7 @@ 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,
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 12/14] perf diff: Add -F option to display formula for computation
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (10 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 11/14] perf diff: Add -p option to display period values for " Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 13/14] perf diff: Include samples without symbol in overall stats Jiri Olsa
` (2 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Adding -F option to display the formula for specified computation.
This is mainly to facilitate debugging, but can be useful anyway.
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>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/Documentation/perf-diff.txt | 4 ++
tools/perf/builtin-diff.c | 67 +++++++++++++++++++++++++++++++++-
tools/perf/ui/hist.c | 24 +++++++++++-
tools/perf/ui/stdio/hist.c | 2 +-
tools/perf/util/hist.h | 2 +
5 files changed, 96 insertions(+), 3 deletions(-)
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 21cc2ef..194f37d 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -87,6 +87,10 @@ OPTIONS
--period::
Show period values for both compared hist entries.
+-F::
+--formula::
+ Show formula for given computation.
+
COMPARISON METHODS
------------------
delta
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 799f395..504caf2 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -25,6 +25,7 @@ static char diff__default_sort_order[] = "dso,symbol";
static bool force;
static bool show_displacement;
static bool show_period;
+static bool show_formula;
static bool show_baseline_only;
static bool sort_compute;
@@ -189,6 +190,62 @@ double perf_diff__compute_wdiff(struct hist_entry *he)
return he->diff.wdiff;
}
+static int formula_delta(struct hist_entry *he, char *buf, size_t size)
+{
+ struct hist_entry *pair = he->pair;
+
+ if (!pair)
+ return -1;
+
+ return scnprintf(buf, size,
+ "(%" PRIu64 " * 100 / %" PRIu64 ") - "
+ "(%" PRIu64 " * 100 / %" PRIu64 ")",
+ he->period, he->hists->stats.total_period,
+ pair->period, pair->hists->stats.total_period);
+}
+
+static int formula_ratio(struct hist_entry *he, char *buf, size_t size)
+{
+ struct hist_entry *pair = he->pair;
+ double new_period = he->period;
+ double old_period = pair ? pair->period : 0;
+
+ if (!pair)
+ return -1;
+
+ return scnprintf(buf, size, "%.0F / %.0F", new_period, old_period);
+}
+
+static int formula_wdiff(struct hist_entry *he, char *buf, size_t size)
+{
+ struct hist_entry *pair = he->pair;
+ u64 new_period = he->period;
+ u64 old_period = pair ? pair->period : 0;
+
+ if (!pair)
+ return -1;
+
+ 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)
+{
+ switch (compute) {
+ case COMPUTE_DELTA:
+ return formula_delta(he, buf, size);
+ case COMPUTE_RATIO:
+ return formula_ratio(he, buf, size);
+ case COMPUTE_WEIGHTED_DIFF:
+ return formula_wdiff(he, buf, size);
+ default:
+ BUG_ON(1);
+ }
+
+ return -1;
+}
+
static int hists__add_entry(struct hists *self,
struct addr_location *al, u64 period)
{
@@ -542,6 +599,8 @@ static const struct option options[] = {
setup_compute),
OPT_BOOLEAN('p', "period", &show_period,
"Show period values."),
+ OPT_BOOLEAN('F', "formula", &show_formula,
+ "Show formula."),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
"dump raw trace in ASCII"),
OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
@@ -570,7 +629,10 @@ static void ui_init(void)
/* No overhead column. */
perf_hpp__column_enable(PERF_HPP__OVERHEAD, false);
- /* Display baseline/delta/ratio/displacement/periods columns. */
+ /*
+ * Display baseline/delta/ratio/displacement/
+ * formula/periods columns.
+ */
perf_hpp__column_enable(PERF_HPP__BASELINE, true);
switch (compute) {
@@ -590,6 +652,9 @@ static void ui_init(void)
if (show_displacement)
perf_hpp__column_enable(PERF_HPP__DISPL, true);
+ if (show_formula)
+ perf_hpp__column_enable(PERF_HPP__FORMULA, true);
+
if (show_period) {
perf_hpp__column_enable(PERF_HPP__PERIOD, true);
perf_hpp__column_enable(PERF_HPP__PERIOD_BASELINE, true);
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index f97c16e..04e34f3 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -359,6 +359,27 @@ 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)
+{
+ 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)
+{
+ return 70;
+}
+
+static int hpp__entry_formula(struct perf_hpp *hpp, struct hist_entry *he)
+{
+ const char *fmt = symbol_conf.field_sep ? "%s" : "%-70s";
+ char buf[96] = " ";
+
+ perf_diff__formula(buf, sizeof(buf), he);
+ return scnprintf(hpp->buf, hpp->size, fmt, buf);
+}
+
#define HPP__COLOR_PRINT_FNS(_name) \
.header = hpp__header_ ## _name, \
.width = hpp__width_ ## _name, \
@@ -383,7 +404,8 @@ struct perf_hpp_fmt perf_hpp__format[] = {
{ .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(displ) },
+ { .cond = false, HPP__PRINT_FNS(formula) }
};
#undef HPP__COLOR_PRINT_FNS
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 850c6d2..176b9ae 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -342,7 +342,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
const char *sep = symbol_conf.field_sep;
const char *col_width = symbol_conf.col_width_list_str;
int idx, nr_rows = 0;
- char bf[64];
+ char bf[96];
struct perf_hpp dummy_hpp = {
.buf = bf,
.size = sizeof(bf),
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index ff307bf..bedbd71 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -145,6 +145,7 @@ enum {
PERF_HPP__RATIO,
PERF_HPP__WEIGHTED_DIFF,
PERF_HPP__DISPL,
+ PERF_HPP__FORMULA,
PERF_HPP__MAX_INDEX
};
@@ -211,4 +212,5 @@ 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);
double perf_diff__compute_wdiff(struct hist_entry *he);
+int perf_diff__formula(char *buf, size_t size, struct hist_entry *he);
#endif /* __PERF_HIST_H */
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 13/14] perf diff: Include samples without symbol in overall stats
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (11 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 12/14] perf diff: Add -F option to display formula for computation Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-09-27 11:09 ` [PATCH 14/14] perf diff: Display empty space for non paired samples Jiri Olsa
2012-09-27 21:31 ` [PATCHv2 00/14] perf diff: Factor diff command Andi Kleen
14 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Currently we omit samples without symbols. This way we get
different and confusing numbers for samples than from report
command.
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>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/builtin-diff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 504caf2..e1e9004 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -268,7 +268,7 @@ static int diff__process_sample_event(struct perf_tool *tool __maybe_unused,
return -1;
}
- if (al.filtered || al.sym == NULL)
+ if (al.filtered)
return 0;
if (hists__add_entry(&evsel->hists, &al, sample->period)) {
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 14/14] perf diff: Display empty space for non paired samples
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (12 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 13/14] perf diff: Include samples without symbol in overall stats Jiri Olsa
@ 2012-09-27 11:09 ` Jiri Olsa
2012-10-04 6:06 ` Namhyung Kim
2012-09-27 21:31 ` [PATCHv2 00/14] perf diff: Factor diff command Andi Kleen
14 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2012-09-27 11:09 UTC (permalink / raw)
To: linux-kernel
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker, Namhyung Kim,
Jiri Olsa
Currently in 'Baseline' and 'Period Base' columns zero values are
displayed in case no pair is found for the sample. This might be
confusing, using empty space instead.
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>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
tools/perf/ui/hist.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 04e34f3..d74ecd2 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -179,7 +179,10 @@ static int hpp__color_baseline(struct perf_hpp *hpp, struct hist_entry *he)
{
double percent = baseline_percent(he);
- return percent_color_snprintf(hpp->buf, hpp->size, " %5.2f%%", percent);
+ if (he->pair)
+ return percent_color_snprintf(hpp->buf, hpp->size, " %5.2f%%", percent);
+ else
+ return scnprintf(hpp->buf, hpp->size, " ");
}
static int hpp__entry_baseline(struct perf_hpp *hpp, struct hist_entry *he)
@@ -246,8 +249,12 @@ static int hpp__entry_period_baseline(struct perf_hpp *hpp, struct hist_entry *h
u64 period = pair ? pair->period : 0;
const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
- return scnprintf(hpp->buf, hpp->size, fmt, period);
+ if (pair)
+ return scnprintf(hpp->buf, hpp->size, fmt, period);
+ else
+ return scnprintf(hpp->buf, hpp->size, " ");
}
+
static int hpp__header_delta(struct perf_hpp *hpp)
{
const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
--
1.7.11.4
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 14/14] perf diff: Display empty space for non paired samples
2012-09-27 11:09 ` [PATCH 14/14] perf diff: Display empty space for non paired samples Jiri Olsa
@ 2012-10-04 6:06 ` Namhyung Kim
0 siblings, 0 replies; 28+ messages in thread
From: Namhyung Kim @ 2012-10-04 6:06 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker
On Thu, 27 Sep 2012 13:09:35 +0200, Jiri Olsa wrote:
> Currently in 'Baseline' and 'Period Base' columns zero values are
> displayed in case no pair is found for the sample. This might be
> confusing, using empty space instead.
[snip]
> @@ -246,8 +249,12 @@ static int hpp__entry_period_baseline(struct perf_hpp *hpp, struct hist_entry *h
> u64 period = pair ? pair->period : 0;
> const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
>
> - return scnprintf(hpp->buf, hpp->size, fmt, period);
> + if (pair)
> + return scnprintf(hpp->buf, hpp->size, fmt, period);
> + else
> + return scnprintf(hpp->buf, hpp->size, " ");
It seems that it's not needed when -t (field separator) switch is given.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCHv2 00/14] perf diff: Factor diff command
2012-09-27 11:09 [PATCHv2 00/14] perf diff: Factor diff command Jiri Olsa
` (13 preceding siblings ...)
2012-09-27 11:09 ` [PATCH 14/14] perf diff: Display empty space for non paired samples Jiri Olsa
@ 2012-09-27 21:31 ` Andi Kleen
2012-10-01 8:16 ` Jiri Olsa
14 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2012-09-27 21:31 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, Andi Kleen, David Ahern, Namhyung Kim
On Thu, Sep 27, 2012 at 01:09:21PM +0200, Jiri Olsa wrote:
> hi,
> this is v2 of diff command changes proposed in here:
> https://lkml.org/lkml/2012/9/6/344
>
> It's now rebased on new hists hpp* interface plus few
> more additional changes.
FWIW I've been playing around with it a bit. It seems useful
both for scaling problems and likely for performance regression tracking too.
Some minor issues I ran into (but no show stoppers):
- The error messages for bad -c expressions could be better
- I found the requirement for no space after -c unintuitive.
- It would be nice to have support for doing the bucketizing per line
instead of per function. With a large function and/or inlining
it's sometimes hard to identify the actual problem
Acme recently added this for perf report.
I agree it would be good to merge.
-Andi
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCHv2 00/14] perf diff: Factor diff command
2012-09-27 21:31 ` [PATCHv2 00/14] perf diff: Factor diff command Andi Kleen
@ 2012-10-01 8:16 ` Jiri Olsa
2012-10-02 16:30 ` Andi Kleen
0 siblings, 1 reply; 28+ messages in thread
From: Jiri Olsa @ 2012-10-01 8:16 UTC (permalink / raw)
To: Andi Kleen
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, David Ahern, Namhyung Kim
On Thu, Sep 27, 2012 at 11:31:02PM +0200, Andi Kleen wrote:
> On Thu, Sep 27, 2012 at 01:09:21PM +0200, Jiri Olsa wrote:
> > hi,
> > this is v2 of diff command changes proposed in here:
> > https://lkml.org/lkml/2012/9/6/344
> >
> > It's now rebased on new hists hpp* interface plus few
> > more additional changes.
>
> FWIW I've been playing around with it a bit. It seems useful
> both for scaling problems and likely for performance regression tracking too.
cool ;)
>
> Some minor issues I ran into (but no show stoppers):
> - The error messages for bad -c expressions could be better
> - I found the requirement for no space after -c unintuitive.
I'll see to that
> - It would be nice to have support for doing the bucketizing per line
> instead of per function. With a large function and/or inlining
> it's sometimes hard to identify the actual problem
> Acme recently added this for perf report.
hm, I missed this one.. hopefully it should be no problem to add it
thanks for testing this!
jirka
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCHv2 00/14] perf diff: Factor diff command
2012-10-01 8:16 ` Jiri Olsa
@ 2012-10-02 16:30 ` Andi Kleen
2012-10-03 13:47 ` Arnaldo Carvalho de Melo
2012-10-03 16:55 ` Andi Kleen
0 siblings, 2 replies; 28+ messages in thread
From: Andi Kleen @ 2012-10-02 16:30 UTC (permalink / raw)
To: Jiri Olsa
Cc: Andi Kleen, linux-kernel, Arnaldo Carvalho de Melo,
Peter Zijlstra, Ingo Molnar, Paul Mackerras, Corey Ashford,
Frederic Weisbecker, Paul E. McKenney, David Ahern, Namhyung Kim
> > Some minor issues I ran into (but no show stoppers):
> > - The error messages for bad -c expressions could be better
> > - I found the requirement for no space after -c unintuitive.
>
> I'll see to that >
> > - It would be nice to have support for doing the bucketizing per line
> > instead of per function. With a large function and/or inlining
> > it's sometimes hard to identify the actual problem
> > Acme recently added this for perf report.
>
> hm, I missed this one.. hopefully it should be no problem to add it
Here's more issues I found (again no show stopper):
For example you have a spinlock problem with different spinlock callers
that you want to handle. Since the spinlocks are own functions which
are mixed together cannot be easily ratiod.
This is probably a bit hard to handle, but one way would be to use
a variant of the inclusive reporting Arun recently submitted.
Define groups of callers that have a combined cost including all
callees. Use this as the diff unit.
I really liked the oprofile behaviour of attributing lock costs
to the caller by default. With that it would just work.
The other problem I ran into is that perf archive doesn't seem to
work very well with kernels, so it's hard to move profiles from
different kernels around to diff them (e.g. for a performance regression)
One way around this would be options to diff to specify the vmlinux etc.
manually
-Andi
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCHv2 00/14] perf diff: Factor diff command
2012-10-02 16:30 ` Andi Kleen
@ 2012-10-03 13:47 ` Arnaldo Carvalho de Melo
2012-10-03 16:18 ` Andi Kleen
2012-10-03 16:55 ` Andi Kleen
1 sibling, 1 reply; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-10-03 13:47 UTC (permalink / raw)
To: Andi Kleen
Cc: Jiri Olsa, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, David Ahern, Namhyung Kim
Em Tue, Oct 02, 2012 at 06:30:02PM +0200, Andi Kleen escreveu:
> The other problem I ran into is that perf archive doesn't seem to work very
> well with kernels, so it's hard to move profiles from different kernels
> around to diff them (e.g. for a performance regression) One way around this
> would be options to diff to specify the vmlinux etc. manually
Can you describe more precisely what is not working well with 'archive'
and build ids?
When you do a 'perf record', at the end of the session, it traverses the
samples looking for DSOs with hits, and then stashes a copy (or a
hardlink, if possible) of these DSOs in your ~/.cache/, keyed by its
build id.
Later, when you do a perf archive, it will look again at your perf.data
files, look again for DSOs with hits and pass them on a tar command
line, you transfer those tarballs to your analysis machine and it should
just work.
To debug you may follow some of these steps:
[root@sandy linux]# perf record -a usleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.447 MB perf.data (~19534 samples) ]
[root@sandy linux]# perf buildid-list
62b97af6897371571caef35fd5ae88e29bde5e89 [kernel.kallsyms]
[root@sandy linux]# ls -la ~/.debug/.build-id/62/b97af6897371571caef35fd5ae88e29bde5e89
lrwxrwxrwx. 1 root root 64 Oct 3 10:41 /root/.debug/.build-id/62/b97af6897371571caef35fd5ae88e29bde5e89 -> ../../[kernel.kallsyms]/62b97af6897371571caef35fd5ae88e29bde5e89
[root@sandy linux]# perf buildid-list --hell
Error: unknown option `hell'
usage: perf buildid-list [<options>]
-H, --with-hits Show only DSOs with hits
-i, --input <file> input file name
-f, --force don't complain, do it
-k, --kernel Show current kernel build id
-v, --verbose be more verbose
[root@sandy linux]# perf buildid-list -k
62b97af6897371571caef35fd5ae88e29bde5e89
[root@sandy linux]# perf archive
Now please run:
$ tar xvf perf.data.tar.bz2 -C ~/.debug
wherever you need to run 'perf report' on.
[root@sandy linux]# tar tvf perf.data.tar.bz2
lrwxrwxrwx root/root 0 2012-10-03 10:41 .build-id/62/b97af6897371571caef35fd5ae88e29bde5e89 -> ../../[kernel.kallsyms]/62b97af6897371571caef35fd5ae88e29bde5e89
-rw-r--r-- root/root 4115683 2012-10-03 10:41 [kernel.kallsyms]/62b97af6897371571caef35fd5ae88e29bde5e89
[root@sandy linux]#
[root@sandy linux]# sha256sum /root/.debug/.build-id/62/b97af6897371571caef35fd5ae88e29bde5e89 /proc/kallsyms
43724edebe7f144d43a8664e237fa14df96bb79943002f1f17f45379a16418bf /root/.debug/.build-id/62/b97af6897371571caef35fd5ae88e29bde5e89
43724edebe7f144d43a8664e237fa14df96bb79943002f1f17f45379a16418bf /proc/kallsyms
[root@sandy linux]#
- Arnaldo
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCHv2 00/14] perf diff: Factor diff command
2012-10-03 13:47 ` Arnaldo Carvalho de Melo
@ 2012-10-03 16:18 ` Andi Kleen
2012-10-03 16:53 ` Andi Kleen
0 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2012-10-03 16:18 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Andi Kleen, Jiri Olsa, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, David Ahern, Namhyung Kim
On Wed, Oct 03, 2012 at 06:47:57AM -0700, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 02, 2012 at 06:30:02PM +0200, Andi Kleen escreveu:
> > The other problem I ran into is that perf archive doesn't seem to work very
> > well with kernels, so it's hard to move profiles from different kernels
> > around to diff them (e.g. for a performance regression) One way around this
> > would be options to diff to specify the vmlinux etc. manually
>
> Can you describe more precisely what is not working well with 'archive'
> and build ids?
It doesn't write anything to anywhere for me
It also breaks with different perf binaries because it seems to just
do system("perf ...") at some point. This may be related to this.
And synching .cache is a quite clumpsy way even if it worked.
% strace -f -e open -o log perf archive
% grep WR log
20041 open("/dev/tty", O_RDWR|O_NONBLOCK) = 3
20042 open("/tmp/perf-archive-buildids.5uDTJ2", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
20044 open("/tmp/perf-archive-buildids.5uDTJ2", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
20045 open("/tmp/perf-archive-manifest.jRy7nP", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
20047 open("/tmp/perf-archive-manifest.jRy7nP", O_WRONLY|O_CREAT|O_APPEND, 0666) = 3
20047 open("/tmp/perf-archive-manifest.jRy7nP", O_WRONLY|O_CREAT|O_APPEND, 0666) = 3
20047 open("/tmp/perf-archive-manifest.jRy7nP", O_WRONLY|O_CREAT|O_APPEND, 0666) = 3
20047 open("/tmp/perf-archive-manifest.jRy7nP", O_WRONLY|O_CREAT|O_APPEND, 0666) = 3
%
-Andi
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCHv2 00/14] perf diff: Factor diff command
2012-10-03 16:18 ` Andi Kleen
@ 2012-10-03 16:53 ` Andi Kleen
2012-10-03 18:06 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2012-10-03 16:53 UTC (permalink / raw)
To: Andi Kleen
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel,
Peter Zijlstra, Ingo Molnar, Paul Mackerras, Corey Ashford,
Frederic Weisbecker, Paul E. McKenney, David Ahern, Namhyung Kim
On Wed, Oct 03, 2012 at 06:18:16PM +0200, Andi Kleen wrote:
> On Wed, Oct 03, 2012 at 06:47:57AM -0700, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Oct 02, 2012 at 06:30:02PM +0200, Andi Kleen escreveu:
> > > The other problem I ran into is that perf archive doesn't seem to work very
> > > well with kernels, so it's hard to move profiles from different kernels
> > > around to diff them (e.g. for a performance regression) One way around this
> > > would be options to diff to specify the vmlinux etc. manually
> >
> > Can you describe more precisely what is not working well with 'archive'
> > and build ids?
>
> It doesn't write anything to anywhere for me
>
> It also breaks with different perf binaries because it seems to just
> do system("perf ...") at some point. This may be related to this.
The problem is with the perf-archive script. After some hacking I got it running
now. I suspect the perf tool needs to pass its own executable name to
the script to run.
Also would be nice if another file than perf.data could be specified
(currently have to symlink)
-Andi
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCHv2 00/14] perf diff: Factor diff command
2012-10-03 16:53 ` Andi Kleen
@ 2012-10-03 18:06 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-10-03 18:06 UTC (permalink / raw)
To: Andi Kleen
Cc: Jiri Olsa, linux-kernel, Peter Zijlstra, Ingo Molnar,
Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, David Ahern, Namhyung Kim
Em Wed, Oct 03, 2012 at 06:53:42PM +0200, Andi Kleen escreveu:
> The problem is with the perf-archive script. After some hacking I got it running
> now. I suspect the perf tool needs to pass its own executable name to
> the script to run.
>
> Also would be nice if another file than perf.data could be specified
> (currently have to symlink)
It was done as a proof of concept to illustrate using scripts as
porcelain, just like in git, but yeah, it needs to be rewritten in C to
handle different filenames, etc, will try to move it to the head of my
todo list.
- Arnaldo
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCHv2 00/14] perf diff: Factor diff command
2012-10-02 16:30 ` Andi Kleen
2012-10-03 13:47 ` Arnaldo Carvalho de Melo
@ 2012-10-03 16:55 ` Andi Kleen
2012-10-03 17:01 ` Jiri Olsa
1 sibling, 1 reply; 28+ messages in thread
From: Andi Kleen @ 2012-10-03 16:55 UTC (permalink / raw)
To: Andi Kleen
Cc: Jiri Olsa, linux-kernel, Arnaldo Carvalho de Melo,
Peter Zijlstra, Ingo Molnar, Paul Mackerras, Corey Ashford,
Frederic Weisbecker, Paul E. McKenney, David Ahern, Namhyung Kim
> > hm, I missed this one.. hopefully it should be no problem to add it
>
> Here's more issues I found (again no show stopper):
And another issue (this one is more serious):
I was trying to compare the same program with two different kernels.
But perf diff does not recognize the same functions in the kernels
as the same bucket. So I cannot actually "ratio" the different
kernel versions.
-Andi
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCHv2 00/14] perf diff: Factor diff command
2012-10-03 16:55 ` Andi Kleen
@ 2012-10-03 17:01 ` Jiri Olsa
0 siblings, 0 replies; 28+ messages in thread
From: Jiri Olsa @ 2012-10-03 17:01 UTC (permalink / raw)
To: Andi Kleen
Cc: linux-kernel, Arnaldo Carvalho de Melo, Peter Zijlstra,
Ingo Molnar, Paul Mackerras, Corey Ashford, Frederic Weisbecker,
Paul E. McKenney, David Ahern, Namhyung Kim
On Wed, Oct 03, 2012 at 06:55:32PM +0200, Andi Kleen wrote:
> > > hm, I missed this one.. hopefully it should be no problem to add it
> >
> > Here's more issues I found (again no show stopper):
>
> And another issue (this one is more serious):
>
> I was trying to compare the same program with two different kernels.
> But perf diff does not recognize the same functions in the kernels
> as the same bucket. So I cannot actually "ratio" the different
> kernel versions.
good point, I'll check how we can fix this
thanks,
jirka
^ permalink raw reply [flat|nested] 28+ messages in thread