Re: [PATCH v2 7/7] perf expr: Migrate expr ids table to a hashmap

[Jiri Olsa]

On Fri, May 15, 2020 at 09:50:07AM -0700, Ian Rogers wrote:

SNIP

> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index b071df373f8b..37be5a368d6e 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -85,8 +85,7 @@ static void metricgroup__rblist_init(struct rblist *metric_events)
>  
>  struct egroup {
>  	struct list_head nd;
> -	int idnum;
> -	const char **ids;
> +	struct expr_parse_ctx pctx;
>  	const char *metric_name;
>  	const char *metric_expr;
>  	const char *metric_unit;
> @@ -94,19 +93,21 @@ struct egroup {
>  };
>  
>  static struct evsel *find_evsel_group(struct evlist *perf_evlist,
> -				      const char **ids,
> -				      int idnum,
> +				      struct expr_parse_ctx *pctx,
>  				      struct evsel **metric_events,
>  				      bool *evlist_used)
>  {
>  	struct evsel *ev;
> -	int i = 0, j = 0;
>  	bool leader_found;
> +	const size_t idnum = hashmap__size(&pctx->ids);
> +	size_t i = 0;
> +	int j = 0;
> +	double *val_ptr;
>  
>  	evlist__for_each_entry (perf_evlist, ev) {
>  		if (evlist_used[j++])
>  			continue;
> -		if (!strcmp(ev->name, ids[i])) {
> +		if (hashmap__find(&pctx->ids, ev->name, (void **)&val_ptr)) {

hum, you sure it's doing the same thing as before?

hashmap__find will succede all the time in here, while the
previous code was looking for the start of the group ...
the logic in here is little convoluted, so maybe I'm
missing some point in here ;-)

jirka

>  			if (!metric_events[i])
>  				metric_events[i] = ev;
>  			i++;
> @@ -118,7 +119,8 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
>  			memset(metric_events, 0,
>  				sizeof(struct evsel *) * idnum);
>  
> -			if (!strcmp(ev->name, ids[i])) {
> +			if (hashmap__find(&pctx->ids, ev->name,
> +					  (void **)&val_ptr)) {
>  				if (!metric_events[i])
>  					metric_events[i] = ev;

SNIP