| Message ID | 20220328062414.1893550-4-irogers@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Make evlist CPUs more accurate | expand |
Em Sun, Mar 27, 2022 at 11:24:12PM -0700, Ian Rogers escreveu: > The merge function gives the union of two cpu maps. Add an intersect > function which will be used in the next change. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/lib/perf/cpumap.c | 38 ++++++++++++++++++++++++++++ > tools/lib/perf/include/perf/cpumap.h | 2 ++ > 2 files changed, 40 insertions(+) > > diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c > index 953bc50b0e41..56b4d213039f 100644 > --- a/tools/lib/perf/cpumap.c > +++ b/tools/lib/perf/cpumap.c > @@ -393,3 +393,41 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > perf_cpu_map__put(orig); > return merged; > } > + > +struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig, > + struct perf_cpu_map *other) > +{ > + struct perf_cpu *tmp_cpus; > + int tmp_len; > + int i, j, k; > + struct perf_cpu_map *merged = NULL; > + > + if (perf_cpu_map__is_subset(other, orig)) > + return orig; > + if (perf_cpu_map__is_subset(orig, other)) { > + perf_cpu_map__put(orig); Why this put(orig)? > + return perf_cpu_map__get(other); And why the get here and not on the first if? > + } > + > + tmp_len = max(orig->nr, other->nr); > + tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu)); > + if (!tmp_cpus) > + return NULL; > + > + i = j = k = 0; > + while (i < orig->nr && j < other->nr) { > + if (orig->map[i].cpu < other->map[j].cpu) > + i++; > + else if (orig->map[i].cpu > other->map[j].cpu) > + j++; > + else { > + j++; > + tmp_cpus[k++] = orig->map[i++]; > + } > + } > + if (k) > + merged = cpu_map__trim_new(k, tmp_cpus); > + free(tmp_cpus); > + perf_cpu_map__put(orig); > + return merged; > +} > diff --git a/tools/lib/perf/include/perf/cpumap.h b/tools/lib/perf/include/perf/cpumap.h > index 4a2edbdb5e2b..a2a7216c0b78 100644 > --- a/tools/lib/perf/include/perf/cpumap.h > +++ b/tools/lib/perf/include/perf/cpumap.h > @@ -19,6 +19,8 @@ LIBPERF_API struct perf_cpu_map *perf_cpu_map__read(FILE *file); > LIBPERF_API struct perf_cpu_map *perf_cpu_map__get(struct perf_cpu_map *map); > LIBPERF_API struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > struct perf_cpu_map *other); > +LIBPERF_API struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig, > + struct perf_cpu_map *other); > LIBPERF_API void perf_cpu_map__put(struct perf_cpu_map *map); > LIBPERF_API struct perf_cpu perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx); > LIBPERF_API int perf_cpu_map__nr(const struct perf_cpu_map *cpus); > -- > 2.35.1.1021.g381101b075-goog
On Mon, Mar 28, 2022 at 1:28 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Sun, Mar 27, 2022 at 11:24:12PM -0700, Ian Rogers escreveu: > > The merge function gives the union of two cpu maps. Add an intersect > > function which will be used in the next change. > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/lib/perf/cpumap.c | 38 ++++++++++++++++++++++++++++ > > tools/lib/perf/include/perf/cpumap.h | 2 ++ > > 2 files changed, 40 insertions(+) > > > > diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c > > index 953bc50b0e41..56b4d213039f 100644 > > --- a/tools/lib/perf/cpumap.c > > +++ b/tools/lib/perf/cpumap.c > > @@ -393,3 +393,41 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > > perf_cpu_map__put(orig); > > return merged; > > } > > + > > +struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig, > > + struct perf_cpu_map *other) > > +{ > > + struct perf_cpu *tmp_cpus; > > + int tmp_len; > > + int i, j, k; > > + struct perf_cpu_map *merged = NULL; > > + > > + if (perf_cpu_map__is_subset(other, orig)) > > + return orig; > > + if (perf_cpu_map__is_subset(orig, other)) { > > + perf_cpu_map__put(orig); > > Why this put(orig)? As with merge, if orig isn't returned then it is put. > > + return perf_cpu_map__get(other); > > And why the get here and not on the first if? The first argument orig is either put or returned while the second may be returned only if the reference count is incremented. We could change the API for merge and intersect to put both arguments, or to not put either argument. Thanks, Ian > > + } > > + > > + tmp_len = max(orig->nr, other->nr); > > + tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu)); > > + if (!tmp_cpus) > > + return NULL; > > + > > + i = j = k = 0; > > + while (i < orig->nr && j < other->nr) { > > + if (orig->map[i].cpu < other->map[j].cpu) > > + i++; > > + else if (orig->map[i].cpu > other->map[j].cpu) > > + j++; > > + else { > > + j++; > > + tmp_cpus[k++] = orig->map[i++]; > > + } > > + } > > + if (k) > > + merged = cpu_map__trim_new(k, tmp_cpus); > > + free(tmp_cpus); > > + perf_cpu_map__put(orig); > > + return merged; > > +} > > diff --git a/tools/lib/perf/include/perf/cpumap.h b/tools/lib/perf/include/perf/cpumap.h > > index 4a2edbdb5e2b..a2a7216c0b78 100644 > > --- a/tools/lib/perf/include/perf/cpumap.h > > +++ b/tools/lib/perf/include/perf/cpumap.h > > @@ -19,6 +19,8 @@ LIBPERF_API struct perf_cpu_map *perf_cpu_map__read(FILE *file); > > LIBPERF_API struct perf_cpu_map *perf_cpu_map__get(struct perf_cpu_map *map); > > LIBPERF_API struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > > struct perf_cpu_map *other); > > +LIBPERF_API struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig, > > + struct perf_cpu_map *other); > > LIBPERF_API void perf_cpu_map__put(struct perf_cpu_map *map); > > LIBPERF_API struct perf_cpu perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx); > > LIBPERF_API int perf_cpu_map__nr(const struct perf_cpu_map *cpus); > > -- > > 2.35.1.1021.g381101b075-goog > > -- > > - Arnaldo
On March 28, 2022 5:54:06 PM GMT-03:00, Ian Rogers <irogers@google.com> wrote: >On Mon, Mar 28, 2022 at 1:28 PM Arnaldo Carvalho de Melo ><acme@kernel.org> wrote: >> >> Em Sun, Mar 27, 2022 at 11:24:12PM -0700, Ian Rogers escreveu: >> > The merge function gives the union of two cpu maps. Add an intersect >> > function which will be used in the next change. >> > >> > Signed-off-by: Ian Rogers <irogers@google.com> >> > --- >> > tools/lib/perf/cpumap.c | 38 ++++++++++++++++++++++++++++ >> > tools/lib/perf/include/perf/cpumap.h | 2 ++ >> > 2 files changed, 40 insertions(+) >> > >> > diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c >> > index 953bc50b0e41..56b4d213039f 100644 >> > --- a/tools/lib/perf/cpumap.c >> > +++ b/tools/lib/perf/cpumap.c >> > @@ -393,3 +393,41 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, >> > perf_cpu_map__put(orig); >> > return merged; >> > } >> > + >> > +struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig, >> > + struct perf_cpu_map *other) >> > +{ >> > + struct perf_cpu *tmp_cpus; >> > + int tmp_len; >> > + int i, j, k; >> > + struct perf_cpu_map *merged = NULL; >> > + >> > + if (perf_cpu_map__is_subset(other, orig)) >> > + return orig; >> > + if (perf_cpu_map__is_subset(orig, other)) { >> > + perf_cpu_map__put(orig); >> >> Why this put(orig)? > >As with merge, if orig isn't returned then it is put. For merge I can see it dropping a reference, i.e. get b and merge it into a, after that b was "consumed" But for intersect? > >> > + return perf_cpu_map__get(other); >> >> And why the get here and not on the first if? > >The first argument orig is either put or returned while the second may >be returned only if the reference count is incremented. We could >change the API for merge and intersect to put both arguments, or to >not put either argument. > >Thanks, >Ian > >> > + } >> > + >> > + tmp_len = max(orig->nr, other->nr); >> > + tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu)); >> > + if (!tmp_cpus) >> > + return NULL; >> > + >> > + i = j = k = 0; >> > + while (i < orig->nr && j < other->nr) { >> > + if (orig->map[i].cpu < other->map[j].cpu) >> > + i++; >> > + else if (orig->map[i].cpu > other->map[j].cpu) >> > + j++; >> > + else { >> > + j++; >> > + tmp_cpus[k++] = orig->map[i++]; >> > + } >> > + } >> > + if (k) >> > + merged = cpu_map__trim_new(k, tmp_cpus); >> > + free(tmp_cpus); >> > + perf_cpu_map__put(orig); >> > + return merged; >> > +} >> > diff --git a/tools/lib/perf/include/perf/cpumap.h b/tools/lib/perf/include/perf/cpumap.h >> > index 4a2edbdb5e2b..a2a7216c0b78 100644 >> > --- a/tools/lib/perf/include/perf/cpumap.h >> > +++ b/tools/lib/perf/include/perf/cpumap.h >> > @@ -19,6 +19,8 @@ LIBPERF_API struct perf_cpu_map *perf_cpu_map__read(FILE *file); >> > LIBPERF_API struct perf_cpu_map *perf_cpu_map__get(struct perf_cpu_map *map); >> > LIBPERF_API struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, >> > struct perf_cpu_map *other); >> > +LIBPERF_API struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig, >> > + struct perf_cpu_map *other); >> > LIBPERF_API void perf_cpu_map__put(struct perf_cpu_map *map); >> > LIBPERF_API struct perf_cpu perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx); >> > LIBPERF_API int perf_cpu_map__nr(const struct perf_cpu_map *cpus); >> > -- >> > 2.35.1.1021.g381101b075-goog >> >> -- >> >> - Arnaldo
On Mon, Mar 28, 2022 at 2:00 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > > > On March 28, 2022 5:54:06 PM GMT-03:00, Ian Rogers <irogers@google.com> wrote: > >On Mon, Mar 28, 2022 at 1:28 PM Arnaldo Carvalho de Melo > ><acme@kernel.org> wrote: > >> > >> Em Sun, Mar 27, 2022 at 11:24:12PM -0700, Ian Rogers escreveu: > >> > The merge function gives the union of two cpu maps. Add an intersect > >> > function which will be used in the next change. > >> > > >> > Signed-off-by: Ian Rogers <irogers@google.com> > >> > --- > >> > tools/lib/perf/cpumap.c | 38 ++++++++++++++++++++++++++++ > >> > tools/lib/perf/include/perf/cpumap.h | 2 ++ > >> > 2 files changed, 40 insertions(+) > >> > > >> > diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c > >> > index 953bc50b0e41..56b4d213039f 100644 > >> > --- a/tools/lib/perf/cpumap.c > >> > +++ b/tools/lib/perf/cpumap.c > >> > @@ -393,3 +393,41 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > >> > perf_cpu_map__put(orig); > >> > return merged; > >> > } > >> > + > >> > +struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig, > >> > + struct perf_cpu_map *other) > >> > +{ > >> > + struct perf_cpu *tmp_cpus; > >> > + int tmp_len; > >> > + int i, j, k; > >> > + struct perf_cpu_map *merged = NULL; > >> > + > >> > + if (perf_cpu_map__is_subset(other, orig)) > >> > + return orig; > >> > + if (perf_cpu_map__is_subset(orig, other)) { > >> > + perf_cpu_map__put(orig); > >> > >> Why this put(orig)? > > > >As with merge, if orig isn't returned then it is put. > > For merge I can see it dropping a reference, i.e. get b and merge it into a, after that b was "consumed" > > But for intersect? The current use case is the intersect of all online CPUs with the merge of all CPU maps from evsels. So we can generally just reuse all_cpus, or the common case of both maps contain every CPU. I think the pattern makes code like: evlist->cpus = perf_cpu_map__intersect(evlist->cpus, other); not quite as messy, as without the put you need: tmp = perf_cpu_map__intersect(evlist->cpus, other); perf_cpu_map__put(evlist->cpus); evlist->cpus = tmp; I'm somewhat agnostic on what the API should be, but it'd be nice if merge and intersect behaved in a similar way. Thanks, Ian > > > >> > + return perf_cpu_map__get(other); > >> > >> And why the get here and not on the first if? > > > >The first argument orig is either put or returned while the second may > >be returned only if the reference count is incremented. We could > >change the API for merge and intersect to put both arguments, or to > >not put either argument. > > > >Thanks, > >Ian > > > >> > + } > >> > + > >> > + tmp_len = max(orig->nr, other->nr); > >> > + tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu)); > >> > + if (!tmp_cpus) > >> > + return NULL; > >> > + > >> > + i = j = k = 0; > >> > + while (i < orig->nr && j < other->nr) { > >> > + if (orig->map[i].cpu < other->map[j].cpu) > >> > + i++; > >> > + else if (orig->map[i].cpu > other->map[j].cpu) > >> > + j++; > >> > + else { > >> > + j++; > >> > + tmp_cpus[k++] = orig->map[i++]; > >> > + } > >> > + } > >> > + if (k) > >> > + merged = cpu_map__trim_new(k, tmp_cpus); > >> > + free(tmp_cpus); > >> > + perf_cpu_map__put(orig); > >> > + return merged; > >> > +} > >> > diff --git a/tools/lib/perf/include/perf/cpumap.h b/tools/lib/perf/include/perf/cpumap.h > >> > index 4a2edbdb5e2b..a2a7216c0b78 100644 > >> > --- a/tools/lib/perf/include/perf/cpumap.h > >> > +++ b/tools/lib/perf/include/perf/cpumap.h > >> > @@ -19,6 +19,8 @@ LIBPERF_API struct perf_cpu_map *perf_cpu_map__read(FILE *file); > >> > LIBPERF_API struct perf_cpu_map *perf_cpu_map__get(struct perf_cpu_map *map); > >> > LIBPERF_API struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, > >> > struct perf_cpu_map *other); > >> > +LIBPERF_API struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig, > >> > + struct perf_cpu_map *other); > >> > LIBPERF_API void perf_cpu_map__put(struct perf_cpu_map *map); > >> > LIBPERF_API struct perf_cpu perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx); > >> > LIBPERF_API int perf_cpu_map__nr(const struct perf_cpu_map *cpus); > >> > -- > >> > 2.35.1.1021.g381101b075-goog > >> > >> -- > >> > >> - Arnaldo
diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c index 953bc50b0e41..56b4d213039f 100644 --- a/tools/lib/perf/cpumap.c +++ b/tools/lib/perf/cpumap.c @@ -393,3 +393,41 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, perf_cpu_map__put(orig); return merged; } + +struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig, + struct perf_cpu_map *other) +{ + struct perf_cpu *tmp_cpus; + int tmp_len; + int i, j, k; + struct perf_cpu_map *merged = NULL; + + if (perf_cpu_map__is_subset(other, orig)) + return orig; + if (perf_cpu_map__is_subset(orig, other)) { + perf_cpu_map__put(orig); + return perf_cpu_map__get(other); + } + + tmp_len = max(orig->nr, other->nr); + tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu)); + if (!tmp_cpus) + return NULL; + + i = j = k = 0; + while (i < orig->nr && j < other->nr) { + if (orig->map[i].cpu < other->map[j].cpu) + i++; + else if (orig->map[i].cpu > other->map[j].cpu) + j++; + else { + j++; + tmp_cpus[k++] = orig->map[i++]; + } + } + if (k) + merged = cpu_map__trim_new(k, tmp_cpus); + free(tmp_cpus); + perf_cpu_map__put(orig); + return merged; +} diff --git a/tools/lib/perf/include/perf/cpumap.h b/tools/lib/perf/include/perf/cpumap.h index 4a2edbdb5e2b..a2a7216c0b78 100644 --- a/tools/lib/perf/include/perf/cpumap.h +++ b/tools/lib/perf/include/perf/cpumap.h @@ -19,6 +19,8 @@ LIBPERF_API struct perf_cpu_map *perf_cpu_map__read(FILE *file); LIBPERF_API struct perf_cpu_map *perf_cpu_map__get(struct perf_cpu_map *map); LIBPERF_API struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig, struct perf_cpu_map *other); +LIBPERF_API struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig, + struct perf_cpu_map *other); LIBPERF_API void perf_cpu_map__put(struct perf_cpu_map *map); LIBPERF_API struct perf_cpu perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx); LIBPERF_API int perf_cpu_map__nr(const struct perf_cpu_map *cpus);
The merge function gives the union of two cpu maps. Add an intersect function which will be used in the next change. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/lib/perf/cpumap.c | 38 ++++++++++++++++++++++++++++ tools/lib/perf/include/perf/cpumap.h | 2 ++ 2 files changed, 40 insertions(+)