* [PATCH 1/7] perf diff: Fix to sort by baseline field by default
2015-01-08 0:45 [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Namhyung Kim
@ 2015-01-08 0:45 ` Namhyung Kim
2015-01-08 0:45 ` [PATCH 2/7] perf diff: Get rid of hists__compute_resort() Namhyung Kim
` (7 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Namhyung Kim @ 2015-01-08 0:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang
The currently perf diff didn't add the baseline and delta (or other
compute) fields to the sort list so output will be sorted by other
fields like alphabetical order of DSO or symbol as below example.
Fix it by adding hpp formats for the fields and provides default
compare functions.
Before:
$ perf diff
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol
# ........ ....... .................. ...............................
#
[bridge] [k] ip_sabotage_in
[btrfs] [k] __etree_search.constprop.47
0.01% [btrfs] [k] btrfs_file_mmap
0.01% -0.01% [btrfs] [k] btrfs_getattr
[e1000e] [k] e1000_watchdog
0.00% [kernel.vmlinux] [k] PageHuge
0.00% [kernel.vmlinux] [k] __acct_update_integrals
0.00% [kernel.vmlinux] [k] __activate_page
[kernel.vmlinux] [k] __alloc_fd
0.02% +0.02% [kernel.vmlinux] [k] __alloc_pages_nodemask
...
After:
# Baseline Delta Shared Object Symbol
# ........ ....... .................. ................................
#
24.73% -4.62% perf [.] append_chain_children
7.96% -1.29% perf [.] dso__find_symbol
6.97% -2.07% libc-2.20.so [.] vfprintf
4.61% +0.88% libc-2.20.so [.] __fprintf_chk
4.41% +2.43% perf [.] sort__comm_cmp
4.10% -0.16% perf [.] comm__str
4.03% -0.93% perf [.] machine__findnew_thread_time
3.82% +3.09% perf [.] __hists__add_entry
2.95% -0.18% perf [.] sort__dso_cmp
...
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index e787a4406d1f..318ab9c3f0ba 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -545,6 +545,42 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
return __hist_entry__cmp_compute(p_left, p_right, c);
}
+static int64_t
+hist_entry__cmp_nop(struct hist_entry *left __maybe_unused,
+ struct hist_entry *right __maybe_unused)
+{
+ return 0;
+}
+
+static int64_t
+hist_entry__cmp_baseline(struct hist_entry *left, struct hist_entry *right)
+{
+ if (sort_compute)
+ return 0;
+
+ if (left->stat.period == right->stat.period)
+ return 0;
+ return left->stat.period > right->stat.period ? 1 : -1;
+}
+
+static int64_t
+hist_entry__cmp_delta(struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_compute(right, left, COMPUTE_DELTA);
+}
+
+static int64_t
+hist_entry__cmp_ratio(struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_compute(right, left, COMPUTE_RATIO);
+}
+
+static int64_t
+hist_entry__cmp_wdiff(struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF);
+}
+
static void insert_hist_entry_by_compute(struct rb_root *root,
struct hist_entry *he,
int c)
@@ -1038,27 +1074,35 @@ static void data__hpp_register(struct data__file *d, int idx)
fmt->header = hpp__header;
fmt->width = hpp__width;
fmt->entry = hpp__entry_global;
+ fmt->cmp = hist_entry__cmp_nop;
+ fmt->collapse = hist_entry__cmp_nop;
/* TODO more colors */
switch (idx) {
case PERF_HPP_DIFF__BASELINE:
fmt->color = hpp__color_baseline;
+ fmt->sort = hist_entry__cmp_baseline;
break;
case PERF_HPP_DIFF__DELTA:
fmt->color = hpp__color_delta;
+ fmt->sort = hist_entry__cmp_delta;
break;
case PERF_HPP_DIFF__RATIO:
fmt->color = hpp__color_ratio;
+ fmt->sort = hist_entry__cmp_ratio;
break;
case PERF_HPP_DIFF__WEIGHTED_DIFF:
fmt->color = hpp__color_wdiff;
+ fmt->sort = hist_entry__cmp_wdiff;
break;
default:
+ fmt->sort = hist_entry__cmp_nop;
break;
}
init_header(d, dfmt);
perf_hpp__column_register(fmt);
+ perf_hpp__register_sort_field(fmt);
}
static void ui_init(void)
--
2.1.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 2/7] perf diff: Get rid of hists__compute_resort()
2015-01-08 0:45 [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Namhyung Kim
2015-01-08 0:45 ` [PATCH 1/7] perf diff: Fix to sort by baseline field by default Namhyung Kim
@ 2015-01-08 0:45 ` Namhyung Kim
2015-01-08 0:45 ` [PATCH 3/7] perf diff: Print diff result more precisely Namhyung Kim
` (6 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Namhyung Kim @ 2015-01-08 0:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang,
Jiri Olsa
The hists__compute_resort() is to sort output fields based on the
given field/criteria. This was done without the sort list but as we
added the field to the sort list, we can do it with normal
hists__output_resort() using the ->sort callback.
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 59 +++--------------------------------------------
1 file changed, 3 insertions(+), 56 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 318ab9c3f0ba..72c718e6549c 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -581,68 +581,15 @@ hist_entry__cmp_wdiff(struct hist_entry *left, struct hist_entry *right)
return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF);
}
-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 *root;
- struct rb_node *next;
-
- if (sort__need_collapse)
- root = &hists->entries_collapsed;
- else
- root = hists->entries_in;
-
- hists->entries = RB_ROOT;
- next = rb_first(root);
-
- hists__reset_stats(hists);
- hists__reset_col_len(hists);
-
- while (next != NULL) {
- struct hist_entry *he;
-
- he = rb_entry(next, struct hist_entry, rb_node_in);
- next = rb_next(&he->rb_node_in);
-
- insert_hist_entry_by_compute(&hists->entries, he, compute);
- hists__inc_stats(hists, he);
-
- if (!he->filtered)
- hists__calc_col_len(hists, he);
- }
-}
-
static void hists__process(struct hists *hists)
{
if (show_baseline_only)
hists__baseline_only(hists);
- if (sort_compute) {
+ if (sort_compute)
hists__precompute(hists);
- hists__compute_resort(hists);
- } else {
- hists__output_resort(hists, NULL);
- }
+
+ hists__output_resort(hists, NULL);
hists__fprintf(hists, true, 0, 0, 0, stdout);
}
--
2.1.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 3/7] perf diff: Print diff result more precisely
2015-01-08 0:45 [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Namhyung Kim
2015-01-08 0:45 ` [PATCH 1/7] perf diff: Fix to sort by baseline field by default Namhyung Kim
2015-01-08 0:45 ` [PATCH 2/7] perf diff: Get rid of hists__compute_resort() Namhyung Kim
@ 2015-01-08 0:45 ` Namhyung Kim
2015-01-08 0:45 ` [PATCH 4/7] perf diff: Introduce fmt_to_data_file() helper Namhyung Kim
` (5 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Namhyung Kim @ 2015-01-08 0:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang,
Jiri Olsa
Current perf diff result is somewhat confusing since it sometimes hide
small result and sometimes there's no result. So do not hide small
result (less than 0.01%) and print "N/A" if baseline is not
recorded (for ratio and wdiff only). Blank means the baseline is
available but its pairs are not.
Before:
# Baseline Delta Shared Object Symbol
# ........ ....... ................. .........................
#
...
0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
0.01% [kernel.kallsyms] [k] scheduler_tick
0.01% [kernel.kallsyms] [k] native_read_msr_safe
0.00% [kernel.kallsyms] [k] __rcu_read_unlock
[kernel.kallsyms] [k] _raw_spin_lock
+0.01% [kernel.kallsyms] [k] apic_timer_interrupt
[kernel.kallsyms] [k] read_tsc
After:
# Baseline Delta Shared Object Symbol
# ........ ....... ................. .........................
#
...
0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
0.01% [kernel.kallsyms] [k] scheduler_tick
0.01% [kernel.kallsyms] [k] native_read_msr_safe
0.00% [kernel.kallsyms] [k] __rcu_read_unlock
+0.01% [kernel.kallsyms] [k] _raw_spin_lock
+0.01% [kernel.kallsyms] [k] apic_timer_interrupt
+0.01% [kernel.kallsyms] [k] read_tsc
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 72c718e6549c..3f86737da2c4 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -788,7 +788,7 @@ static int __hpp__color_compare(struct perf_hpp_fmt *fmt,
char pfmt[20] = " ";
if (!pair)
- goto dummy_print;
+ goto no_print;
switch (comparison_method) {
case COMPUTE_DELTA:
@@ -797,8 +797,6 @@ static int __hpp__color_compare(struct perf_hpp_fmt *fmt,
else
diff = compute_delta(he, pair);
- if (fabs(diff) < 0.01)
- goto dummy_print;
scnprintf(pfmt, 20, "%%%+d.2f%%%%", dfmt->header_width - 1);
return percent_color_snprintf(hpp->buf, hpp->size,
pfmt, diff);
@@ -830,6 +828,9 @@ static int __hpp__color_compare(struct perf_hpp_fmt *fmt,
}
dummy_print:
return scnprintf(hpp->buf, hpp->size, "%*s",
+ dfmt->header_width, "N/A");
+no_print:
+ return scnprintf(hpp->buf, hpp->size, "%*s",
dfmt->header_width, pfmt);
}
@@ -879,14 +880,15 @@ hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
else
diff = compute_delta(he, pair);
- if (fabs(diff) >= 0.01)
- scnprintf(buf, size, "%+4.2F%%", diff);
+ scnprintf(buf, size, "%+4.2F%%", diff);
break;
case PERF_HPP_DIFF__RATIO:
/* No point for ratio number if we are dummy.. */
- if (he->dummy)
+ if (he->dummy) {
+ scnprintf(buf, size, "N/A");
break;
+ }
if (pair->diff.computed)
ratio = pair->diff.period_ratio;
@@ -899,8 +901,10 @@ hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
case PERF_HPP_DIFF__WEIGHTED_DIFF:
/* No point for wdiff number if we are dummy.. */
- if (he->dummy)
+ if (he->dummy) {
+ scnprintf(buf, size, "N/A");
break;
+ }
if (pair->diff.computed)
wdiff = pair->diff.wdiff;
--
2.1.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 4/7] perf diff: Introduce fmt_to_data_file() helper
2015-01-08 0:45 [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Namhyung Kim
` (2 preceding siblings ...)
2015-01-08 0:45 ` [PATCH 3/7] perf diff: Print diff result more precisely Namhyung Kim
@ 2015-01-08 0:45 ` Namhyung Kim
2015-01-28 15:07 ` [tip:perf/core] " tip-bot for Namhyung Kim
2015-01-08 0:45 ` [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks Namhyung Kim
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2015-01-08 0:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang
The fmt_to_data_file() is to retrieve struct data__file from
perf_hpp_fmt which is embedded in diff_hpp_fmt. It'll be used by sort
callback functions later.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 3f86737da2c4..ae8f62151b34 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -390,6 +390,15 @@ static void perf_evlist__collapse_resort(struct perf_evlist *evlist)
}
}
+static struct data__file *fmt_to_data_file(struct perf_hpp_fmt *fmt)
+{
+ struct diff_hpp_fmt *dfmt = container_of(fmt, struct diff_hpp_fmt, fmt);
+ void *ptr = dfmt - dfmt->idx;
+ struct data__file *d = container_of(ptr, struct data__file, fmt);
+
+ return d;
+}
+
static struct hist_entry*
get_pair_data(struct hist_entry *he, struct data__file *d)
{
@@ -407,8 +416,7 @@ get_pair_data(struct hist_entry *he, struct data__file *d)
static struct hist_entry*
get_pair_fmt(struct hist_entry *he, struct diff_hpp_fmt *dfmt)
{
- void *ptr = dfmt - dfmt->idx;
- struct data__file *d = container_of(ptr, struct data__file, fmt);
+ struct data__file *d = fmt_to_data_file(&dfmt->fmt);
return get_pair_data(he, d);
}
--
2.1.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [tip:perf/core] perf diff: Introduce fmt_to_data_file() helper
2015-01-08 0:45 ` [PATCH 4/7] perf diff: Introduce fmt_to_data_file() helper Namhyung Kim
@ 2015-01-28 15:07 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-01-28 15:07 UTC (permalink / raw)
To: linux-tip-commits
Cc: a.p.zijlstra, tglx, linux-kernel, kan.liang, jolsa, hpa, namhyung,
acme, mingo, jolsa
Commit-ID: ff21cef67e85ae77682560973b8c80ef64125221
Gitweb: http://git.kernel.org/tip/ff21cef67e85ae77682560973b8c80ef64125221
Author: Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 8 Jan 2015 09:45:45 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 21 Jan 2015 13:24:34 -0300
perf diff: Introduce fmt_to_data_file() helper
The fmt_to_data_file() is to retrieve struct data__file from
perf_hpp_fmt which is embedded in diff_hpp_fmt. It'll be used by sort
callback functions later.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1420677949-6719-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-diff.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 3f86737..ae8f621 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -390,6 +390,15 @@ static void perf_evlist__collapse_resort(struct perf_evlist *evlist)
}
}
+static struct data__file *fmt_to_data_file(struct perf_hpp_fmt *fmt)
+{
+ struct diff_hpp_fmt *dfmt = container_of(fmt, struct diff_hpp_fmt, fmt);
+ void *ptr = dfmt - dfmt->idx;
+ struct data__file *d = container_of(ptr, struct data__file, fmt);
+
+ return d;
+}
+
static struct hist_entry*
get_pair_data(struct hist_entry *he, struct data__file *d)
{
@@ -407,8 +416,7 @@ get_pair_data(struct hist_entry *he, struct data__file *d)
static struct hist_entry*
get_pair_fmt(struct hist_entry *he, struct diff_hpp_fmt *dfmt)
{
- void *ptr = dfmt - dfmt->idx;
- struct data__file *d = container_of(ptr, struct data__file, fmt);
+ struct data__file *d = fmt_to_data_file(&dfmt->fmt);
return get_pair_data(he, d);
}
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks
2015-01-08 0:45 [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Namhyung Kim
` (3 preceding siblings ...)
2015-01-08 0:45 ` [PATCH 4/7] perf diff: Introduce fmt_to_data_file() helper Namhyung Kim
@ 2015-01-08 0:45 ` Namhyung Kim
2015-01-12 16:21 ` Arnaldo Carvalho de Melo
2015-01-28 15:07 ` [tip:perf/core] " tip-bot for Namhyung Kim
2015-01-08 0:45 ` [PATCH 6/7] perf diff: Fix output ordering to honor next column Namhyung Kim
` (3 subsequent siblings)
8 siblings, 2 replies; 20+ messages in thread
From: Namhyung Kim @ 2015-01-08 0:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang,
Jiri Olsa
Currently ->cmp, ->collapse and ->sort callbacks doesn't pass
corresponding fmt. But it'll be needed by upcoming changes in
perf diff command.
Suggested-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/hist.c | 12 ++++++++----
tools/perf/util/hist.c | 6 +++---
tools/perf/util/hist.h | 9 ++++++---
tools/perf/util/sort.c | 37 ++++++++++++++++++++++++++++++++++---
4 files changed, 51 insertions(+), 13 deletions(-)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index dc0d095f318c..a2db53f44a5b 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -282,7 +282,8 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *fmt, \
}
#define __HPP_SORT_FN(_type, _field) \
-static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b) \
+static int64_t hpp__sort_##_type(struct perf_hpp_fmt *fmt __maybe_unused, \
+ struct hist_entry *a, struct hist_entry *b) \
{ \
return __hpp__sort(a, b, he_get_##_field); \
}
@@ -309,7 +310,8 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *fmt, \
}
#define __HPP_SORT_ACC_FN(_type, _field) \
-static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b) \
+static int64_t hpp__sort_##_type(struct perf_hpp_fmt *fmt __maybe_unused, \
+ struct hist_entry *a, struct hist_entry *b) \
{ \
return __hpp__sort_acc(a, b, he_get_acc_##_field); \
}
@@ -328,7 +330,8 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *fmt, \
}
#define __HPP_SORT_RAW_FN(_type, _field) \
-static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b) \
+static int64_t hpp__sort_##_type(struct perf_hpp_fmt *fmt __maybe_unused, \
+ struct hist_entry *a, struct hist_entry *b) \
{ \
return __hpp__sort(a, b, he_get_raw_##_field); \
}
@@ -358,7 +361,8 @@ HPP_PERCENT_ACC_FNS(overhead_acc, period)
HPP_RAW_FNS(samples, nr_events)
HPP_RAW_FNS(period, period)
-static int64_t hpp__nop_cmp(struct hist_entry *a __maybe_unused,
+static int64_t hpp__nop_cmp(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *a __maybe_unused,
struct hist_entry *b __maybe_unused)
{
return 0;
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 30ff2cb92884..da77d52984db 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -913,7 +913,7 @@ hist_entry__cmp(struct hist_entry *left, struct hist_entry *right)
if (perf_hpp__should_skip(fmt))
continue;
- cmp = fmt->cmp(left, right);
+ cmp = fmt->cmp(fmt, left, right);
if (cmp)
break;
}
@@ -931,7 +931,7 @@ hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)
if (perf_hpp__should_skip(fmt))
continue;
- cmp = fmt->collapse(left, right);
+ cmp = fmt->collapse(fmt, left, right);
if (cmp)
break;
}
@@ -1060,7 +1060,7 @@ static int hist_entry__sort(struct hist_entry *a, struct hist_entry *b)
if (perf_hpp__should_skip(fmt))
continue;
- cmp = fmt->sort(a, b);
+ cmp = fmt->sort(fmt, a, b);
if (cmp)
break;
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 9305580cd53e..2b690d028907 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -195,9 +195,12 @@ struct perf_hpp_fmt {
struct hist_entry *he);
int (*entry)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
struct hist_entry *he);
- int64_t (*cmp)(struct hist_entry *a, struct hist_entry *b);
- int64_t (*collapse)(struct hist_entry *a, struct hist_entry *b);
- int64_t (*sort)(struct hist_entry *a, struct hist_entry *b);
+ int64_t (*cmp)(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b);
+ int64_t (*collapse)(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b);
+ int64_t (*sort)(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b);
struct list_head list;
struct list_head sort_list;
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 9139dda9f9a3..7a39c1ed8d37 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1304,6 +1304,37 @@ static int __sort__hpp_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
return hse->se->se_snprintf(he, hpp->buf, hpp->size, len);
}
+static int64_t __sort__hpp_cmp(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b)
+{
+ struct hpp_sort_entry *hse;
+
+ hse = container_of(fmt, struct hpp_sort_entry, hpp);
+ return hse->se->se_cmp(a, b);
+}
+
+static int64_t __sort__hpp_collapse(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b)
+{
+ struct hpp_sort_entry *hse;
+ int64_t (*collapse_fn)(struct hist_entry *, struct hist_entry *);
+
+ hse = container_of(fmt, struct hpp_sort_entry, hpp);
+ collapse_fn = hse->se->se_collapse ?: hse->se->se_cmp;
+ return collapse_fn(a, b);
+}
+
+static int64_t __sort__hpp_sort(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b)
+{
+ struct hpp_sort_entry *hse;
+ int64_t (*sort_fn)(struct hist_entry *, struct hist_entry *);
+
+ hse = container_of(fmt, struct hpp_sort_entry, hpp);
+ sort_fn = hse->se->se_sort ?: hse->se->se_cmp;
+ return sort_fn(a, b);
+}
+
static struct hpp_sort_entry *
__sort_dimension__alloc_hpp(struct sort_dimension *sd)
{
@@ -1322,9 +1353,9 @@ __sort_dimension__alloc_hpp(struct sort_dimension *sd)
hse->hpp.entry = __sort__hpp_entry;
hse->hpp.color = NULL;
- hse->hpp.cmp = sd->entry->se_cmp;
- hse->hpp.collapse = sd->entry->se_collapse ? : sd->entry->se_cmp;
- hse->hpp.sort = sd->entry->se_sort ? : hse->hpp.collapse;
+ hse->hpp.cmp = __sort__hpp_cmp;
+ hse->hpp.collapse = __sort__hpp_collapse;
+ hse->hpp.sort = __sort__hpp_sort;
INIT_LIST_HEAD(&hse->hpp.list);
INIT_LIST_HEAD(&hse->hpp.sort_list);
--
2.1.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks
2015-01-08 0:45 ` [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks Namhyung Kim
@ 2015-01-12 16:21 ` Arnaldo Carvalho de Melo
2015-01-12 16:27 ` Arnaldo Carvalho de Melo
2015-01-28 15:07 ` [tip:perf/core] " tip-bot for Namhyung Kim
1 sibling, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-12 16:21 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang,
Jiri Olsa
Em Thu, Jan 08, 2015 at 09:45:46AM +0900, Namhyung Kim escreveu:
> Currently ->cmp, ->collapse and ->sort callbacks doesn't pass
> corresponding fmt. But it'll be needed by upcoming changes in
> perf diff command.
>
> Suggested-by: Jiri Olsa <jolsa@kernel.org>
CC /tmp/build/perf/builtin-evlist.o
builtin-diff.c: In function ‘data__hpp_register’:
builtin-diff.c:1036:11: error: assignment from incompatible pointer type [-Werror]
fmt->cmp = hist_entry__cmp_nop;
^
builtin-diff.c:1037:16: error: assignment from incompatible pointer type [-Werror]
fmt->collapse = hist_entry__cmp_nop;
^
builtin-diff.c:1043:13: error: assignment from incompatible pointer type [-Werror]
fmt->sort = hist_entry__cmp_baseline;
^
builtin-diff.c:1047:13: error: assignment from incompatible pointer type [-Werror]
fmt->sort = hist_entry__cmp_delta;
^
builtin-diff.c:1051:13: error: assignment from incompatible pointer type [-Werror]
fmt->sort = hist_entry__cmp_ratio;
^
builtin-diff.c:1055:13: error: assignment from incompatible pointer type [-Werror]
fmt->sort = hist_entry__cmp_wdiff;
^
builtin-diff.c:1058:13: error: assignment from incompatible pointer type [-Werror]
fmt->sort = hist_entry__cmp_nop;
^
cc1: all warnings being treated as errors
LINK plugin_kvm.so
make[1]: *** [/tmp/build/perf/builtin-diff.o] Error 1
make[1]: *** Waiting for unfinished jobs....
LINK plugin_mac80211.so
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks
2015-01-12 16:21 ` Arnaldo Carvalho de Melo
@ 2015-01-12 16:27 ` Arnaldo Carvalho de Melo
2015-01-12 16:39 ` Arnaldo Carvalho de Melo
2015-01-12 16:40 ` Jiri Olsa
0 siblings, 2 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-12 16:27 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang,
Jiri Olsa
Em Mon, Jan 12, 2015 at 01:21:40PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 08, 2015 at 09:45:46AM +0900, Namhyung Kim escreveu:
> > Currently ->cmp, ->collapse and ->sort callbacks doesn't pass
> > corresponding fmt. But it'll be needed by upcoming changes in
> > perf diff command.
> >
> > Suggested-by: Jiri Olsa <jolsa@kernel.org>
>
>
> CC /tmp/build/perf/builtin-evlist.o
> builtin-diff.c: In function ‘data__hpp_register’:
> builtin-diff.c:1036:11: error: assignment from incompatible pointer type [-Werror]
> fmt->cmp = hist_entry__cmp_nop;
> ^
> builtin-diff.c:1037:16: error: assignment from incompatible pointer type [-Werror]
> fmt->collapse = hist_entry__cmp_nop;
> ^
> builtin-diff.c:1043:13: error: assignment from incompatible pointer type [-Werror]
> fmt->sort = hist_entry__cmp_baseline;
> ^
> builtin-diff.c:1047:13: error: assignment from incompatible pointer type [-Werror]
> fmt->sort = hist_entry__cmp_delta;
> ^
> builtin-diff.c:1051:13: error: assignment from incompatible pointer type [-Werror]
> fmt->sort = hist_entry__cmp_ratio;
> ^
> builtin-diff.c:1055:13: error: assignment from incompatible pointer type [-Werror]
> fmt->sort = hist_entry__cmp_wdiff;
> ^
> builtin-diff.c:1058:13: error: assignment from incompatible pointer type [-Werror]
> fmt->sort = hist_entry__cmp_nop;
> ^
> cc1: all warnings being treated as errors
> LINK plugin_kvm.so
> make[1]: *** [/tmp/build/perf/builtin-diff.o] Error 1
> make[1]: *** Waiting for unfinished jobs....
> LINK plugin_mac80211.so
Am I missing something? Lemme continue trying applying the rest of the
kit...
- Arnaldo
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index ae8f62151b34..4816989a84b0 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -554,14 +554,16 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
}
static int64_t
-hist_entry__cmp_nop(struct hist_entry *left __maybe_unused,
+hist_entry__cmp_nop(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left __maybe_unused,
struct hist_entry *right __maybe_unused)
{
return 0;
}
static int64_t
-hist_entry__cmp_baseline(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
{
if (sort_compute)
return 0;
@@ -572,19 +574,22 @@ hist_entry__cmp_baseline(struct hist_entry *left, struct hist_entry *right)
}
static int64_t
-hist_entry__cmp_delta(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_delta(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
{
return hist_entry__cmp_compute(right, left, COMPUTE_DELTA);
}
static int64_t
-hist_entry__cmp_ratio(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_ratio(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
{
return hist_entry__cmp_compute(right, left, COMPUTE_RATIO);
}
static int64_t
-hist_entry__cmp_wdiff(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
{
return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF);
}
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks
2015-01-12 16:27 ` Arnaldo Carvalho de Melo
@ 2015-01-12 16:39 ` Arnaldo Carvalho de Melo
2015-01-12 16:40 ` Jiri Olsa
1 sibling, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-12 16:39 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang,
Jiri Olsa
Em Mon, Jan 12, 2015 at 01:27:36PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Jan 12, 2015 at 01:21:40PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jan 08, 2015 at 09:45:46AM +0900, Namhyung Kim escreveu:
> > > Currently ->cmp, ->collapse and ->sort callbacks doesn't pass
> > > corresponding fmt. But it'll be needed by upcoming changes in
> > > perf diff command.
> > >
> > > Suggested-by: Jiri Olsa <jolsa@kernel.org>
> >
> >
> > CC /tmp/build/perf/builtin-evlist.o
> > builtin-diff.c: In function ‘data__hpp_register’:
> > builtin-diff.c:1036:11: error: assignment from incompatible pointer type [-Werror]
> > fmt->cmp = hist_entry__cmp_nop;
> > ^
> > builtin-diff.c:1037:16: error: assignment from incompatible pointer type [-Werror]
> > fmt->collapse = hist_entry__cmp_nop;
> > ^
> > builtin-diff.c:1043:13: error: assignment from incompatible pointer type [-Werror]
> > fmt->sort = hist_entry__cmp_baseline;
> > ^
> > builtin-diff.c:1047:13: error: assignment from incompatible pointer type [-Werror]
> > fmt->sort = hist_entry__cmp_delta;
> > ^
> > builtin-diff.c:1051:13: error: assignment from incompatible pointer type [-Werror]
> > fmt->sort = hist_entry__cmp_ratio;
> > ^
> > builtin-diff.c:1055:13: error: assignment from incompatible pointer type [-Werror]
> > fmt->sort = hist_entry__cmp_wdiff;
> > ^
> > builtin-diff.c:1058:13: error: assignment from incompatible pointer type [-Werror]
> > fmt->sort = hist_entry__cmp_nop;
> > ^
> > cc1: all warnings being treated as errors
> > LINK plugin_kvm.so
> > make[1]: *** [/tmp/build/perf/builtin-diff.o] Error 1
> > make[1]: *** Waiting for unfinished jobs....
> > LINK plugin_mac80211.so
>
> Am I missing something? Lemme continue trying applying the rest of the
> kit...
Ok, the fix came in the next patch in your kit, I fixed things up to
make it fully bisectable, this is how the next patch ended up:
commit 444002b1bb093419bfd383b20959ef6e9c0c7d3e
Author: Namhyung Kim <namhyung@kernel.org>
Date: Thu Jan 8 09:45:47 2015 +0900
perf diff: Fix output ordering to honor next column
When perf diff prints output, it sorts the entries using baseline field
by default, but entries which don't have baseline are not sorted
properly. This patch makes it sorted by values of next column.
Before:
# Baseline/0 Delta/1 Delta/2 Shared Object Symbol
# .......... ....... ....... ................. ..........................................
#
32.75% +0.28% -0.83% libc-2.20.so [.] malloc
31.50% -0.74% -0.23% libc-2.20.so [.] _int_free
22.98% +0.51% +0.52% libc-2.20.so [.] _int_malloc
5.70% +0.28% +0.30% libc-2.20.so [.] free
4.38% -0.21% +0.25% a.out [.] main
1.32% -0.15% +0.05% a.out [.] free@plt
1.31% +0.03% -0.06% a.out [.] malloc@plt
0.01% -0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
0.01% [kernel.kallsyms] [k] scheduler_tick
0.01% -0.00% [kernel.kallsyms] [k] native_read_msr_safe
+0.01% [kernel.kallsyms] [k] _raw_spin_lock_irqsave
+0.01% +0.01% [kernel.kallsyms] [k] apic_timer_interrupt
+0.01% [kernel.kallsyms] [k] intel_pstate_timer_func
+0.01% [kernel.kallsyms] [k] perf_adjust_freq_unthr_context.part.82
+0.01% [kernel.kallsyms] [k] read_tsc
+0.01% [kernel.kallsyms] [k] timekeeping_update.constprop.8
After:
# Baseline/0 Delta/1 Delta/2 Shared Object Symbol
# .......... ....... ....... ................. ..........................................
#
32.75% +0.28% -0.83% libc-2.20.so [.] malloc
31.50% -0.74% -0.23% libc-2.20.so [.] _int_free
22.98% +0.51% +0.52% libc-2.20.so [.] _int_malloc
5.70% +0.28% +0.30% libc-2.20.so [.] free
4.38% -0.21% +0.25% a.out [.] main
1.32% -0.15% +0.05% a.out [.] free@plt
1.31% +0.03% -0.06% a.out [.] malloc@plt
0.01% -0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
0.01% [kernel.kallsyms] [k] scheduler_tick
0.01% -0.00% [kernel.kallsyms] [k] native_read_msr_safe
+0.01% +0.01% [kernel.kallsyms] [k] apic_timer_interrupt
+0.01% [kernel.kallsyms] [k] read_tsc
+0.01% [kernel.kallsyms] [k] perf_adjust_freq_unthr_context.part.82
+0.01% [kernel.kallsyms] [k] intel_pstate_timer_func
+0.01% [kernel.kallsyms] [k] _raw_spin_lock_irqsave
+0.01% [kernel.kallsyms] [k] timekeeping_update.constprop.8
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1420677949-6719-7-git-send-email-namhyung@kernel.org
[ Fixed up hist_entry__cmp_ method signatures, fallout from making previous cset buildable ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 4816989a84b0..98444561d9b4 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -456,26 +456,30 @@ static void hists__precompute(struct hists *hists)
next = rb_first(root);
while (next != NULL) {
struct hist_entry *he, *pair;
+ struct data__file *d;
+ int i;
he = rb_entry(next, struct hist_entry, rb_node_in);
next = rb_next(&he->rb_node_in);
- pair = get_pair_data(he, &data__files[sort_compute]);
- if (!pair)
- continue;
+ data__for_each_file_new(i, d) {
+ pair = get_pair_data(he, d);
+ if (!pair)
+ continue;
- switch (compute) {
- case COMPUTE_DELTA:
- compute_delta(he, pair);
- break;
- case COMPUTE_RATIO:
- compute_ratio(he, pair);
- break;
- case COMPUTE_WEIGHTED_DIFF:
- compute_wdiff(he, pair);
- break;
- default:
- BUG_ON(1);
+ switch (compute) {
+ case COMPUTE_DELTA:
+ compute_delta(he, pair);
+ break;
+ case COMPUTE_RATIO:
+ compute_ratio(he, pair);
+ break;
+ case COMPUTE_WEIGHTED_DIFF:
+ compute_wdiff(he, pair);
+ break;
+ default:
+ BUG_ON(1);
+ }
}
}
}
@@ -525,7 +529,7 @@ __hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
static int64_t
hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
- int c)
+ int c, int sort_idx)
{
bool pairs_left = hist_entry__has_pairs(left);
bool pairs_right = hist_entry__has_pairs(right);
@@ -537,8 +541,8 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
if (!pairs_left || !pairs_right)
return pairs_left ? -1 : 1;
- p_left = get_pair_data(left, &data__files[sort_compute]);
- p_right = get_pair_data(right, &data__files[sort_compute]);
+ p_left = get_pair_data(left, &data__files[sort_idx]);
+ p_right = get_pair_data(right, &data__files[sort_idx]);
if (!p_left && !p_right)
return 0;
@@ -565,33 +569,36 @@ static int64_t
hist_entry__cmp_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
struct hist_entry *left, struct hist_entry *right)
{
- if (sort_compute)
- return 0;
-
if (left->stat.period == right->stat.period)
return 0;
return left->stat.period > right->stat.period ? 1 : -1;
}
static int64_t
-hist_entry__cmp_delta(struct perf_hpp_fmt *fmt __maybe_unused,
+hist_entry__cmp_delta(struct perf_hpp_fmt *fmt,
struct hist_entry *left, struct hist_entry *right)
{
- return hist_entry__cmp_compute(right, left, COMPUTE_DELTA);
+ struct data__file *d = fmt_to_data_file(fmt);
+
+ return hist_entry__cmp_compute(right, left, COMPUTE_DELTA, d->idx);
}
static int64_t
-hist_entry__cmp_ratio(struct perf_hpp_fmt *fmt __maybe_unused,
+hist_entry__cmp_ratio(struct perf_hpp_fmt *fmt,
struct hist_entry *left, struct hist_entry *right)
{
- return hist_entry__cmp_compute(right, left, COMPUTE_RATIO);
+ struct data__file *d = fmt_to_data_file(fmt);
+
+ return hist_entry__cmp_compute(right, left, COMPUTE_RATIO, d->idx);
}
static int64_t
-hist_entry__cmp_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
+hist_entry__cmp_wdiff(struct perf_hpp_fmt *fmt,
struct hist_entry *left, struct hist_entry *right)
{
- return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF);
+ struct data__file *d = fmt_to_data_file(fmt);
+
+ return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF, d->idx);
}
static void hists__process(struct hists *hists)
@@ -599,9 +606,7 @@ static void hists__process(struct hists *hists)
if (show_baseline_only)
hists__baseline_only(hists);
- if (sort_compute)
- hists__precompute(hists);
-
+ hists__precompute(hists);
hists__output_resort(hists, NULL);
hists__fprintf(hists, true, 0, 0, 0, stdout);
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks
2015-01-12 16:27 ` Arnaldo Carvalho de Melo
2015-01-12 16:39 ` Arnaldo Carvalho de Melo
@ 2015-01-12 16:40 ` Jiri Olsa
2015-01-12 16:44 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 20+ messages in thread
From: Jiri Olsa @ 2015-01-12 16:40 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Namhyung Kim, Ingo Molnar, Peter Zijlstra, LKML, Kan Liang,
Jiri Olsa
On Mon, Jan 12, 2015 at 01:27:36PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jan 12, 2015 at 01:21:40PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jan 08, 2015 at 09:45:46AM +0900, Namhyung Kim escreveu:
> > > Currently ->cmp, ->collapse and ->sort callbacks doesn't pass
> > > corresponding fmt. But it'll be needed by upcoming changes in
> > > perf diff command.
> > >
> > > Suggested-by: Jiri Olsa <jolsa@kernel.org>
> >
> >
> > CC /tmp/build/perf/builtin-evlist.o
> > builtin-diff.c: In function ‘data__hpp_register’:
> > builtin-diff.c:1036:11: error: assignment from incompatible pointer type [-Werror]
> > fmt->cmp = hist_entry__cmp_nop;
> > ^
> > builtin-diff.c:1037:16: error: assignment from incompatible pointer type [-Werror]
> > fmt->collapse = hist_entry__cmp_nop;
> > ^
> > builtin-diff.c:1043:13: error: assignment from incompatible pointer type [-Werror]
> > fmt->sort = hist_entry__cmp_baseline;
> > ^
> > builtin-diff.c:1047:13: error: assignment from incompatible pointer type [-Werror]
> > fmt->sort = hist_entry__cmp_delta;
> > ^
> > builtin-diff.c:1051:13: error: assignment from incompatible pointer type [-Werror]
> > fmt->sort = hist_entry__cmp_ratio;
> > ^
> > builtin-diff.c:1055:13: error: assignment from incompatible pointer type [-Werror]
> > fmt->sort = hist_entry__cmp_wdiff;
> > ^
> > builtin-diff.c:1058:13: error: assignment from incompatible pointer type [-Werror]
> > fmt->sort = hist_entry__cmp_nop;
> > ^
> > cc1: all warnings being treated as errors
> > LINK plugin_kvm.so
> > make[1]: *** [/tmp/build/perf/builtin-diff.o] Error 1
> > make[1]: *** Waiting for unfinished jobs....
> > LINK plugin_mac80211.so
>
> Am I missing something? Lemme continue trying applying the rest of the
> kit...
hum,
looks like those (= hist_entry__cmp_delta,.. functions) are fixed (=added fmt param)
later in patch 6/7 of this series.. which is wrong, should be updated in this patch,
as you did below
jirka
>
> - Arnaldo
>
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index ae8f62151b34..4816989a84b0 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -554,14 +554,16 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
> }
>
> static int64_t
> -hist_entry__cmp_nop(struct hist_entry *left __maybe_unused,
> +hist_entry__cmp_nop(struct perf_hpp_fmt *fmt __maybe_unused,
> + struct hist_entry *left __maybe_unused,
> struct hist_entry *right __maybe_unused)
> {
> return 0;
> }
>
> static int64_t
> -hist_entry__cmp_baseline(struct hist_entry *left, struct hist_entry *right)
> +hist_entry__cmp_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
> + struct hist_entry *left, struct hist_entry *right)
> {
> if (sort_compute)
> return 0;
> @@ -572,19 +574,22 @@ hist_entry__cmp_baseline(struct hist_entry *left, struct hist_entry *right)
> }
>
> static int64_t
> -hist_entry__cmp_delta(struct hist_entry *left, struct hist_entry *right)
> +hist_entry__cmp_delta(struct perf_hpp_fmt *fmt __maybe_unused,
> + struct hist_entry *left, struct hist_entry *right)
> {
> return hist_entry__cmp_compute(right, left, COMPUTE_DELTA);
> }
>
> static int64_t
> -hist_entry__cmp_ratio(struct hist_entry *left, struct hist_entry *right)
> +hist_entry__cmp_ratio(struct perf_hpp_fmt *fmt __maybe_unused,
> + struct hist_entry *left, struct hist_entry *right)
> {
> return hist_entry__cmp_compute(right, left, COMPUTE_RATIO);
> }
>
> static int64_t
> -hist_entry__cmp_wdiff(struct hist_entry *left, struct hist_entry *right)
> +hist_entry__cmp_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
> + struct hist_entry *left, struct hist_entry *right)
> {
> return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF);
> }
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks
2015-01-12 16:40 ` Jiri Olsa
@ 2015-01-12 16:44 ` Arnaldo Carvalho de Melo
2015-01-14 1:35 ` Namhyung Kim
0 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-12 16:44 UTC (permalink / raw)
To: Jiri Olsa
Cc: Namhyung Kim, Ingo Molnar, Peter Zijlstra, LKML, Kan Liang,
Jiri Olsa
Em Mon, Jan 12, 2015 at 05:40:50PM +0100, Jiri Olsa escreveu:
> On Mon, Jan 12, 2015 at 01:27:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Jan 12, 2015 at 01:21:40PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Thu, Jan 08, 2015 at 09:45:46AM +0900, Namhyung Kim escreveu:
> > > > Currently ->cmp, ->collapse and ->sort callbacks doesn't pass
> > > > corresponding fmt. But it'll be needed by upcoming changes in
> > > > perf diff command.
> > > >
> > > > Suggested-by: Jiri Olsa <jolsa@kernel.org>
> > >
> > >
> > > CC /tmp/build/perf/builtin-evlist.o
> > > builtin-diff.c: In function ‘data__hpp_register’:
> > > builtin-diff.c:1036:11: error: assignment from incompatible pointer type [-Werror]
> > > fmt->cmp = hist_entry__cmp_nop;
> > > ^
> > > builtin-diff.c:1037:16: error: assignment from incompatible pointer type [-Werror]
> > > fmt->collapse = hist_entry__cmp_nop;
> > > ^
> > > builtin-diff.c:1043:13: error: assignment from incompatible pointer type [-Werror]
> > > fmt->sort = hist_entry__cmp_baseline;
> > > ^
> > > builtin-diff.c:1047:13: error: assignment from incompatible pointer type [-Werror]
> > > fmt->sort = hist_entry__cmp_delta;
> > > ^
> > > builtin-diff.c:1051:13: error: assignment from incompatible pointer type [-Werror]
> > > fmt->sort = hist_entry__cmp_ratio;
> > > ^
> > > builtin-diff.c:1055:13: error: assignment from incompatible pointer type [-Werror]
> > > fmt->sort = hist_entry__cmp_wdiff;
> > > ^
> > > builtin-diff.c:1058:13: error: assignment from incompatible pointer type [-Werror]
> > > fmt->sort = hist_entry__cmp_nop;
> > > ^
> > > cc1: all warnings being treated as errors
> > > LINK plugin_kvm.so
> > > make[1]: *** [/tmp/build/perf/builtin-diff.o] Error 1
> > > make[1]: *** Waiting for unfinished jobs....
> > > LINK plugin_mac80211.so
> >
> > Am I missing something? Lemme continue trying applying the rest of the
> > kit...
>
> hum,
> looks like those (= hist_entry__cmp_delta,.. functions) are fixed (=added fmt param)
> later in patch 6/7 of this series.. which is wrong, should be updated in this patch,
> as you did below
I have it all fixed up and pushed to my perf/core branch, thanks for
checking!
- Arnaldo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks
2015-01-12 16:44 ` Arnaldo Carvalho de Melo
@ 2015-01-14 1:35 ` Namhyung Kim
0 siblings, 0 replies; 20+ messages in thread
From: Namhyung Kim @ 2015-01-14 1:35 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Ingo Molnar, Peter Zijlstra, LKML, Kan Liang,
Jiri Olsa
Hi Arnaldo and Jiri,
On Mon, Jan 12, 2015 at 01:44:06PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jan 12, 2015 at 05:40:50PM +0100, Jiri Olsa escreveu:
> > On Mon, Jan 12, 2015 at 01:27:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Mon, Jan 12, 2015 at 01:21:40PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Thu, Jan 08, 2015 at 09:45:46AM +0900, Namhyung Kim escreveu:
> > > > > Currently ->cmp, ->collapse and ->sort callbacks doesn't pass
> > > > > corresponding fmt. But it'll be needed by upcoming changes in
> > > > > perf diff command.
> > > > >
> > > > > Suggested-by: Jiri Olsa <jolsa@kernel.org>
> > > >
> > > >
> > > > CC /tmp/build/perf/builtin-evlist.o
> > > > builtin-diff.c: In function ‘data__hpp_register’:
> > > > builtin-diff.c:1036:11: error: assignment from incompatible pointer type [-Werror]
> > > > fmt->cmp = hist_entry__cmp_nop;
> > > > ^
> > > > builtin-diff.c:1037:16: error: assignment from incompatible pointer type [-Werror]
> > > > fmt->collapse = hist_entry__cmp_nop;
> > > > ^
> > > > builtin-diff.c:1043:13: error: assignment from incompatible pointer type [-Werror]
> > > > fmt->sort = hist_entry__cmp_baseline;
> > > > ^
> > > > builtin-diff.c:1047:13: error: assignment from incompatible pointer type [-Werror]
> > > > fmt->sort = hist_entry__cmp_delta;
> > > > ^
> > > > builtin-diff.c:1051:13: error: assignment from incompatible pointer type [-Werror]
> > > > fmt->sort = hist_entry__cmp_ratio;
> > > > ^
> > > > builtin-diff.c:1055:13: error: assignment from incompatible pointer type [-Werror]
> > > > fmt->sort = hist_entry__cmp_wdiff;
> > > > ^
> > > > builtin-diff.c:1058:13: error: assignment from incompatible pointer type [-Werror]
> > > > fmt->sort = hist_entry__cmp_nop;
> > > > ^
> > > > cc1: all warnings being treated as errors
> > > > LINK plugin_kvm.so
> > > > make[1]: *** [/tmp/build/perf/builtin-diff.o] Error 1
> > > > make[1]: *** Waiting for unfinished jobs....
> > > > LINK plugin_mac80211.so
> > >
> > > Am I missing something? Lemme continue trying applying the rest of the
> > > kit...
> >
> > hum,
> > looks like those (= hist_entry__cmp_delta,.. functions) are fixed (=added fmt param)
> > later in patch 6/7 of this series.. which is wrong, should be updated in this patch,
> > as you did below
>
> I have it all fixed up and pushed to my perf/core branch, thanks for
> checking!
Oops, my bad. It must came from the many of rebases during the
development. Sorry about that and really thank you to fix them up.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 20+ messages in thread
* [tip:perf/core] perf tools: Pass struct perf_hpp_fmt to its callbacks
2015-01-08 0:45 ` [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks Namhyung Kim
2015-01-12 16:21 ` Arnaldo Carvalho de Melo
@ 2015-01-28 15:07 ` tip-bot for Namhyung Kim
1 sibling, 0 replies; 20+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-01-28 15:07 UTC (permalink / raw)
To: linux-tip-commits
Cc: namhyung, jolsa, a.p.zijlstra, acme, hpa, mingo, tglx, kan.liang,
linux-kernel, jolsa
Commit-ID: 87bbdf768ff962f1c04d3b8f6db1e179279132d1
Gitweb: http://git.kernel.org/tip/87bbdf768ff962f1c04d3b8f6db1e179279132d1
Author: Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 8 Jan 2015 09:45:46 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 21 Jan 2015 13:24:34 -0300
perf tools: Pass struct perf_hpp_fmt to its callbacks
Currently ->cmp, ->collapse and ->sort callbacks doesn't pass
corresponding fmt. But it'll be needed by upcoming changes in
perf diff command.
Suggested-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1420677949-6719-6-git-send-email-namhyung@kernel.org
[ fix build by passing perf_hpp_fmt pointer to hist_entry__cmp_ methods ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-diff.c | 15 ++++++++++-----
tools/perf/ui/hist.c | 12 ++++++++----
tools/perf/util/hist.c | 6 +++---
tools/perf/util/hist.h | 9 ++++++---
tools/perf/util/sort.c | 37 ++++++++++++++++++++++++++++++++++---
5 files changed, 61 insertions(+), 18 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index ae8f621..4816989 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -554,14 +554,16 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
}
static int64_t
-hist_entry__cmp_nop(struct hist_entry *left __maybe_unused,
+hist_entry__cmp_nop(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left __maybe_unused,
struct hist_entry *right __maybe_unused)
{
return 0;
}
static int64_t
-hist_entry__cmp_baseline(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
{
if (sort_compute)
return 0;
@@ -572,19 +574,22 @@ hist_entry__cmp_baseline(struct hist_entry *left, struct hist_entry *right)
}
static int64_t
-hist_entry__cmp_delta(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_delta(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
{
return hist_entry__cmp_compute(right, left, COMPUTE_DELTA);
}
static int64_t
-hist_entry__cmp_ratio(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_ratio(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
{
return hist_entry__cmp_compute(right, left, COMPUTE_RATIO);
}
static int64_t
-hist_entry__cmp_wdiff(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
{
return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF);
}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 482adae..25d6083 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -285,7 +285,8 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *fmt, \
}
#define __HPP_SORT_FN(_type, _field) \
-static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b) \
+static int64_t hpp__sort_##_type(struct perf_hpp_fmt *fmt __maybe_unused, \
+ struct hist_entry *a, struct hist_entry *b) \
{ \
return __hpp__sort(a, b, he_get_##_field); \
}
@@ -312,7 +313,8 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *fmt, \
}
#define __HPP_SORT_ACC_FN(_type, _field) \
-static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b) \
+static int64_t hpp__sort_##_type(struct perf_hpp_fmt *fmt __maybe_unused, \
+ struct hist_entry *a, struct hist_entry *b) \
{ \
return __hpp__sort_acc(a, b, he_get_acc_##_field); \
}
@@ -331,7 +333,8 @@ static int hpp__entry_##_type(struct perf_hpp_fmt *fmt, \
}
#define __HPP_SORT_RAW_FN(_type, _field) \
-static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b) \
+static int64_t hpp__sort_##_type(struct perf_hpp_fmt *fmt __maybe_unused, \
+ struct hist_entry *a, struct hist_entry *b) \
{ \
return __hpp__sort(a, b, he_get_raw_##_field); \
}
@@ -361,7 +364,8 @@ HPP_PERCENT_ACC_FNS(overhead_acc, period)
HPP_RAW_FNS(samples, nr_events)
HPP_RAW_FNS(period, period)
-static int64_t hpp__nop_cmp(struct hist_entry *a __maybe_unused,
+static int64_t hpp__nop_cmp(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *a __maybe_unused,
struct hist_entry *b __maybe_unused)
{
return 0;
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index e17163f..70b48a6 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -913,7 +913,7 @@ hist_entry__cmp(struct hist_entry *left, struct hist_entry *right)
if (perf_hpp__should_skip(fmt))
continue;
- cmp = fmt->cmp(left, right);
+ cmp = fmt->cmp(fmt, left, right);
if (cmp)
break;
}
@@ -931,7 +931,7 @@ hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)
if (perf_hpp__should_skip(fmt))
continue;
- cmp = fmt->collapse(left, right);
+ cmp = fmt->collapse(fmt, left, right);
if (cmp)
break;
}
@@ -1061,7 +1061,7 @@ static int hist_entry__sort(struct hist_entry *a, struct hist_entry *b)
if (perf_hpp__should_skip(fmt))
continue;
- cmp = fmt->sort(a, b);
+ cmp = fmt->sort(fmt, a, b);
if (cmp)
break;
}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 9305580..2b690d0 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -195,9 +195,12 @@ struct perf_hpp_fmt {
struct hist_entry *he);
int (*entry)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
struct hist_entry *he);
- int64_t (*cmp)(struct hist_entry *a, struct hist_entry *b);
- int64_t (*collapse)(struct hist_entry *a, struct hist_entry *b);
- int64_t (*sort)(struct hist_entry *a, struct hist_entry *b);
+ int64_t (*cmp)(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b);
+ int64_t (*collapse)(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b);
+ int64_t (*sort)(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b);
struct list_head list;
struct list_head sort_list;
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 9139dda..7a39c1e 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1304,6 +1304,37 @@ static int __sort__hpp_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
return hse->se->se_snprintf(he, hpp->buf, hpp->size, len);
}
+static int64_t __sort__hpp_cmp(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b)
+{
+ struct hpp_sort_entry *hse;
+
+ hse = container_of(fmt, struct hpp_sort_entry, hpp);
+ return hse->se->se_cmp(a, b);
+}
+
+static int64_t __sort__hpp_collapse(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b)
+{
+ struct hpp_sort_entry *hse;
+ int64_t (*collapse_fn)(struct hist_entry *, struct hist_entry *);
+
+ hse = container_of(fmt, struct hpp_sort_entry, hpp);
+ collapse_fn = hse->se->se_collapse ?: hse->se->se_cmp;
+ return collapse_fn(a, b);
+}
+
+static int64_t __sort__hpp_sort(struct perf_hpp_fmt *fmt,
+ struct hist_entry *a, struct hist_entry *b)
+{
+ struct hpp_sort_entry *hse;
+ int64_t (*sort_fn)(struct hist_entry *, struct hist_entry *);
+
+ hse = container_of(fmt, struct hpp_sort_entry, hpp);
+ sort_fn = hse->se->se_sort ?: hse->se->se_cmp;
+ return sort_fn(a, b);
+}
+
static struct hpp_sort_entry *
__sort_dimension__alloc_hpp(struct sort_dimension *sd)
{
@@ -1322,9 +1353,9 @@ __sort_dimension__alloc_hpp(struct sort_dimension *sd)
hse->hpp.entry = __sort__hpp_entry;
hse->hpp.color = NULL;
- hse->hpp.cmp = sd->entry->se_cmp;
- hse->hpp.collapse = sd->entry->se_collapse ? : sd->entry->se_cmp;
- hse->hpp.sort = sd->entry->se_sort ? : hse->hpp.collapse;
+ hse->hpp.cmp = __sort__hpp_cmp;
+ hse->hpp.collapse = __sort__hpp_collapse;
+ hse->hpp.sort = __sort__hpp_sort;
INIT_LIST_HEAD(&hse->hpp.list);
INIT_LIST_HEAD(&hse->hpp.sort_list);
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 6/7] perf diff: Fix output ordering to honor next column
2015-01-08 0:45 [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Namhyung Kim
` (4 preceding siblings ...)
2015-01-08 0:45 ` [PATCH 5/7] perf tools: Pass struct perf_hpp_fmt to its callbacks Namhyung Kim
@ 2015-01-08 0:45 ` Namhyung Kim
2015-01-28 15:08 ` [tip:perf/core] " tip-bot for Namhyung Kim
2015-01-08 0:45 ` [PATCH 7/7] perf diff: Fix -o/--order option behavior Namhyung Kim
` (2 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2015-01-08 0:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang
When perf diff prints output, it sorts the entries using baseline field
by default, but entries which don't have baseline are not sorted
properly. This patch makes it sorted by values of next column.
Before:
# Baseline/0 Delta/1 Delta/2 Shared Object Symbol
# .......... ....... ....... ................. ..........................................
#
32.75% +0.28% -0.83% libc-2.20.so [.] malloc
31.50% -0.74% -0.23% libc-2.20.so [.] _int_free
22.98% +0.51% +0.52% libc-2.20.so [.] _int_malloc
5.70% +0.28% +0.30% libc-2.20.so [.] free
4.38% -0.21% +0.25% a.out [.] main
1.32% -0.15% +0.05% a.out [.] free@plt
1.31% +0.03% -0.06% a.out [.] malloc@plt
0.01% -0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
0.01% [kernel.kallsyms] [k] scheduler_tick
0.01% -0.00% [kernel.kallsyms] [k] native_read_msr_safe
+0.01% [kernel.kallsyms] [k] _raw_spin_lock_irqsave
+0.01% +0.01% [kernel.kallsyms] [k] apic_timer_interrupt
+0.01% [kernel.kallsyms] [k] intel_pstate_timer_func
+0.01% [kernel.kallsyms] [k] perf_adjust_freq_unthr_context.part.82
+0.01% [kernel.kallsyms] [k] read_tsc
+0.01% [kernel.kallsyms] [k] timekeeping_update.constprop.8
After:
# Baseline/0 Delta/1 Delta/2 Shared Object Symbol
# .......... ....... ....... ................. ..........................................
#
32.75% +0.28% -0.83% libc-2.20.so [.] malloc
31.50% -0.74% -0.23% libc-2.20.so [.] _int_free
22.98% +0.51% +0.52% libc-2.20.so [.] _int_malloc
5.70% +0.28% +0.30% libc-2.20.so [.] free
4.38% -0.21% +0.25% a.out [.] main
1.32% -0.15% +0.05% a.out [.] free@plt
1.31% +0.03% -0.06% a.out [.] malloc@plt
0.01% -0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
0.01% [kernel.kallsyms] [k] scheduler_tick
0.01% -0.00% [kernel.kallsyms] [k] native_read_msr_safe
+0.01% +0.01% [kernel.kallsyms] [k] apic_timer_interrupt
+0.01% [kernel.kallsyms] [k] read_tsc
+0.01% [kernel.kallsyms] [k] perf_adjust_freq_unthr_context.part.82
+0.01% [kernel.kallsyms] [k] intel_pstate_timer_func
+0.01% [kernel.kallsyms] [k] _raw_spin_lock_irqsave
+0.01% [kernel.kallsyms] [k] timekeeping_update.constprop.8
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 74 +++++++++++++++++++++++++++--------------------
1 file changed, 42 insertions(+), 32 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index ae8f62151b34..98444561d9b4 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -456,26 +456,30 @@ static void hists__precompute(struct hists *hists)
next = rb_first(root);
while (next != NULL) {
struct hist_entry *he, *pair;
+ struct data__file *d;
+ int i;
he = rb_entry(next, struct hist_entry, rb_node_in);
next = rb_next(&he->rb_node_in);
- pair = get_pair_data(he, &data__files[sort_compute]);
- if (!pair)
- continue;
+ data__for_each_file_new(i, d) {
+ pair = get_pair_data(he, d);
+ if (!pair)
+ continue;
- switch (compute) {
- case COMPUTE_DELTA:
- compute_delta(he, pair);
- break;
- case COMPUTE_RATIO:
- compute_ratio(he, pair);
- break;
- case COMPUTE_WEIGHTED_DIFF:
- compute_wdiff(he, pair);
- break;
- default:
- BUG_ON(1);
+ switch (compute) {
+ case COMPUTE_DELTA:
+ compute_delta(he, pair);
+ break;
+ case COMPUTE_RATIO:
+ compute_ratio(he, pair);
+ break;
+ case COMPUTE_WEIGHTED_DIFF:
+ compute_wdiff(he, pair);
+ break;
+ default:
+ BUG_ON(1);
+ }
}
}
}
@@ -525,7 +529,7 @@ __hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
static int64_t
hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
- int c)
+ int c, int sort_idx)
{
bool pairs_left = hist_entry__has_pairs(left);
bool pairs_right = hist_entry__has_pairs(right);
@@ -537,8 +541,8 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
if (!pairs_left || !pairs_right)
return pairs_left ? -1 : 1;
- p_left = get_pair_data(left, &data__files[sort_compute]);
- p_right = get_pair_data(right, &data__files[sort_compute]);
+ p_left = get_pair_data(left, &data__files[sort_idx]);
+ p_right = get_pair_data(right, &data__files[sort_idx]);
if (!p_left && !p_right)
return 0;
@@ -554,39 +558,47 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
}
static int64_t
-hist_entry__cmp_nop(struct hist_entry *left __maybe_unused,
+hist_entry__cmp_nop(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left __maybe_unused,
struct hist_entry *right __maybe_unused)
{
return 0;
}
static int64_t
-hist_entry__cmp_baseline(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
{
- if (sort_compute)
- return 0;
-
if (left->stat.period == right->stat.period)
return 0;
return left->stat.period > right->stat.period ? 1 : -1;
}
static int64_t
-hist_entry__cmp_delta(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_delta(struct perf_hpp_fmt *fmt,
+ struct hist_entry *left, struct hist_entry *right)
{
- return hist_entry__cmp_compute(right, left, COMPUTE_DELTA);
+ struct data__file *d = fmt_to_data_file(fmt);
+
+ return hist_entry__cmp_compute(right, left, COMPUTE_DELTA, d->idx);
}
static int64_t
-hist_entry__cmp_ratio(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_ratio(struct perf_hpp_fmt *fmt,
+ struct hist_entry *left, struct hist_entry *right)
{
- return hist_entry__cmp_compute(right, left, COMPUTE_RATIO);
+ struct data__file *d = fmt_to_data_file(fmt);
+
+ return hist_entry__cmp_compute(right, left, COMPUTE_RATIO, d->idx);
}
static int64_t
-hist_entry__cmp_wdiff(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp_wdiff(struct perf_hpp_fmt *fmt,
+ struct hist_entry *left, struct hist_entry *right)
{
- return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF);
+ struct data__file *d = fmt_to_data_file(fmt);
+
+ return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF, d->idx);
}
static void hists__process(struct hists *hists)
@@ -594,9 +606,7 @@ static void hists__process(struct hists *hists)
if (show_baseline_only)
hists__baseline_only(hists);
- if (sort_compute)
- hists__precompute(hists);
-
+ hists__precompute(hists);
hists__output_resort(hists, NULL);
hists__fprintf(hists, true, 0, 0, 0, stdout);
--
2.1.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [tip:perf/core] perf diff: Fix output ordering to honor next column
2015-01-08 0:45 ` [PATCH 6/7] perf diff: Fix output ordering to honor next column Namhyung Kim
@ 2015-01-28 15:08 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-01-28 15:08 UTC (permalink / raw)
To: linux-tip-commits
Cc: hpa, a.p.zijlstra, jolsa, acme, tglx, linux-kernel, jolsa, mingo,
namhyung, kan.liang
Commit-ID: 56495a8affabe35aa0d94aae050d3e0e60d0455f
Gitweb: http://git.kernel.org/tip/56495a8affabe35aa0d94aae050d3e0e60d0455f
Author: Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 8 Jan 2015 09:45:47 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 21 Jan 2015 13:24:34 -0300
perf diff: Fix output ordering to honor next column
When perf diff prints output, it sorts the entries using baseline field
by default, but entries which don't have baseline are not sorted
properly. This patch makes it sorted by values of next column.
Before:
# Baseline/0 Delta/1 Delta/2 Shared Object Symbol
# .......... ....... ....... ................. ..........................................
#
32.75% +0.28% -0.83% libc-2.20.so [.] malloc
31.50% -0.74% -0.23% libc-2.20.so [.] _int_free
22.98% +0.51% +0.52% libc-2.20.so [.] _int_malloc
5.70% +0.28% +0.30% libc-2.20.so [.] free
4.38% -0.21% +0.25% a.out [.] main
1.32% -0.15% +0.05% a.out [.] free@plt
1.31% +0.03% -0.06% a.out [.] malloc@plt
0.01% -0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
0.01% [kernel.kallsyms] [k] scheduler_tick
0.01% -0.00% [kernel.kallsyms] [k] native_read_msr_safe
+0.01% [kernel.kallsyms] [k] _raw_spin_lock_irqsave
+0.01% +0.01% [kernel.kallsyms] [k] apic_timer_interrupt
+0.01% [kernel.kallsyms] [k] intel_pstate_timer_func
+0.01% [kernel.kallsyms] [k] perf_adjust_freq_unthr_context.part.82
+0.01% [kernel.kallsyms] [k] read_tsc
+0.01% [kernel.kallsyms] [k] timekeeping_update.constprop.8
After:
# Baseline/0 Delta/1 Delta/2 Shared Object Symbol
# .......... ....... ....... ................. ..........................................
#
32.75% +0.28% -0.83% libc-2.20.so [.] malloc
31.50% -0.74% -0.23% libc-2.20.so [.] _int_free
22.98% +0.51% +0.52% libc-2.20.so [.] _int_malloc
5.70% +0.28% +0.30% libc-2.20.so [.] free
4.38% -0.21% +0.25% a.out [.] main
1.32% -0.15% +0.05% a.out [.] free@plt
1.31% +0.03% -0.06% a.out [.] malloc@plt
0.01% -0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
0.01% [kernel.kallsyms] [k] scheduler_tick
0.01% -0.00% [kernel.kallsyms] [k] native_read_msr_safe
+0.01% +0.01% [kernel.kallsyms] [k] apic_timer_interrupt
+0.01% [kernel.kallsyms] [k] read_tsc
+0.01% [kernel.kallsyms] [k] perf_adjust_freq_unthr_context.part.82
+0.01% [kernel.kallsyms] [k] intel_pstate_timer_func
+0.01% [kernel.kallsyms] [k] _raw_spin_lock_irqsave
+0.01% [kernel.kallsyms] [k] timekeeping_update.constprop.8
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1420677949-6719-7-git-send-email-namhyung@kernel.org
[ Fixed up hist_entry__cmp_ method signatures, fallout from making previous cset buildable ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-diff.c | 65 +++++++++++++++++++++++++----------------------
1 file changed, 35 insertions(+), 30 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 4816989..9844456 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -456,26 +456,30 @@ static void hists__precompute(struct hists *hists)
next = rb_first(root);
while (next != NULL) {
struct hist_entry *he, *pair;
+ struct data__file *d;
+ int i;
he = rb_entry(next, struct hist_entry, rb_node_in);
next = rb_next(&he->rb_node_in);
- pair = get_pair_data(he, &data__files[sort_compute]);
- if (!pair)
- continue;
+ data__for_each_file_new(i, d) {
+ pair = get_pair_data(he, d);
+ if (!pair)
+ continue;
- switch (compute) {
- case COMPUTE_DELTA:
- compute_delta(he, pair);
- break;
- case COMPUTE_RATIO:
- compute_ratio(he, pair);
- break;
- case COMPUTE_WEIGHTED_DIFF:
- compute_wdiff(he, pair);
- break;
- default:
- BUG_ON(1);
+ switch (compute) {
+ case COMPUTE_DELTA:
+ compute_delta(he, pair);
+ break;
+ case COMPUTE_RATIO:
+ compute_ratio(he, pair);
+ break;
+ case COMPUTE_WEIGHTED_DIFF:
+ compute_wdiff(he, pair);
+ break;
+ default:
+ BUG_ON(1);
+ }
}
}
}
@@ -525,7 +529,7 @@ __hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
static int64_t
hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
- int c)
+ int c, int sort_idx)
{
bool pairs_left = hist_entry__has_pairs(left);
bool pairs_right = hist_entry__has_pairs(right);
@@ -537,8 +541,8 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
if (!pairs_left || !pairs_right)
return pairs_left ? -1 : 1;
- p_left = get_pair_data(left, &data__files[sort_compute]);
- p_right = get_pair_data(right, &data__files[sort_compute]);
+ p_left = get_pair_data(left, &data__files[sort_idx]);
+ p_right = get_pair_data(right, &data__files[sort_idx]);
if (!p_left && !p_right)
return 0;
@@ -565,33 +569,36 @@ static int64_t
hist_entry__cmp_baseline(struct perf_hpp_fmt *fmt __maybe_unused,
struct hist_entry *left, struct hist_entry *right)
{
- if (sort_compute)
- return 0;
-
if (left->stat.period == right->stat.period)
return 0;
return left->stat.period > right->stat.period ? 1 : -1;
}
static int64_t
-hist_entry__cmp_delta(struct perf_hpp_fmt *fmt __maybe_unused,
+hist_entry__cmp_delta(struct perf_hpp_fmt *fmt,
struct hist_entry *left, struct hist_entry *right)
{
- return hist_entry__cmp_compute(right, left, COMPUTE_DELTA);
+ struct data__file *d = fmt_to_data_file(fmt);
+
+ return hist_entry__cmp_compute(right, left, COMPUTE_DELTA, d->idx);
}
static int64_t
-hist_entry__cmp_ratio(struct perf_hpp_fmt *fmt __maybe_unused,
+hist_entry__cmp_ratio(struct perf_hpp_fmt *fmt,
struct hist_entry *left, struct hist_entry *right)
{
- return hist_entry__cmp_compute(right, left, COMPUTE_RATIO);
+ struct data__file *d = fmt_to_data_file(fmt);
+
+ return hist_entry__cmp_compute(right, left, COMPUTE_RATIO, d->idx);
}
static int64_t
-hist_entry__cmp_wdiff(struct perf_hpp_fmt *fmt __maybe_unused,
+hist_entry__cmp_wdiff(struct perf_hpp_fmt *fmt,
struct hist_entry *left, struct hist_entry *right)
{
- return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF);
+ struct data__file *d = fmt_to_data_file(fmt);
+
+ return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF, d->idx);
}
static void hists__process(struct hists *hists)
@@ -599,9 +606,7 @@ static void hists__process(struct hists *hists)
if (show_baseline_only)
hists__baseline_only(hists);
- if (sort_compute)
- hists__precompute(hists);
-
+ hists__precompute(hists);
hists__output_resort(hists, NULL);
hists__fprintf(hists, true, 0, 0, 0, stdout);
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 7/7] perf diff: Fix -o/--order option behavior
2015-01-08 0:45 [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Namhyung Kim
` (5 preceding siblings ...)
2015-01-08 0:45 ` [PATCH 6/7] perf diff: Fix output ordering to honor next column Namhyung Kim
@ 2015-01-08 0:45 ` Namhyung Kim
2015-01-28 15:08 ` [tip:perf/core] " tip-bot for Namhyung Kim
2015-01-08 14:42 ` [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Jiri Olsa
2015-01-12 16:40 ` Arnaldo Carvalho de Melo
8 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2015-01-08 0:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang
The prior change fixes default output ordering with each column but it
breaks -o/--order option. This patch prepends a new hpp fmt struct to
sort list but not to output field list so that it can affect ordering
without adding a new output column.
The new hpp fmt uses its own compare functions which treats dummy
entries (which have no baseline) little differently - the delta field
can be computed without baseline but others (ratio and wdiff) are not.
The new output will look like below:
$ perf diff -o 2 perf.data.{old,cur,new}
...
# Baseline/0 Delta/1 Delta/2 Shared Object Symbol
# .......... ....... ....... ................. ..........................................
22.98% +0.51% +0.52% libc-2.20.so [.] _int_malloc
5.70% +0.28% +0.30% libc-2.20.so [.] free
4.38% -0.21% +0.25% a.out [.] main
1.32% -0.15% +0.05% a.out [.] free@plt
+0.01% [kernel.kallsyms] [k] intel_pstate_timer_func
+0.01% [kernel.kallsyms] [k] _raw_spin_lock_irqsave
+0.01% [kernel.kallsyms] [k] timekeeping_update.constprop.8
+0.01% +0.01% [kernel.kallsyms] [k] apic_timer_interrupt
0.01% -0.00% [kernel.kallsyms] [k] native_read_msr_safe
0.01% -0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
1.31% +0.03% -0.06% a.out [.] malloc@plt
31.50% -0.74% -0.23% libc-2.20.so [.] _int_free
32.75% +0.28% -0.83% libc-2.20.so [.] malloc
0.01% [kernel.kallsyms] [k] scheduler_tick
+0.01% [kernel.kallsyms] [k] read_tsc
+0.01% [kernel.kallsyms] [k] perf_adjust_freq_unthr_context.part.82
In above example, the output was sorted by 'Delta/2' column first, and
then 'Baseline/0' and finally 'Delta/1'.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-diff.c | 101 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 99 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 98444561d9b4..74aada554b12 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -558,6 +558,37 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
}
static int64_t
+hist_entry__cmp_compute_idx(struct hist_entry *left, struct hist_entry *right,
+ int c, int sort_idx)
+{
+ struct hist_entry *p_right, *p_left;
+
+ p_left = get_pair_data(left, &data__files[sort_idx]);
+ p_right = get_pair_data(right, &data__files[sort_idx]);
+
+ if (!p_left && !p_right)
+ return 0;
+
+ if (!p_left || !p_right)
+ return p_left ? -1 : 1;
+
+ if (c != COMPUTE_DELTA) {
+ /*
+ * The delta can be computed without the baseline, but
+ * others are not. Put those entries which have no
+ * values below.
+ */
+ if (left->dummy && right->dummy)
+ return 0;
+
+ if (left->dummy || right->dummy)
+ return left->dummy ? 1 : -1;
+ }
+
+ return __hist_entry__cmp_compute(p_left, p_right, c);
+}
+
+static int64_t
hist_entry__cmp_nop(struct perf_hpp_fmt *fmt __maybe_unused,
struct hist_entry *left __maybe_unused,
struct hist_entry *right __maybe_unused)
@@ -601,6 +632,30 @@ hist_entry__cmp_wdiff(struct perf_hpp_fmt *fmt,
return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF, d->idx);
}
+static int64_t
+hist_entry__cmp_delta_idx(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_compute_idx(right, left, COMPUTE_DELTA,
+ sort_compute);
+}
+
+static int64_t
+hist_entry__cmp_ratio_idx(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_compute_idx(right, left, COMPUTE_RATIO,
+ sort_compute);
+}
+
+static int64_t
+hist_entry__cmp_wdiff_idx(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_compute_idx(right, left, COMPUTE_WEIGHTED_DIFF,
+ sort_compute);
+}
+
static void hists__process(struct hists *hists)
{
if (show_baseline_only)
@@ -1074,9 +1129,10 @@ static void data__hpp_register(struct data__file *d, int idx)
perf_hpp__register_sort_field(fmt);
}
-static void ui_init(void)
+static int ui_init(void)
{
struct data__file *d;
+ struct perf_hpp_fmt *fmt;
int i;
data__for_each_file(i, d) {
@@ -1106,6 +1162,46 @@ static void ui_init(void)
data__hpp_register(d, i ? PERF_HPP_DIFF__PERIOD :
PERF_HPP_DIFF__PERIOD_BASELINE);
}
+
+ if (!sort_compute)
+ return 0;
+
+ /*
+ * Prepend an fmt to sort on columns at 'sort_compute' first.
+ * This fmt is added only to the sort list but not to the
+ * output fields list.
+ *
+ * Note that this column (data) can be compared twice - one
+ * for this 'sort_compute' fmt and another for the normal
+ * diff_hpp_fmt. But it shouldn't a problem as most entries
+ * will be sorted out by first try or baseline and comparing
+ * is not a costly operation.
+ */
+ fmt = zalloc(sizeof(*fmt));
+ if (fmt == NULL) {
+ pr_err("Memory allocation failed\n");
+ return -1;
+ }
+
+ fmt->cmp = hist_entry__cmp_nop;
+ fmt->collapse = hist_entry__cmp_nop;
+
+ switch (compute) {
+ case COMPUTE_DELTA:
+ fmt->sort = hist_entry__cmp_delta_idx;
+ break;
+ case COMPUTE_RATIO:
+ fmt->sort = hist_entry__cmp_ratio_idx;
+ break;
+ case COMPUTE_WEIGHTED_DIFF:
+ fmt->sort = hist_entry__cmp_wdiff_idx;
+ break;
+ default:
+ BUG_ON(1);
+ }
+
+ list_add(&fmt->sort_list, &perf_hpp__sort_list);
+ return 0;
}
static int data_init(int argc, const char **argv)
@@ -1171,7 +1267,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
if (data_init(argc, argv) < 0)
return -1;
- ui_init();
+ if (ui_init() < 0)
+ return -1;
sort__mode = SORT_MODE__DIFF;
--
2.1.3
^ permalink raw reply related [flat|nested] 20+ messages in thread* [tip:perf/core] perf diff: Fix -o/--order option behavior
2015-01-08 0:45 ` [PATCH 7/7] perf diff: Fix -o/--order option behavior Namhyung Kim
@ 2015-01-28 15:08 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Namhyung Kim @ 2015-01-28 15:08 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, tglx, a.p.zijlstra, jolsa, kan.liang, mingo, hpa, namhyung,
jolsa, linux-kernel
Commit-ID: 566b5cfb035fb496280be61f976b5281563bfa27
Gitweb: http://git.kernel.org/tip/566b5cfb035fb496280be61f976b5281563bfa27
Author: Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 8 Jan 2015 09:45:48 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 21 Jan 2015 13:24:35 -0300
perf diff: Fix -o/--order option behavior
The prior change fixes default output ordering with each column but it
breaks -o/--order option. This patch prepends a new hpp fmt struct to
sort list but not to output field list so that it can affect ordering
without adding a new output column.
The new hpp fmt uses its own compare functions which treats dummy
entries (which have no baseline) little differently - the delta field
can be computed without baseline but others (ratio and wdiff) are not.
The new output will look like below:
$ perf diff -o 2 perf.data.{old,cur,new}
...
# Baseline/0 Delta/1 Delta/2 Shared Object Symbol
# .......... ....... ....... ................. ..........................................
22.98% +0.51% +0.52% libc-2.20.so [.] _int_malloc
5.70% +0.28% +0.30% libc-2.20.so [.] free
4.38% -0.21% +0.25% a.out [.] main
1.32% -0.15% +0.05% a.out [.] free@plt
+0.01% [kernel.kallsyms] [k] intel_pstate_timer_func
+0.01% [kernel.kallsyms] [k] _raw_spin_lock_irqsave
+0.01% [kernel.kallsyms] [k] timekeeping_update.constprop.8
+0.01% +0.01% [kernel.kallsyms] [k] apic_timer_interrupt
0.01% -0.00% [kernel.kallsyms] [k] native_read_msr_safe
0.01% -0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
1.31% +0.03% -0.06% a.out [.] malloc@plt
31.50% -0.74% -0.23% libc-2.20.so [.] _int_free
32.75% +0.28% -0.83% libc-2.20.so [.] malloc
0.01% [kernel.kallsyms] [k] scheduler_tick
+0.01% [kernel.kallsyms] [k] read_tsc
+0.01% [kernel.kallsyms] [k] perf_adjust_freq_unthr_context.part.82
In above example, the output was sorted by 'Delta/2' column first, and
then 'Baseline/0' and finally 'Delta/1'.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1420677949-6719-8-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-diff.c | 101 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 99 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 9844456..74aada5 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -558,6 +558,37 @@ hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
}
static int64_t
+hist_entry__cmp_compute_idx(struct hist_entry *left, struct hist_entry *right,
+ int c, int sort_idx)
+{
+ struct hist_entry *p_right, *p_left;
+
+ p_left = get_pair_data(left, &data__files[sort_idx]);
+ p_right = get_pair_data(right, &data__files[sort_idx]);
+
+ if (!p_left && !p_right)
+ return 0;
+
+ if (!p_left || !p_right)
+ return p_left ? -1 : 1;
+
+ if (c != COMPUTE_DELTA) {
+ /*
+ * The delta can be computed without the baseline, but
+ * others are not. Put those entries which have no
+ * values below.
+ */
+ if (left->dummy && right->dummy)
+ return 0;
+
+ if (left->dummy || right->dummy)
+ return left->dummy ? 1 : -1;
+ }
+
+ return __hist_entry__cmp_compute(p_left, p_right, c);
+}
+
+static int64_t
hist_entry__cmp_nop(struct perf_hpp_fmt *fmt __maybe_unused,
struct hist_entry *left __maybe_unused,
struct hist_entry *right __maybe_unused)
@@ -601,6 +632,30 @@ hist_entry__cmp_wdiff(struct perf_hpp_fmt *fmt,
return hist_entry__cmp_compute(right, left, COMPUTE_WEIGHTED_DIFF, d->idx);
}
+static int64_t
+hist_entry__cmp_delta_idx(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_compute_idx(right, left, COMPUTE_DELTA,
+ sort_compute);
+}
+
+static int64_t
+hist_entry__cmp_ratio_idx(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_compute_idx(right, left, COMPUTE_RATIO,
+ sort_compute);
+}
+
+static int64_t
+hist_entry__cmp_wdiff_idx(struct perf_hpp_fmt *fmt __maybe_unused,
+ struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_compute_idx(right, left, COMPUTE_WEIGHTED_DIFF,
+ sort_compute);
+}
+
static void hists__process(struct hists *hists)
{
if (show_baseline_only)
@@ -1074,9 +1129,10 @@ static void data__hpp_register(struct data__file *d, int idx)
perf_hpp__register_sort_field(fmt);
}
-static void ui_init(void)
+static int ui_init(void)
{
struct data__file *d;
+ struct perf_hpp_fmt *fmt;
int i;
data__for_each_file(i, d) {
@@ -1106,6 +1162,46 @@ static void ui_init(void)
data__hpp_register(d, i ? PERF_HPP_DIFF__PERIOD :
PERF_HPP_DIFF__PERIOD_BASELINE);
}
+
+ if (!sort_compute)
+ return 0;
+
+ /*
+ * Prepend an fmt to sort on columns at 'sort_compute' first.
+ * This fmt is added only to the sort list but not to the
+ * output fields list.
+ *
+ * Note that this column (data) can be compared twice - one
+ * for this 'sort_compute' fmt and another for the normal
+ * diff_hpp_fmt. But it shouldn't a problem as most entries
+ * will be sorted out by first try or baseline and comparing
+ * is not a costly operation.
+ */
+ fmt = zalloc(sizeof(*fmt));
+ if (fmt == NULL) {
+ pr_err("Memory allocation failed\n");
+ return -1;
+ }
+
+ fmt->cmp = hist_entry__cmp_nop;
+ fmt->collapse = hist_entry__cmp_nop;
+
+ switch (compute) {
+ case COMPUTE_DELTA:
+ fmt->sort = hist_entry__cmp_delta_idx;
+ break;
+ case COMPUTE_RATIO:
+ fmt->sort = hist_entry__cmp_ratio_idx;
+ break;
+ case COMPUTE_WEIGHTED_DIFF:
+ fmt->sort = hist_entry__cmp_wdiff_idx;
+ break;
+ default:
+ BUG_ON(1);
+ }
+
+ list_add(&fmt->sort_list, &perf_hpp__sort_list);
+ return 0;
}
static int data_init(int argc, const char **argv)
@@ -1171,7 +1267,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
if (data_init(argc, argv) < 0)
return -1;
- ui_init();
+ if (ui_init() < 0)
+ return -1;
sort__mode = SORT_MODE__DIFF;
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2)
2015-01-08 0:45 [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Namhyung Kim
` (6 preceding siblings ...)
2015-01-08 0:45 ` [PATCH 7/7] perf diff: Fix -o/--order option behavior Namhyung Kim
@ 2015-01-08 14:42 ` Jiri Olsa
2015-01-12 16:40 ` Arnaldo Carvalho de Melo
8 siblings, 0 replies; 20+ messages in thread
From: Jiri Olsa @ 2015-01-08 14:42 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, LKML,
Kan Liang
On Thu, Jan 08, 2015 at 09:45:41AM +0900, Namhyung Kim wrote:
> Hello,
>
> This patchset improves perf diff output to sort by baseline and then
> next columns in turn. And fix -o/--order option behavior too.
>
> * changes in v2)
> - pass fmt arg to ->cmp, ->collapse, ->sort callbacks (Jiri)
> - add Acked-by from Jiri
>
> The patch 1-3 are resend. The patch 4-5 are preparation and the patch
> 6-7 are the main change.
for the new stuff ^^^ :
Acked-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
>
>
> Before:
>
> # Baseline/0 Delta/1 Delta/2 Shared Object Symbol
> # .......... ....... ....... ................. ..........................................
> #
> 32.75% +0.28% -0.83% libc-2.20.so [.] malloc
> 31.50% -0.74% -0.23% libc-2.20.so [.] _int_free
> 22.98% +0.51% +0.52% libc-2.20.so [.] _int_malloc
> 5.70% +0.28% +0.30% libc-2.20.so [.] free
> 4.38% -0.21% +0.25% a.out [.] main
> 1.32% -0.15% +0.05% a.out [.] free@plt
> 1.31% +0.03% -0.06% a.out [.] malloc@plt
> 0.01% -0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
> 0.01% [kernel.kallsyms] [k] scheduler_tick
> 0.01% -0.00% [kernel.kallsyms] [k] native_read_msr_safe
> +0.01% [kernel.kallsyms] [k] _raw_spin_lock_irqsave
> +0.01% +0.01% [kernel.kallsyms] [k] apic_timer_interrupt
> +0.01% [kernel.kallsyms] [k] intel_pstate_timer_func
> +0.01% [kernel.kallsyms] [k] perf_adjust_freq_unthr_context.part.82
> +0.01% [kernel.kallsyms] [k] read_tsc
> +0.01% [kernel.kallsyms] [k] timekeeping_update.constprop.8
>
> After:
>
> # Baseline/0 Delta/1 Delta/2 Shared Object Symbol
> # .......... ....... ....... ................. ..........................................
> #
> 32.75% +0.28% -0.83% libc-2.20.so [.] malloc
> 31.50% -0.74% -0.23% libc-2.20.so [.] _int_free
> 22.98% +0.51% +0.52% libc-2.20.so [.] _int_malloc
> 5.70% +0.28% +0.30% libc-2.20.so [.] free
> 4.38% -0.21% +0.25% a.out [.] main
> 1.32% -0.15% +0.05% a.out [.] free@plt
> 1.31% +0.03% -0.06% a.out [.] malloc@plt
> 0.01% -0.01% -0.01% [kernel.kallsyms] [k] native_write_msr_safe
> 0.01% [kernel.kallsyms] [k] scheduler_tick
> 0.01% -0.00% [kernel.kallsyms] [k] native_read_msr_safe
> +0.01% +0.01% [kernel.kallsyms] [k] apic_timer_interrupt
> +0.01% [kernel.kallsyms] [k] read_tsc
> +0.01% [kernel.kallsyms] [k] perf_adjust_freq_unthr_context.part.82
> +0.01% [kernel.kallsyms] [k] intel_pstate_timer_func
> +0.01% [kernel.kallsyms] [k] _raw_spin_lock_irqsave
> +0.01% [kernel.kallsyms] [k] timekeeping_update.constprop.8
>
>
> You can get this from 'perf/diff-sort-v2' branch on my tree:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
>
> Any comments are welcome, thanks
> Namhyung
>
>
> Namhyung Kim (7):
> perf diff: Fix to sort by baseline field by default
> perf diff: Get rid of hists__compute_resort()
> perf diff: Print diff result more precisely
> perf diff: Introduce fmt_to_data_file() helper
> perf tools: Pass struct perf_hpp_fmt to its callbacks
> perf diff: Fix output ordering to honor next column
> perf diff: Fix -o/--order option behavior
>
> tools/perf/builtin-diff.c | 252 +++++++++++++++++++++++++++++++++-------------
> tools/perf/ui/hist.c | 12 ++-
> tools/perf/util/hist.c | 6 +-
> tools/perf/util/hist.h | 9 +-
> tools/perf/util/sort.c | 37 ++++++-
> 5 files changed, 232 insertions(+), 84 deletions(-)
>
> --
> 2.1.3
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2)
2015-01-08 0:45 [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Namhyung Kim
` (7 preceding siblings ...)
2015-01-08 14:42 ` [PATCHSET 0/7] perf diff: Fix and improve output ordering (v2) Jiri Olsa
@ 2015-01-12 16:40 ` Arnaldo Carvalho de Melo
8 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-01-12 16:40 UTC (permalink / raw)
To: Namhyung Kim; +Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, Kan Liang
Em Thu, Jan 08, 2015 at 09:45:41AM +0900, Namhyung Kim escreveu:
> Hello,
>
> This patchset improves perf diff output to sort by baseline and then
> next columns in turn. And fix -o/--order option behavior too.
>
> * changes in v2)
> - pass fmt arg to ->cmp, ->collapse, ->sort callbacks (Jiri)
> - add Acked-by from Jiri
>
> The patch 1-3 are resend. The patch 4-5 are preparation and the patch
> 6-7 are the main change.
Applied to perf/core.
- Arnaldo
^ permalink raw reply [flat|nested] 20+ messages in thread