Messages in this thread | | | From | Arnaldo Carvalho de Melo <> | Date | Tue, 10 Mar 2020 14:45:38 -0300 | Subject | Re: [PATCH V2 2/5] perf metricgroup: Factor out metricgroup__add_metric_weak_group() |
| |
Em Mon, Feb 24, 2020 at 01:59:21PM -0800, kan.liang@linux.intel.com escreveu: > From: Kan Liang <kan.liang@linux.intel.com> > > Factor out metricgroup__add_metric_weak_group() which add metrics into a > weak group. The change can improve code readability. Because following > patch will introduce a function which add standalone metrics. > > Signed-off-by: Kan Liang <kan.liang@linux.intel.com> > --- > tools/perf/util/metricgroup.c | 57 +++++++++++++++++++++++++------------------ > 1 file changed, 33 insertions(+), 24 deletions(-) > > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c > index 02aee94..1cd042c 100644 > --- a/tools/perf/util/metricgroup.c > +++ b/tools/perf/util/metricgroup.c > @@ -399,13 +399,42 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter, > strlist__delete(metriclist); > } > > +static void metricgroup__add_metric_weak_group(struct strbuf *events, > + const char **ids, > + int idnum) > +{ > + bool no_group = false; > + int i; > + > + for (i = 0; i < idnum; i++) { > + pr_debug("found event %s\n", ids[i]); > + /* > + * Duration time maps to a software event and can make > + * groups not count. Always use it outside a > + * group. > + */ > + if (!strcmp(ids[i], "duration_time")) { > + if (i > 0) > + strbuf_addf(events, "}:W,"); > + strbuf_addf(events, "duration_time");
At some point we need to do some error checking on these strbuf calls, but since this was the state in this file, lets not block this patchkit on such grounds,
- Arnaldo
> + no_group = true; > + continue; > + } > + strbuf_addf(events, "%s%s", > + i == 0 || no_group ? "{" : ",", > + ids[i]); > + no_group = false; > + } > + if (!no_group) > + strbuf_addf(events, "}:W"); > +} > + > static int metricgroup__add_metric(const char *metric, struct strbuf *events, > struct list_head *group_list) > { > struct pmu_events_map *map = perf_pmu__find_map(NULL); > struct pmu_event *pe; > - int ret = -EINVAL; > - int i, j; > + int i, ret = -EINVAL; > > if (!map) > return 0; > @@ -422,7 +451,6 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events, > const char **ids; > int idnum; > struct egroup *eg; > - bool no_group = false; > > pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name); > > @@ -431,27 +459,8 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events, > continue; > if (events->len > 0) > strbuf_addf(events, ","); > - for (j = 0; j < idnum; j++) { > - pr_debug("found event %s\n", ids[j]); > - /* > - * Duration time maps to a software event and can make > - * groups not count. Always use it outside a > - * group. > - */ > - if (!strcmp(ids[j], "duration_time")) { > - if (j > 0) > - strbuf_addf(events, "}:W,"); > - strbuf_addf(events, "duration_time"); > - no_group = true; > - continue; > - } > - strbuf_addf(events, "%s%s", > - j == 0 || no_group ? "{" : ",", > - ids[j]); > - no_group = false; > - } > - if (!no_group) > - strbuf_addf(events, "}:W"); > + > + metricgroup__add_metric_weak_group(events, ids, idnum); > > eg = malloc(sizeof(struct egroup)); > if (!eg) { > -- > 2.7.4 >
--
- Arnaldo
| |