[PATCH v2 1/2] perf hist: Deduplicate cmp/sort/collapse code
From: Dmitry Vyukov
Date: Mon Jan 13 2025 - 08:34:50 EST
Application of cmp/sort/collapse fmt callbacks is duplicated 6 times.
Factor it into a common helper function. NFC.
Signed-off-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
Cc: Ian Rogers <irogers@xxxxxxxxxx>
Cc: linux-perf-users@xxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
---
Changes in v2:
- add perf_hpp_fmt_cmp_t typedef to util/hist.h
- use the typedef for perf_hpp_fmt::{cmp,collapse,sort}
---
tools/perf/util/hist.c | 103 +++++++++++++++++------------------------
tools/perf/util/hist.h | 14 +++---
2 files changed, 49 insertions(+), 68 deletions(-)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index fff1345658016..a4b8ec3805565 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -32,6 +32,9 @@
#include <linux/time64.h>
#include <linux/zalloc.h>
+static int64_t hist_entry__cmp(struct hist_entry *left, struct hist_entry *right);
+static int64_t hist_entry__collapse(struct hist_entry *left, struct hist_entry *right);
+
static bool hists__filter_entry_by_dso(struct hists *hists,
struct hist_entry *he);
static bool hists__filter_entry_by_thread(struct hists *hists,
@@ -1292,19 +1295,26 @@ int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al,
return err;
}
-int64_t
-hist_entry__cmp(struct hist_entry *left, struct hist_entry *right)
+static int64_t
+hist_entry__cmp_impl(struct perf_hpp_list *hpp_list, struct hist_entry *left,
+ struct hist_entry *right, unsigned long fn_offset,
+ bool ignore_dynamic, bool ignore_skipped)
{
struct hists *hists = left->hists;
struct perf_hpp_fmt *fmt;
int64_t cmp = 0;
+ perf_hpp_fmt_cmp_t fn;
- hists__for_each_sort_list(hists, fmt) {
- if (perf_hpp__is_dynamic_entry(fmt) &&
+ perf_hpp_list__for_each_sort_list(hpp_list, fmt) {
+ if (ignore_dynamic && perf_hpp__is_dynamic_entry(fmt) &&
!perf_hpp__defined_dynamic_entry(fmt, hists))
continue;
- cmp = fmt->cmp(fmt, left, right);
+ if (ignore_skipped && perf_hpp__should_skip(fmt, hists))
+ continue;
+
+ fn = *(perf_hpp_fmt_cmp_t *)(((char *)fmt) + fn_offset);
+ cmp = fn(fmt, left, right);
if (cmp)
break;
}
@@ -1313,23 +1323,33 @@ hist_entry__cmp(struct hist_entry *left, struct hist_entry *right)
}
int64_t
-hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)
+hist_entry__cmp(struct hist_entry *left, struct hist_entry *right)
{
- struct hists *hists = left->hists;
- struct perf_hpp_fmt *fmt;
- int64_t cmp = 0;
+ return hist_entry__cmp_impl(left->hists->hpp_list, left, right,
+ offsetof(struct perf_hpp_fmt, cmp), true, false);
+}
- hists__for_each_sort_list(hists, fmt) {
- if (perf_hpp__is_dynamic_entry(fmt) &&
- !perf_hpp__defined_dynamic_entry(fmt, hists))
- continue;
+static int64_t
+hist_entry__sort(struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_impl(left->hists->hpp_list, left, right,
+ offsetof(struct perf_hpp_fmt, sort), false, true);
+}
- cmp = fmt->collapse(fmt, left, right);
- if (cmp)
- break;
- }
+int64_t
+hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)
+{
+ return hist_entry__cmp_impl(left->hists->hpp_list, left, right,
+ offsetof(struct perf_hpp_fmt, collapse), true, false);
+}
- return cmp;
+static int64_t
+hist_entry__collapse_hierarchy(struct perf_hpp_list *hpp_list,
+ struct hist_entry *left,
+ struct hist_entry *right)
+{
+ return hist_entry__cmp_impl(hpp_list, left, right,
+ offsetof(struct perf_hpp_fmt, collapse), false, false);
}
void hist_entry__delete(struct hist_entry *he)
@@ -1503,14 +1523,7 @@ static struct hist_entry *hierarchy_insert_entry(struct hists *hists,
while (*p != NULL) {
parent = *p;
iter = rb_entry(parent, struct hist_entry, rb_node_in);
-
- cmp = 0;
- perf_hpp_list__for_each_sort_list(hpp_list, fmt) {
- cmp = fmt->collapse(fmt, iter, he);
- if (cmp)
- break;
- }
-
+ cmp = hist_entry__collapse_hierarchy(hpp_list, iter, he);
if (!cmp) {
he_stat__add_stat(&iter->stat, &he->stat);
return iter;
@@ -1730,24 +1743,6 @@ int hists__collapse_resort(struct hists *hists, struct ui_progress *prog)
return 0;
}
-static int64_t hist_entry__sort(struct hist_entry *a, struct hist_entry *b)
-{
- struct hists *hists = a->hists;
- struct perf_hpp_fmt *fmt;
- int64_t cmp = 0;
-
- hists__for_each_sort_list(hists, fmt) {
- if (perf_hpp__should_skip(fmt, a->hists))
- continue;
-
- cmp = fmt->sort(fmt, a, b);
- if (cmp)
- break;
- }
-
- return cmp;
-}
-
static void hists__reset_filter_stats(struct hists *hists)
{
hists->nr_non_filtered_entries = 0;
@@ -2449,21 +2444,15 @@ static struct hist_entry *add_dummy_hierarchy_entry(struct hists *hists,
struct rb_node **p;
struct rb_node *parent = NULL;
struct hist_entry *he;
- struct perf_hpp_fmt *fmt;
bool leftmost = true;
p = &root->rb_root.rb_node;
while (*p != NULL) {
- int64_t cmp = 0;
+ int64_t cmp;
parent = *p;
he = rb_entry(parent, struct hist_entry, rb_node_in);
-
- perf_hpp_list__for_each_sort_list(he->hpp_list, fmt) {
- cmp = fmt->collapse(fmt, he, pair);
- if (cmp)
- break;
- }
+ cmp = hist_entry__collapse_hierarchy(he->hpp_list, he, pair);
if (!cmp)
goto out;
@@ -2521,16 +2510,10 @@ static struct hist_entry *hists__find_hierarchy_entry(struct rb_root_cached *roo
while (n) {
struct hist_entry *iter;
- struct perf_hpp_fmt *fmt;
- int64_t cmp = 0;
+ int64_t cmp;
iter = rb_entry(n, struct hist_entry, rb_node_in);
- perf_hpp_list__for_each_sort_list(he->hpp_list, fmt) {
- cmp = fmt->collapse(fmt, iter, he);
- if (cmp)
- break;
- }
-
+ cmp = hist_entry__collapse_hierarchy(he->hpp_list, iter, he);
if (cmp < 0)
n = n->rb_left;
else if (cmp > 0)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 1131056924d9c..46c8373e31465 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -342,8 +342,6 @@ int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al,
struct perf_hpp;
struct perf_hpp_fmt;
-int64_t hist_entry__cmp(struct hist_entry *left, struct hist_entry *right);
-int64_t hist_entry__collapse(struct hist_entry *left, struct hist_entry *right);
int hist_entry__transaction_len(void);
int hist_entry__sort_snprintf(struct hist_entry *he, char *bf, size_t size,
struct hists *hists);
@@ -452,6 +450,9 @@ struct perf_hpp {
bool skip;
};
+typedef int64_t (*perf_hpp_fmt_cmp_t)(
+ struct perf_hpp_fmt *, struct hist_entry *, struct hist_entry *);
+
struct perf_hpp_fmt {
const char *name;
int (*header)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
@@ -463,12 +464,9 @@ 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 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);
+ perf_hpp_fmt_cmp_t cmp;
+ perf_hpp_fmt_cmp_t collapse;
+ perf_hpp_fmt_cmp_t sort;
bool (*equal)(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b);
void (*free)(struct perf_hpp_fmt *fmt);
--
2.47.1.688.g23fc6f90ad-goog