lkml.org 
[lkml]   [2020]   [May]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/4] perf expr: Add parsing support for multiple expressions
On Tue, May 12, 2020 at 11:50:18PM -0700, Ian Rogers wrote:
> On Mon, May 11, 2020 at 1:54 PM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding support to parse metric difinitions in following form:
>
> Typo on definitions.

right

SNIP

> > +int expr__parse_custom(struct expr_parse_ctx *ctx, const char *expr)
> > +{
> > + return __expr__parse(NULL, ctx, expr, EXPR_CUSTOM, 0);
> > +}
> > +
> > static bool
> > already_seen(const char *val, const char *one, const char **other,
> > int num_other)
> > diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
> > index 40fc452b0f2b..ef116b58a5d4 100644
> > --- a/tools/perf/util/expr.h
> > +++ b/tools/perf/util/expr.h
> > @@ -4,15 +4,29 @@
> >
> > #define EXPR_MAX_OTHER 64
> > #define MAX_PARSE_ID EXPR_MAX_OTHER
> > +#define EXPR_MAX 20
>
> Currently deduplication of ids is done after rather than during
> expression pasing, meaning hitting these limits is quite easy. This is
> fixed in:
> https://lore.kernel.org/lkml/20200508053629.210324-8-irogers@google.com/
> But not for custom expressions being added here. I plan to rebase that
> work and clone hashmap from libbpf into libapi to workaround the
> dependency issue.
> That patch also adds expr__ctx_clear as a convenience for cleaning up
> the context, and passes the context around inside of metricgroup
> rather than ids.

ok

SNIP

> > symbol ({spec}|{sym})+
> > +all [^;]+
> >
> > %%
> > struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
> > @@ -100,6 +103,12 @@ symbol ({spec}|{sym})+
> > }
> > }
> >
> > +<custom>{
> > +
> > +{all} { BEGIN(INITIAL); return str(yyscanner, ALL, sctx->runtime); }
> > +
> > +}
> > +
> > max { return MAX; }
> > min { return MIN; }
> > if { return IF; }
> > @@ -118,6 +127,9 @@ else { return ELSE; }
> > "(" { return '('; }
> > ")" { return ')'; }
> > "," { return ','; }
> > +";" { return ';'; }
> > +"=" { BEGIN(custom); return '='; }
>
> Will this interfere with the \\= encoded in MetricExpr? Could you test with:
> https://lore.kernel.org/lkml/20200513062752.3681-2-irogers@google.com/

it shouldn't, the escape is matched first.. but I'll put it on top
of the new tests to be sure

>
> > +\n { }
> > . { }
> > %%
> >
> > diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
> > index 21e82a1e11a2..0521e48fa5e3 100644
> > --- a/tools/perf/util/expr.y
> > +++ b/tools/perf/util/expr.y
> > @@ -24,9 +24,10 @@
> > char *str;
> > }
> >
> > -%token EXPR_PARSE EXPR_OTHER EXPR_ERROR
> > +%token EXPR_PARSE EXPR_OTHER EXPR_CUSTOM EXPR_ERROR
> > %token <num> NUMBER
> > %token <str> ID
> > +%token <str> ALL
>
> Missing %destructor, fix is here:
> https://lore.kernel.org/lkml/20200513000318.15166-1-irogers@google.com/

oops, right.. thanks

jirka

\
 
 \ /
  Last update: 2020-05-13 13:26    [W:0.087 / U:0.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site