Messages in this thread | | | From | Ian Rogers <> | Date | Mon, 10 Oct 2022 15:53:10 -0700 | Subject | Re: [PATCH 06/19] perf stat: Add 'needs_sort' argument to cpu_aggr_map__new() |
| |
On Sun, Oct 9, 2022 at 10:36 PM Namhyung Kim <namhyung@kernel.org> wrote: > > In case of no aggregation, it needs to keep the original (cpu) ordering > in the aggr_map so that it can be in sync with the cpu map. This will > make the code easier to handle AGGR_NONE similar to others. >
The CPU map is sorted and so sorting the aggr_map should be fine. If the data is already sorted then it is O(n) to sort. I think this is preferable to having additional complexity around whether the aggr_map is sorted.
Thanks, Ian
> Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/builtin-stat.c | 7 +++++-- > tools/perf/util/cpumap.c | 6 ++++-- > tools/perf/util/cpumap.h | 2 +- > 3 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index b00ef20aef5b..e5ddf60ab31d 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -1461,8 +1461,9 @@ static int perf_stat_init_aggr_mode(void) > aggr_cpu_id_get_t get_id = aggr_mode__get_aggr(stat_config.aggr_mode); > > if (get_id) { > + bool needs_sort = stat_config.aggr_mode != AGGR_NONE; > stat_config.aggr_map = cpu_aggr_map__new(evsel_list->core.user_requested_cpus, > - get_id, /*data=*/NULL); > + get_id, /*data=*/NULL, needs_sort); > if (!stat_config.aggr_map) { > pr_err("cannot build %s map", aggr_mode__string[stat_config.aggr_mode]); > return -1; > @@ -1677,11 +1678,13 @@ static int perf_stat_init_aggr_mode_file(struct perf_stat *st) > { > struct perf_env *env = &st->session->header.env; > aggr_cpu_id_get_t get_id = aggr_mode__get_aggr_file(stat_config.aggr_mode); > + bool needs_sort = stat_config.aggr_mode != AGGR_NONE; > > if (!get_id) > return 0; > > - stat_config.aggr_map = cpu_aggr_map__new(evsel_list->core.user_requested_cpus, get_id, env); > + stat_config.aggr_map = cpu_aggr_map__new(evsel_list->core.user_requested_cpus, > + get_id, env, needs_sort); > if (!stat_config.aggr_map) { > pr_err("cannot build %s map", aggr_mode__string[stat_config.aggr_mode]); > return -1; > diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c > index 60209fe87456..6e3fcf523de9 100644 > --- a/tools/perf/util/cpumap.c > +++ b/tools/perf/util/cpumap.c > @@ -234,7 +234,7 @@ static int aggr_cpu_id__cmp(const void *a_pointer, const void *b_pointer) > > struct cpu_aggr_map *cpu_aggr_map__new(const struct perf_cpu_map *cpus, > aggr_cpu_id_get_t get_id, > - void *data) > + void *data, bool needs_sort) > { > int idx; > struct perf_cpu cpu; > @@ -270,8 +270,10 @@ struct cpu_aggr_map *cpu_aggr_map__new(const struct perf_cpu_map *cpus, > if (trimmed_c) > c = trimmed_c; > } > + > /* ensure we process id in increasing order */ > - qsort(c->map, c->nr, sizeof(struct aggr_cpu_id), aggr_cpu_id__cmp); > + if (needs_sort) > + qsort(c->map, c->nr, sizeof(struct aggr_cpu_id), aggr_cpu_id__cmp); > > return c; > > diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h > index b2ff648bc417..da28b3146ef9 100644 > --- a/tools/perf/util/cpumap.h > +++ b/tools/perf/util/cpumap.h > @@ -97,7 +97,7 @@ typedef struct aggr_cpu_id (*aggr_cpu_id_get_t)(struct perf_cpu cpu, void *data) > */ > struct cpu_aggr_map *cpu_aggr_map__new(const struct perf_cpu_map *cpus, > aggr_cpu_id_get_t get_id, > - void *data); > + void *data, bool needs_sort); > > bool aggr_cpu_id__equal(const struct aggr_cpu_id *a, const struct aggr_cpu_id *b); > bool aggr_cpu_id__is_empty(const struct aggr_cpu_id *a); > -- > 2.38.0.rc1.362.ged0d419d3c-goog >
| |