From 1240005e0d3a7e03c2fd05603fb01676e5a004f7 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Sat, 13 Oct 2012 00:06:16 +0200 Subject: perf hists: Introduce perf_hpp__list for period related columns Adding perf_hpp__list list to register and contain all period related columns the command is interested in. This way we get rid of static array holding all possible columns and enable commands to register their own columns. It'll be handy for diff command in future to process and display data for multiple files. Signed-off-by: Jiri Olsa Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Paul Mackerras Cc: Corey Ashford Cc: Frederic Weisbecker Cc: Namhyung Kim Link: http://lkml.kernel.org/n/tip-kiykge4igrcl7etmpmveto1h@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-diff.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) (limited to 'tools/perf/builtin-diff.c') diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index 93b852f8a5d..9fbbc01c5ad 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -597,40 +597,35 @@ static const struct option options[] = { static void ui_init(void) { - perf_hpp__init(); - - /* No overhead column. */ - perf_hpp__column_enable(PERF_HPP__OVERHEAD, false); - /* * Display baseline/delta/ratio/displacement/ * formula/periods columns. */ - perf_hpp__column_enable(PERF_HPP__BASELINE, true); + perf_hpp__column_enable(PERF_HPP__BASELINE); switch (compute) { case COMPUTE_DELTA: - perf_hpp__column_enable(PERF_HPP__DELTA, true); + perf_hpp__column_enable(PERF_HPP__DELTA); break; case COMPUTE_RATIO: - perf_hpp__column_enable(PERF_HPP__RATIO, true); + perf_hpp__column_enable(PERF_HPP__RATIO); break; case COMPUTE_WEIGHTED_DIFF: - perf_hpp__column_enable(PERF_HPP__WEIGHTED_DIFF, true); + perf_hpp__column_enable(PERF_HPP__WEIGHTED_DIFF); break; default: BUG_ON(1); }; if (show_displacement) - perf_hpp__column_enable(PERF_HPP__DISPL, true); + perf_hpp__column_enable(PERF_HPP__DISPL); if (show_formula) - perf_hpp__column_enable(PERF_HPP__FORMULA, true); + perf_hpp__column_enable(PERF_HPP__FORMULA); if (show_period) { - perf_hpp__column_enable(PERF_HPP__PERIOD, true); - perf_hpp__column_enable(PERF_HPP__PERIOD_BASELINE, true); + perf_hpp__column_enable(PERF_HPP__PERIOD); + perf_hpp__column_enable(PERF_HPP__PERIOD_BASELINE); } } -- cgit v1.2.3-70-g09d2 From 05472daa4d8ab88a071bfcaa3bb47473e4071848 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Wed, 28 Nov 2012 14:52:40 +0100 Subject: perf diff: Change compute methods to work with pair directly Changing compute methods to operate over hist entry and its pair directly. This makes the code more obvious and readable, instead of all time checking for pair being != NULL. Signed-off-by: Jiri Olsa Acked-by: Namhyung Kim Cc: Corey Ashford Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1354110769-2998-6-git-send-email-jolsa@redhat.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-diff.c | 38 +++++++++++++++++--------------------- tools/perf/ui/hist.c | 40 +++++++++++++++++++++++++--------------- tools/perf/util/hist.h | 7 ++++--- 3 files changed, 46 insertions(+), 39 deletions(-) (limited to 'tools/perf/builtin-diff.c') diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index 9fbbc01c5ad..342085a1807 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -146,47 +146,40 @@ static int setup_compute(const struct option *opt, const char *str, return -EINVAL; } -static double get_period_percent(struct hist_entry *he, u64 period) +double perf_diff__period_percent(struct hist_entry *he, u64 period) { u64 total = he->hists->stats.total_period; return (period * 100.0) / total; } -double perf_diff__compute_delta(struct hist_entry *he) +double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair) { - struct hist_entry *pair = hist_entry__next_pair(he); - double new_percent = get_period_percent(he, he->stat.period); - double old_percent = pair ? get_period_percent(pair, pair->stat.period) : 0.0; + double new_percent = perf_diff__period_percent(he, he->stat.period); + double old_percent = perf_diff__period_percent(pair, pair->stat.period); he->diff.period_ratio_delta = new_percent - old_percent; he->diff.computed = true; return he->diff.period_ratio_delta; } -double perf_diff__compute_ratio(struct hist_entry *he) +double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair) { - struct hist_entry *pair = hist_entry__next_pair(he); double new_period = he->stat.period; - double old_period = pair ? pair->stat.period : 0; + double old_period = pair->stat.period; he->diff.computed = true; - he->diff.period_ratio = pair ? (new_period / old_period) : 0; + he->diff.period_ratio = new_period / old_period; return he->diff.period_ratio; } -s64 perf_diff__compute_wdiff(struct hist_entry *he) +s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair) { - struct hist_entry *pair = hist_entry__next_pair(he); u64 new_period = he->stat.period; - u64 old_period = pair ? pair->stat.period : 0; + u64 old_period = pair->stat.period; he->diff.computed = true; - - if (!pair) - he->diff.wdiff = 0; - else - he->diff.wdiff = new_period * compute_wdiff_w2 - - old_period * compute_wdiff_w1; + he->diff.wdiff = new_period * compute_wdiff_w2 - + old_period * compute_wdiff_w1; return he->diff.wdiff; } @@ -385,18 +378,21 @@ static void hists__precompute(struct hists *hists) while (next != NULL) { struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node); + struct hist_entry *pair = hist_entry__next_pair(he); next = rb_next(&he->rb_node); + if (!pair) + continue; switch (compute) { case COMPUTE_DELTA: - perf_diff__compute_delta(he); + perf_diff__compute_delta(he, pair); break; case COMPUTE_RATIO: - perf_diff__compute_ratio(he); + perf_diff__compute_ratio(he, pair); break; case COMPUTE_WEIGHTED_DIFF: - perf_diff__compute_wdiff(he); + perf_diff__compute_wdiff(he, pair); break; default: BUG_ON(1); diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c index 6e639b50682..108e5ed6762 100644 --- a/tools/perf/ui/hist.c +++ b/tools/perf/ui/hist.c @@ -268,14 +268,18 @@ static int hpp__width_delta(struct perf_hpp *hpp __maybe_unused) static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he) { + struct hist_entry *pair = hist_entry__next_pair(he); const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s"; char buf[32] = " "; - double diff; + double diff = 0.0; - if (he->diff.computed) - diff = he->diff.period_ratio_delta; - else - diff = perf_diff__compute_delta(he); + if (pair) { + if (he->diff.computed) + diff = he->diff.period_ratio_delta; + else + diff = perf_diff__compute_delta(he, pair); + } else + diff = perf_diff__period_percent(he, he->stat.period); if (fabs(diff) >= 0.01) scnprintf(buf, sizeof(buf), "%+4.2F%%", diff); @@ -297,14 +301,17 @@ static int hpp__width_ratio(struct perf_hpp *hpp __maybe_unused) static int hpp__entry_ratio(struct perf_hpp *hpp, struct hist_entry *he) { + struct hist_entry *pair = hist_entry__next_pair(he); const char *fmt = symbol_conf.field_sep ? "%s" : "%14s"; char buf[32] = " "; - double ratio; + double ratio = 0.0; - if (he->diff.computed) - ratio = he->diff.period_ratio; - else - ratio = perf_diff__compute_ratio(he); + if (pair) { + if (he->diff.computed) + ratio = he->diff.period_ratio; + else + ratio = perf_diff__compute_ratio(he, pair); + } if (ratio > 0.0) scnprintf(buf, sizeof(buf), "%+14.6F", ratio); @@ -326,14 +333,17 @@ static int hpp__width_wdiff(struct perf_hpp *hpp __maybe_unused) static int hpp__entry_wdiff(struct perf_hpp *hpp, struct hist_entry *he) { + struct hist_entry *pair = hist_entry__next_pair(he); const char *fmt = symbol_conf.field_sep ? "%s" : "%14s"; char buf[32] = " "; - s64 wdiff; + s64 wdiff = 0; - if (he->diff.computed) - wdiff = he->diff.wdiff; - else - wdiff = perf_diff__compute_wdiff(he); + if (pair) { + if (he->diff.computed) + wdiff = he->diff.wdiff; + else + wdiff = perf_diff__compute_wdiff(he, pair); + } if (wdiff != 0) scnprintf(buf, sizeof(buf), "%14ld", wdiff); diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index a935a60d521..235503aac08 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -226,8 +226,9 @@ int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist __maybe_unused, unsigned int hists__sort_list_width(struct hists *self); -double perf_diff__compute_delta(struct hist_entry *he); -double perf_diff__compute_ratio(struct hist_entry *he); -s64 perf_diff__compute_wdiff(struct hist_entry *he); +double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair); +double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair); +s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair); int perf_diff__formula(char *buf, size_t size, struct hist_entry *he); +double perf_diff__period_percent(struct hist_entry *he, u64 period); #endif /* __PERF_HIST_H */ -- cgit v1.2.3-70-g09d2 From f4c8bae1920c459b7b9c12363d11e8a588862e42 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Wed, 28 Nov 2012 14:52:41 +0100 Subject: perf diff: Change formula methods to work with pair directly Changing formula methods to operate over hist entry and its pair directly. This makes the code more obvious and readable, instead of all time checking for pair being != NULL. Signed-off-by: Jiri Olsa Acked-by: Namhyung Kim Cc: Corey Ashford Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1354110769-2998-7-git-send-email-jolsa@redhat.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-diff.c | 35 +++++++++++++---------------------- tools/perf/ui/hist.c | 5 ++++- tools/perf/util/hist.h | 3 ++- 3 files changed, 19 insertions(+), 24 deletions(-) (limited to 'tools/perf/builtin-diff.c') diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index 342085a1807..d869029fb75 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -184,13 +184,9 @@ s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair) return he->diff.wdiff; } -static int formula_delta(struct hist_entry *he, char *buf, size_t size) +static int formula_delta(struct hist_entry *he, struct hist_entry *pair, + char *buf, size_t size) { - struct hist_entry *pair = hist_entry__next_pair(he); - - if (!pair) - return -1; - return scnprintf(buf, size, "(%" PRIu64 " * 100 / %" PRIu64 ") - " "(%" PRIu64 " * 100 / %" PRIu64 ")", @@ -198,41 +194,36 @@ static int formula_delta(struct hist_entry *he, char *buf, size_t size) pair->stat.period, pair->hists->stats.total_period); } -static int formula_ratio(struct hist_entry *he, char *buf, size_t size) +static int formula_ratio(struct hist_entry *he, struct hist_entry *pair, + char *buf, size_t size) { - struct hist_entry *pair = hist_entry__next_pair(he); double new_period = he->stat.period; - double old_period = pair ? pair->stat.period : 0; - - if (!pair) - return -1; + double old_period = pair->stat.period; return scnprintf(buf, size, "%.0F / %.0F", new_period, old_period); } -static int formula_wdiff(struct hist_entry *he, char *buf, size_t size) +static int formula_wdiff(struct hist_entry *he, struct hist_entry *pair, + char *buf, size_t size) { - struct hist_entry *pair = hist_entry__next_pair(he); u64 new_period = he->stat.period; - u64 old_period = pair ? pair->stat.period : 0; - - if (!pair) - return -1; + u64 old_period = pair->stat.period; return scnprintf(buf, size, "(%" PRIu64 " * " "%" PRId64 ") - (%" PRIu64 " * " "%" PRId64 ")", new_period, compute_wdiff_w2, old_period, compute_wdiff_w1); } -int perf_diff__formula(char *buf, size_t size, struct hist_entry *he) +int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair, + char *buf, size_t size) { switch (compute) { case COMPUTE_DELTA: - return formula_delta(he, buf, size); + return formula_delta(he, pair, buf, size); case COMPUTE_RATIO: - return formula_ratio(he, buf, size); + return formula_ratio(he, pair, buf, size); case COMPUTE_WEIGHTED_DIFF: - return formula_wdiff(he, buf, size); + return formula_wdiff(he, pair, buf, size); default: BUG_ON(1); } diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c index 108e5ed6762..1785bab7adf 100644 --- a/tools/perf/ui/hist.c +++ b/tools/perf/ui/hist.c @@ -389,10 +389,13 @@ static int hpp__width_formula(struct perf_hpp *hpp __maybe_unused) static int hpp__entry_formula(struct perf_hpp *hpp, struct hist_entry *he) { + struct hist_entry *pair = hist_entry__next_pair(he); const char *fmt = symbol_conf.field_sep ? "%s" : "%-70s"; char buf[96] = " "; - perf_diff__formula(buf, sizeof(buf), he); + if (pair) + perf_diff__formula(he, pair, buf, sizeof(buf)); + return scnprintf(hpp->buf, hpp->size, fmt, buf); } diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 235503aac08..c1b2fade8e7 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -229,6 +229,7 @@ unsigned int hists__sort_list_width(struct hists *self); double perf_diff__compute_delta(struct hist_entry *he, struct hist_entry *pair); double perf_diff__compute_ratio(struct hist_entry *he, struct hist_entry *pair); s64 perf_diff__compute_wdiff(struct hist_entry *he, struct hist_entry *pair); -int perf_diff__formula(char *buf, size_t size, struct hist_entry *he); +int perf_diff__formula(struct hist_entry *he, struct hist_entry *pair, + char *buf, size_t size); double perf_diff__period_percent(struct hist_entry *he, u64 period); #endif /* __PERF_HIST_H */ -- cgit v1.2.3-70-g09d2 From db6d0bb86164497f6c9ef46020cf1881953f4b08 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Thu, 6 Dec 2012 14:22:28 +0100 Subject: perf diff: Remove displacement output option It seems not very useful, because it's possible and event more convenient to lookup related symbol by name. Also the output value for both 'baseline' and 'new' data is quite apparent from diff output. And above all it complicates hist code factoring ;) Ditching out PERF_HPP__DISPL column with related output functions. Suggested-by: Arnaldo Carvalho de Melo Signed-off-by: Jiri Olsa Cc: Corey Ashford Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Ingo Molnar Cc: Namhyung Kim Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/20121206132228.GB1080@krava.brq.redhat.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-diff.txt | 4 ---- tools/perf/builtin-diff.c | 29 +++++++---------------------- tools/perf/ui/hist.c | 25 ------------------------- tools/perf/util/hist.h | 1 - 4 files changed, 7 insertions(+), 52 deletions(-) (limited to 'tools/perf/builtin-diff.c') diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt index 194f37d635d..5b3123d5721 100644 --- a/tools/perf/Documentation/perf-diff.txt +++ b/tools/perf/Documentation/perf-diff.txt @@ -22,10 +22,6 @@ specified perf.data files. OPTIONS ------- --M:: ---displacement:: - Show position displacement relative to baseline. - -D:: --dump-raw-trace:: Dump raw trace in ASCII. diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index d869029fb75..b2e7d39f099 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -23,7 +23,6 @@ static char const *input_old = "perf.data.old", *input_new = "perf.data"; 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; @@ -296,9 +295,8 @@ static void insert_hist_entry_by_name(struct rb_root *root, rb_insert_color(&he->rb_node, root); } -static void hists__name_resort(struct hists *self, bool sort) +static void hists__name_resort(struct hists *self) { - unsigned long position = 1; struct rb_root tmp = RB_ROOT; struct rb_node *next = rb_first(&self->entries); @@ -306,16 +304,12 @@ static void hists__name_resort(struct hists *self, bool sort) struct hist_entry *n = rb_entry(next, struct hist_entry, rb_node); next = rb_next(&n->rb_node); - n->position = position++; - if (sort) { - rb_erase(&n->rb_node, &self->entries); - insert_hist_entry_by_name(&tmp, n); - } + rb_erase(&n->rb_node, &self->entries); + insert_hist_entry_by_name(&tmp, n); } - if (sort) - self->entries = tmp; + self->entries = tmp; } static struct perf_evsel *evsel_match(struct perf_evsel *evsel, @@ -339,12 +333,8 @@ static void perf_evlist__resort_hists(struct perf_evlist *evlist, bool name) 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); + if (name) + hists__name_resort(hists); } } @@ -549,8 +539,6 @@ static const char * const diff_usage[] = { static const struct option options[] = { OPT_INCR('v', "verbose", &verbose, "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_CALLBACK('c', "compute", &compute, @@ -585,7 +573,7 @@ static const struct option options[] = { static void ui_init(void) { /* - * Display baseline/delta/ratio/displacement/ + * Display baseline/delta/ratio * formula/periods columns. */ perf_hpp__column_enable(PERF_HPP__BASELINE); @@ -604,9 +592,6 @@ static void ui_init(void) BUG_ON(1); }; - if (show_displacement) - perf_hpp__column_enable(PERF_HPP__DISPL); - if (show_formula) perf_hpp__column_enable(PERF_HPP__FORMULA); diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c index 1785bab7adf..1889c12ca81 100644 --- a/tools/perf/ui/hist.c +++ b/tools/perf/ui/hist.c @@ -351,30 +351,6 @@ static int hpp__entry_wdiff(struct perf_hpp *hpp, struct hist_entry *he) return scnprintf(hpp->buf, hpp->size, fmt, buf); } -static int hpp__header_displ(struct perf_hpp *hpp) -{ - return scnprintf(hpp->buf, hpp->size, "Displ."); -} - -static int hpp__width_displ(struct perf_hpp *hpp __maybe_unused) -{ - return 6; -} - -static int hpp__entry_displ(struct perf_hpp *hpp, - struct hist_entry *he) -{ - struct hist_entry *pair = hist_entry__next_pair(he); - long displacement = pair ? pair->position - he->position : 0; - const char *fmt = symbol_conf.field_sep ? "%s" : "%6.6s"; - char buf[32] = " "; - - if (displacement) - scnprintf(buf, sizeof(buf), "%+4ld", displacement); - - return scnprintf(hpp->buf, hpp->size, fmt, buf); -} - static int hpp__header_formula(struct perf_hpp *hpp) { const char *fmt = symbol_conf.field_sep ? "%s" : "%70s"; @@ -427,7 +403,6 @@ struct perf_hpp_fmt perf_hpp__format[] = { HPP__PRINT_FNS(delta), HPP__PRINT_FNS(ratio), HPP__PRINT_FNS(wdiff), - HPP__PRINT_FNS(displ), HPP__PRINT_FNS(formula) }; diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index c1b2fade8e7..5b3b0075be6 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -154,7 +154,6 @@ enum { PERF_HPP__DELTA, PERF_HPP__RATIO, PERF_HPP__WEIGHTED_DIFF, - PERF_HPP__DISPL, PERF_HPP__FORMULA, PERF_HPP__MAX_INDEX -- cgit v1.2.3-70-g09d2 From 9afcf930b1fa1158b0878afeba3eff299300dc65 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 10 Dec 2012 17:29:54 +0900 Subject: perf hists: Exchange order of comparing items when collapsing hists When comparing entries for collapsing put the given entry first, and then the iterated entry. This is not the case of hist_entry__cmp() when called if given sort keys don't require collapsing. So change the order for the sake of consistency. It will be required for matching and/or linking multiple hist entries. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Ingo Molnar Cc: Jiri Olsa Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1355128197-18193-2-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-diff.c | 2 +- tools/perf/util/hist.c | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) (limited to 'tools/perf/builtin-diff.c') diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index b2e7d39f099..4dda6f4dc61 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -285,7 +285,7 @@ static void insert_hist_entry_by_name(struct rb_root *root, while (*p != NULL) { parent = *p; iter = rb_entry(parent, struct hist_entry, rb_node); - if (hist_entry__cmp(he, iter) < 0) + if (hist_entry__cmp(iter, he) < 0) p = &(*p)->rb_left; else p = &(*p)->rb_right; diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 82df1b26f0d..3a9ccd09835 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -285,7 +285,13 @@ static struct hist_entry *add_hist_entry(struct hists *hists, parent = *p; he = rb_entry(parent, struct hist_entry, rb_node_in); - cmp = hist_entry__cmp(entry, he); + /* + * Make sure that it receives arguments in a same order as + * hist_entry__collapse() so that we can use an appropriate + * function when searching an entry regardless which sort + * keys were used. + */ + cmp = hist_entry__cmp(he, entry); if (!cmp) { he_stat__add_period(&he->stat, period); @@ -729,7 +735,7 @@ static struct hist_entry *hists__add_dummy_entry(struct hists *hists, parent = *p; he = rb_entry(parent, struct hist_entry, rb_node); - cmp = hist_entry__cmp(pair, he); + cmp = hist_entry__cmp(he, pair); if (!cmp) goto out; @@ -759,7 +765,7 @@ static struct hist_entry *hists__find_entry(struct hists *hists, while (n) { struct hist_entry *iter = rb_entry(n, struct hist_entry, rb_node); - int64_t cmp = hist_entry__cmp(he, iter); + int64_t cmp = hist_entry__cmp(iter, he); if (cmp < 0) n = n->rb_left; -- cgit v1.2.3-70-g09d2 From ce74f60eab3cc8b7a3b0cb9c29ec9b1e1abac7d2 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 10 Dec 2012 17:29:55 +0900 Subject: perf hists: Link hist entries before inserting to an output tree For matching and/or linking hist entries, they need to be sorted by given sort keys. However current hists__match/link did this on the output trees, so that the entries in the output tree need to be resort before doing it. This looks not so good since we have trees for collecting or collapsing entries before passing them to an output tree and they're already sorted by the given sort keys. Since we don't need to print anything at the time of matching/linking, we can use these internal trees directly instead of bothering with double resort on the output tree. Its only user - at the time of this writing - perf diff can be easily converted to use the internal tree and can save some lines too by getting rid of unnecessary resorting codes. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Ingo Molnar Cc: Jiri Olsa Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1355128197-18193-3-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-diff.c | 65 +++++++++++++---------------------------------- tools/perf/util/hist.c | 49 ++++++++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 60 deletions(-) (limited to 'tools/perf/builtin-diff.c') diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index 4dda6f4dc61..8b896f5eded 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -275,43 +275,6 @@ static struct perf_tool tool = { .ordering_requires_timestamps = true, }; -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; - struct hist_entry *iter; - - while (*p != NULL) { - parent = *p; - iter = rb_entry(parent, struct hist_entry, rb_node); - if (hist_entry__cmp(iter, he) < 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__name_resort(struct hists *self) -{ - struct rb_root tmp = RB_ROOT; - struct rb_node *next = rb_first(&self->entries); - - while (next != NULL) { - 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); - insert_hist_entry_by_name(&tmp, n); - } - - self->entries = tmp; -} - static struct perf_evsel *evsel_match(struct perf_evsel *evsel, struct perf_evlist *evlist) { @@ -324,30 +287,34 @@ static struct perf_evsel *evsel_match(struct perf_evsel *evsel, return NULL; } -static void perf_evlist__resort_hists(struct perf_evlist *evlist, bool name) +static void perf_evlist__collapse_resort(struct perf_evlist *evlist) { struct perf_evsel *evsel; list_for_each_entry(evsel, &evlist->entries, node) { struct hists *hists = &evsel->hists; - hists__output_resort(hists); - - if (name) - hists__name_resort(hists); + hists__collapse_resort(hists); } } static void hists__baseline_only(struct hists *hists) { - struct rb_node *next = rb_first(&hists->entries); + struct rb_root *root; + struct rb_node *next; + + if (sort__need_collapse) + root = &hists->entries_collapsed; + else + root = hists->entries_in; + next = rb_first(root); while (next != NULL) { - struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node); + struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node_in); - next = rb_next(&he->rb_node); + next = rb_next(&he->rb_node_in); if (!hist_entry__next_pair(he)) { - rb_erase(&he->rb_node, &hists->entries); + rb_erase(&he->rb_node_in, root); hist_entry__free(he); } } @@ -471,6 +438,8 @@ static void hists__process(struct hists *old, struct hists *new) else hists__link(new, old); + hists__output_resort(new); + if (sort_compute) { hists__precompute(new); hists__compute_resort(new); @@ -505,8 +474,8 @@ static int __cmd_diff(void) evlist_old = older->evlist; evlist_new = newer->evlist; - perf_evlist__resort_hists(evlist_old, true); - perf_evlist__resort_hists(evlist_new, false); + perf_evlist__collapse_resort(evlist_old); + perf_evlist__collapse_resort(evlist_new); list_for_each_entry(evsel, &evlist_new->entries, node) { struct perf_evsel *evsel_old; diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 3a9ccd09835..8ff3c2f8a5d 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -726,16 +726,24 @@ void hists__inc_nr_events(struct hists *hists, u32 type) static struct hist_entry *hists__add_dummy_entry(struct hists *hists, struct hist_entry *pair) { - struct rb_node **p = &hists->entries.rb_node; + struct rb_root *root; + struct rb_node **p; struct rb_node *parent = NULL; struct hist_entry *he; int cmp; + if (sort__need_collapse) + root = &hists->entries_collapsed; + else + root = hists->entries_in; + + p = &root->rb_node; + while (*p != NULL) { parent = *p; - he = rb_entry(parent, struct hist_entry, rb_node); + he = rb_entry(parent, struct hist_entry, rb_node_in); - cmp = hist_entry__cmp(he, pair); + cmp = hist_entry__collapse(he, pair); if (!cmp) goto out; @@ -750,8 +758,8 @@ static struct hist_entry *hists__add_dummy_entry(struct hists *hists, if (he) { memset(&he->stat, 0, sizeof(he->stat)); he->hists = hists; - rb_link_node(&he->rb_node, parent, p); - rb_insert_color(&he->rb_node, &hists->entries); + rb_link_node(&he->rb_node_in, parent, p); + rb_insert_color(&he->rb_node_in, root); hists__inc_nr_entries(hists, he); } out: @@ -761,11 +769,16 @@ out: static struct hist_entry *hists__find_entry(struct hists *hists, struct hist_entry *he) { - struct rb_node *n = hists->entries.rb_node; + struct rb_node *n; + + if (sort__need_collapse) + n = hists->entries_collapsed.rb_node; + else + n = hists->entries_in->rb_node; while (n) { - struct hist_entry *iter = rb_entry(n, struct hist_entry, rb_node); - int64_t cmp = hist_entry__cmp(iter, he); + struct hist_entry *iter = rb_entry(n, struct hist_entry, rb_node_in); + int64_t cmp = hist_entry__collapse(iter, he); if (cmp < 0) n = n->rb_left; @@ -783,11 +796,17 @@ static struct hist_entry *hists__find_entry(struct hists *hists, */ void hists__match(struct hists *leader, struct hists *other) { + struct rb_root *root; struct rb_node *nd; struct hist_entry *pos, *pair; - for (nd = rb_first(&leader->entries); nd; nd = rb_next(nd)) { - pos = rb_entry(nd, struct hist_entry, rb_node); + if (sort__need_collapse) + root = &leader->entries_collapsed; + else + root = leader->entries_in; + + for (nd = rb_first(root); nd; nd = rb_next(nd)) { + pos = rb_entry(nd, struct hist_entry, rb_node_in); pair = hists__find_entry(other, pos); if (pair) @@ -802,11 +821,17 @@ void hists__match(struct hists *leader, struct hists *other) */ int hists__link(struct hists *leader, struct hists *other) { + struct rb_root *root; struct rb_node *nd; struct hist_entry *pos, *pair; - for (nd = rb_first(&other->entries); nd; nd = rb_next(nd)) { - pos = rb_entry(nd, struct hist_entry, rb_node); + if (sort__need_collapse) + root = &other->entries_collapsed; + else + root = other->entries_in; + + for (nd = rb_first(root); nd; nd = rb_next(nd)) { + pos = rb_entry(nd, struct hist_entry, rb_node_in); if (!hist_entry__has_pairs(pos)) { pair = hists__add_dummy_entry(leader, pos); -- cgit v1.2.3-70-g09d2 From 66f97ed3ac44c24958171bbc5cc04896147752b7 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 10 Dec 2012 17:29:56 +0900 Subject: perf diff: Use internal rb tree for compute resort There's no reason to run hists_compute_resort() using output tree. Convert it to use internal tree so that it can remove unnecessary _output_resort. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Ingo Molnar Cc: Jiri Olsa Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1355128197-18193-4-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-diff.c | 31 +++++++++++++++++++++---------- tools/perf/util/hist.c | 2 +- tools/perf/util/hist.h | 1 + 3 files changed, 23 insertions(+), 11 deletions(-) (limited to 'tools/perf/builtin-diff.c') diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index 8b896f5eded..4af0b580b04 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -414,19 +414,30 @@ static void insert_hist_entry_by_compute(struct rb_root *root, static void hists__compute_resort(struct hists *hists) { - struct rb_root tmp = RB_ROOT; - struct rb_node *next = rb_first(&hists->entries); + 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->nr_entries = 0; + hists->stats.total_period = 0; + hists__reset_col_len(hists); while (next != NULL) { - struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node); + struct hist_entry *he; - next = rb_next(&he->rb_node); + he = rb_entry(next, struct hist_entry, rb_node_in); + next = rb_next(&he->rb_node_in); - rb_erase(&he->rb_node, &hists->entries); - insert_hist_entry_by_compute(&tmp, he, compute); + insert_hist_entry_by_compute(&hists->entries, he, compute); + hists__inc_nr_entries(hists, he); } - - hists->entries = tmp; } static void hists__process(struct hists *old, struct hists *new) @@ -438,11 +449,11 @@ static void hists__process(struct hists *old, struct hists *new) else hists__link(new, old); - hists__output_resort(new); - if (sort_compute) { hists__precompute(new); hists__compute_resort(new); + } else { + hists__output_resort(new); } hists__fprintf(new, true, 0, 0, stdout); diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 8ff3c2f8a5d..37179af7440 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -251,7 +251,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template) return he; } -static void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h) +void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h) { if (!h->filtered) { hists__calc_col_len(hists, h); diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 5b3b0075be6..d3664abea6d 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -96,6 +96,7 @@ void hists__decay_entries_threaded(struct hists *hists, bool zap_user, bool zap_kernel); void hists__output_recalc_col_len(struct hists *hists, int max_rows); +void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h); void hists__inc_nr_events(struct hists *self, u32 type); size_t hists__fprintf_nr_events(struct hists *self, FILE *fp); -- cgit v1.2.3-70-g09d2 From 553099857702bb77e541c47bde47f6863834d2e2 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Wed, 6 Feb 2013 14:57:16 +0900 Subject: perf sort: Make setup_sorting returns an error code Currently the setup_sorting() is called for parsing sort keys and exits if it failed to add the sort key. As it's included in libperf it'd be better returning an error code rather than exiting application inside of the library. Signed-off-by: Namhyung Kim Suggested-by: Arnaldo Carvalho de Melo Cc: Ingo Molnar Cc: Jiri Olsa Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1360130237-9963-2-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-annotate.c | 3 ++- tools/perf/builtin-diff.c | 4 +++- tools/perf/builtin-report.c | 3 ++- tools/perf/builtin-top.c | 3 ++- tools/perf/tests/hists_link.c | 3 ++- tools/perf/util/sort.c | 10 ++++++---- tools/perf/util/sort.h | 2 +- 7 files changed, 18 insertions(+), 10 deletions(-) (limited to 'tools/perf/builtin-diff.c') diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index dc870cf31b7..95a2ad3f043 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -309,7 +309,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused) if (symbol__init() < 0) return -1; - setup_sorting(annotate_usage, options); + if (setup_sorting() < 0) + usage_with_options(annotate_usage, options); if (argc) { /* diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index 4af0b580b04..d207a97a2db 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -605,7 +605,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused) ui_init(); - setup_sorting(diff_usage, options); + if (setup_sorting() < 0) + usage_with_options(diff_usage, options); + setup_pager(); sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", NULL); diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 91555d4885f..96b5a7fee4b 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -751,7 +751,8 @@ repeat: } - setup_sorting(report_usage, options); + if (setup_sorting() < 0) + usage_with_options(report_usage, options); /* * Only in the newt browser we are doing integrated annotation, diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index f561757b1bf..72f6eb7b417 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -1129,7 +1129,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused) if (sort_order == default_sort_order) sort_order = "dso,symbol"; - setup_sorting(top_usage, options); + if (setup_sorting() < 0) + usage_with_options(top_usage, options); if (top.use_stdio) use_browser = 0; diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c index 0afd9223bde..1be64a6c5da 100644 --- a/tools/perf/tests/hists_link.c +++ b/tools/perf/tests/hists_link.c @@ -449,7 +449,8 @@ int test__hists_link(void) goto out; /* default sort order (comm,dso,sym) will be used */ - setup_sorting(NULL, NULL); + if (setup_sorting() < 0) + goto out; machines__init(&machines); diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index 03cabe5678d..d8b48827a17 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -565,23 +565,25 @@ int sort_dimension__add(const char *tok) return -ESRCH; } -void setup_sorting(const char * const usagestr[], const struct option *opts) +int setup_sorting(void) { char *tmp, *tok, *str = strdup(sort_order); + int ret = 0; for (tok = strtok_r(str, ", ", &tmp); tok; tok = strtok_r(NULL, ", ", &tmp)) { - int ret = sort_dimension__add(tok); + ret = sort_dimension__add(tok); if (ret == -EINVAL) { error("Invalid --sort key: `%s'", tok); - usage_with_options(usagestr, opts); + break; } else if (ret == -ESRCH) { error("Unknown --sort key: `%s'", tok); - usage_with_options(usagestr, opts); + break; } } free(str); + return ret; } void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list, diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h index e994ad3e989..b13e56f6ccb 100644 --- a/tools/perf/util/sort.h +++ b/tools/perf/util/sort.h @@ -160,7 +160,7 @@ struct sort_entry { extern struct sort_entry sort_thread; extern struct list_head hist_entry__sort_list; -void setup_sorting(const char * const usagestr[], const struct option *opts); +int setup_sorting(void); extern int sort_dimension__add(const char *); void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list, const char *list_name, FILE *fp); -- cgit v1.2.3-70-g09d2