Re: [PATCH 09/10] perf tools: Compute other metrics

[Jiri Olsa <jolsa@redhat.com>]

On Fri, Jun 26, 2020 at 02:24:38PM -0700, Ian Rogers wrote:

SNIP

> > +
> > +                                       if (expr__get_id(ctx, lookup, &data) || !data) {
> >                                                 pr_debug("%s not found\n", $1);
> >                                                 free($1);
> >                                                 YYABORT;
> >                                         }
> > +
> > +                                       pr_debug2("lookup: is_other %d, counted %d: %s\n",
> > +                                                 data->is_other, data->other.counted, lookup);
> > +
> > +                                       if (data->is_other && !data->other.counted) {
> > +                                               data->other.counted = true;
> > +                                               if (expr__parse(&data->val, ctx, data->other.metric_expr, 1)) {
> 
> Ah, so this handles the problem the referenced metric isn't calculated
> and calculates it - with the sharing of events this doesn't impose
> extra pmu cost. Do we need to worry about detecting recursion? For
> example:
> 
> "MetricName": "Foo",
> "MetricExpr": "1/metric:Foo",

right, we should add some recursion check,
I'll lcheck on it

> 
> It seems unfortunate to have the MetricExpr calculated twice, but it

hum, not sure what you mean by twice.. we do that just once for
each involved metric and store the value.. the metric is also
processed before for 'other' metrics

jirka

> is understandable. Is it also a property that referenced/other metrics
> won't be reported individually? Perhaps these are sub-metrics?

> 
> Thanks,
> Ian
> 
> > +                                                       pr_debug("%s failed to count\n", $1);
> > +                                                       free($1);
> > +                                                       YYABORT;
> > +                                               }
> > +                                       }
> > +
> >                                         $$ = data->val;
> >                                         free($1);
> >                                 }
> > --
> > 2.25.4
> >
>