From: Ian Rogers Date: Fri, 15 Oct 2021 17:21:23 +0000 (-0700) Subject: perf metric: Simplify metric_refs calculation X-Git-Tag: block-5.16-2021-11-13~30^2~66 X-Git-Url: https://git.kernel.dk/?a=commitdiff_plain;h=46bdc0bf8d21;p=linux-block.git perf metric: Simplify metric_refs calculation Don't build a list and then turn to an array, just directly build the array. The size of the array is known due to the search for a duplicate. Signed-off-by: Ian Rogers Acked-by: Andi Kleen Cc: Adrian Hunter Cc: Alexander Antonov Cc: Alexander Shishkin Cc: Andrew Kilroy Cc: Andrew Morton Cc: Changbin Du Cc: Denys Zagorui Cc: Fabian Hemmer Cc: Felix Fietkau Cc: Heiko Carstens Cc: Ingo Molnar Cc: Jacob Keller Cc: Jiapeng Chong Cc: Jin Yao Cc: Jiri Olsa Cc: Joakim Zhang Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Kees Kook Cc: Mark Rutland Cc: Namhyung Kim Cc: Nicholas Fraser Cc: Nick Desaulniers Cc: Paul Clarke Cc: Peter Zijlstra Cc: Riccardo Mancini Cc: Sami Tolvanen Cc: ShihCheng Tu Cc: Song Liu Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Thomas Richter Cc: Wan Jiabing Cc: Zhen Lei Link: https://lore.kernel.org/r/20211015172132.1162559-13-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c index 632867cedbae..b48836d7c080 100644 --- a/tools/perf/util/metricgroup.c +++ b/tools/perf/util/metricgroup.c @@ -137,10 +137,8 @@ struct metric { * output. */ const char *metric_unit; - /** The list of metrics referenced by this one. */ - struct list_head metric_refs; - /** The size of the metric_refs list. */ - int metric_refs_cnt; + /** Optional null terminated array of referenced metrics. */ + struct metric_ref *metric_refs; /** * Is there a constraint on the group of events? In which case the * events won't be grouped. @@ -202,20 +200,14 @@ static struct metric *metric__new(const struct pmu_event *pe, m->metric_unit = pe->unit; m->pctx->runtime = runtime; m->has_constraint = metric_no_group || metricgroup__has_constraint(pe); - INIT_LIST_HEAD(&m->metric_refs); - m->metric_refs_cnt = 0; + m->metric_refs = NULL; return m; } static void metric__free(struct metric *m) { - struct metric_ref_node *ref, *tmp; - - list_for_each_entry_safe(ref, tmp, &m->metric_refs, list) { - list_del(&ref->list); - free(ref); - } + free(m->metric_refs); expr__ctx_free(m->pctx); free(m); } @@ -393,7 +385,6 @@ static int metricgroup__setup_events(struct list_head *groups, list_for_each_entry (m, groups, nd) { struct evsel **metric_events; - struct metric_ref *metric_refs = NULL; const size_t ids_size = hashmap__size(m->pctx->ids); metric_events = calloc(sizeof(void *), @@ -427,36 +418,8 @@ static int metricgroup__setup_events(struct list_head *groups, break; } - /* - * Collect and store collected nested expressions - * for metric processing. - */ - if (m->metric_refs_cnt) { - struct metric_ref_node *ref; - - metric_refs = zalloc(sizeof(struct metric_ref) * (m->metric_refs_cnt + 1)); - if (!metric_refs) { - ret = -ENOMEM; - free(metric_events); - free(expr); - break; - } - - i = 0; - list_for_each_entry(ref, &m->metric_refs, list) { - /* - * Intentionally passing just const char pointers, - * originally from 'struct pmu_event' object. - * We don't need to change them, so there's no - * need to create our own copy. - */ - metric_refs[i].metric_name = ref->metric_name; - metric_refs[i].metric_expr = ref->metric_expr; - i++; - } - } - - expr->metric_refs = metric_refs; + expr->metric_refs = m->metric_refs; + m->metric_refs = NULL; expr->metric_expr = m->metric_expr; expr->metric_name = m->metric_name; expr->metric_unit = m->metric_unit; @@ -936,7 +899,6 @@ static int __add_metric(struct list_head *metric_list, const struct visited_metric *visited, const struct pmu_events_map *map) { - struct metric_ref_node *ref; const struct visited_metric *vm; int ret; bool is_root = !root_metric; @@ -962,19 +924,25 @@ static int __add_metric(struct list_head *metric_list, return -ENOMEM; } else { + int cnt = 0; + /* * This metric was referenced in a metric higher in the * tree. Check if the same metric is already resolved in the * metric_refs list. */ - list_for_each_entry(ref, &root_metric->metric_refs, list) { - if (!strcmp(pe->metric_name, ref->metric_name)) - return 0; + if (root_metric->metric_refs) { + for (; root_metric->metric_refs[cnt].metric_name; cnt++) { + if (!strcmp(pe->metric_name, + root_metric->metric_refs[cnt].metric_name)) + return 0; + } } - /* Create reference */ - ref = malloc(sizeof(*ref)); - if (!ref) + /* Create reference. Need space for the entry and the terminator. */ + root_metric->metric_refs = realloc(root_metric->metric_refs, + (cnt + 2) * sizeof(struct metric_ref)); + if (!root_metric->metric_refs) return -ENOMEM; /* @@ -983,11 +951,12 @@ static int __add_metric(struct list_head *metric_list, * need to change them, so there's no need to create * our own copy. */ - ref->metric_name = pe->metric_name; - ref->metric_expr = pe->metric_expr; + root_metric->metric_refs[cnt].metric_name = pe->metric_name; + root_metric->metric_refs[cnt].metric_expr = pe->metric_expr; - list_add(&ref->list, &root_metric->metric_refs); - root_metric->metric_refs_cnt++; + /* Null terminate array. */ + root_metric->metric_refs[cnt+1].metric_name = NULL; + root_metric->metric_refs[cnt+1].metric_expr = NULL; } /*