Re: [PATCH v5 04/13] perf evlist: Maintain evlist->all_cpus

[Jiri Olsa]

On Thu, Nov 07, 2019 at 10:16:37AM -0800, Andi Kleen wrote:

SNIP

> diff --git a/tools/perf/lib/cpumap.c b/tools/perf/lib/cpumap.c
> index d81656b4635e..b9f573438b93 100644
> --- a/tools/perf/lib/cpumap.c
> +++ b/tools/perf/lib/cpumap.c
> @@ -286,3 +286,46 @@ int perf_cpu_map__max(struct perf_cpu_map *map)
>  
>  	return max;
>  }
> +
> +struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig,
> +					 struct perf_cpu_map *other)
> +{
> +	int *tmp_cpus;
> +	int tmp_len;
> +	int i, j, k;
> +	struct perf_cpu_map *merged;
> +
> +	if (!orig && !other)
> +		return NULL;
> +	if (!orig) {
> +		perf_cpu_map__get(other);
> +		return other;
> +	}
> +	if (!other)
> +		return orig;

so we bump refcnt for other but not for orig?

could you please put to the comment expected results
wrt refcnt values for above cases?

> +	if (orig->nr == other->nr &&
> +	    !memcmp(orig->map, other->map, orig->nr * sizeof(int)))
> +		return orig;
> +	tmp_len = orig->nr + other->nr;
> +	tmp_cpus = malloc(tmp_len * sizeof(int));
> +	if (!tmp_cpus)
> +		return NULL;
> +	i = j = k = 0;
> +	while (i < orig->nr && j < other->nr) {
> +		if (orig->map[i] <= other->map[j]) {
> +			if (orig->map[i] == other->map[j])
> +				j++;
> +			tmp_cpus[k++] = orig->map[i++];
> +		} else
> +			tmp_cpus[k++] = other->map[j++];
> +	}
> +	while (i < orig->nr)
> +		tmp_cpus[k++] = orig->map[i++];
> +	while (j < other->nr)
> +		tmp_cpus[k++] = other->map[j++];
> +	assert(k <= tmp_len);
> +	merged = cpu_map__trim_new(k, tmp_cpus);
> +	free(tmp_cpus);
> +	perf_cpu_map__put(orig);
> +	return merged;

please use some comments and blank lines to separate
the code above, it's too much.. ;-)

SNIP

>  LIBPERF_API int 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);
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 8b286e9b7549..5fa37cf7f283 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -259,6 +259,11 @@ static struct test generic_tests[] = {
>  		.desc = "Print cpu map",
>  		.func = test__cpu_map_print,
>  	},
> +	{
> +		.desc = "Merge cpu map",
> +		.func = test__cpu_map_merge,
> +	},

awesome ,thanks

jirka