All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Garry <john.g.garry@oracle.com>
To: Ian Rogers <irogers@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Sumanth Korikkar <sumanthk@linux.ibm.com>,
	Thomas Richter <tmricht@linux.ibm.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
Cc: Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH v3] perf jevents: Parse metrics during conversion
Date: Mon, 5 Dec 2022 15:24:12 +0000	[thread overview]
Message-ID: <61e2f5b7-e332-08d1-7ffb-404cecd23ecb@oracle.com> (raw)
In-Reply-To: <20221201034138.417972-1-irogers@google.com>

On 01/12/2022 03:41, Ian Rogers wrote:
> Currently the 'MetricExpr' json value is passed from the json
> file to the pmu-events.c. This change introduces an expression
> tree that is parsed into. The parsing is done largely by using
> operator overloading and python's 'eval' function. Two advantages
> in doing this are:
> 
> 1) Broken metrics fail at compile time rather than relying on
>     `perf test` to detect. `perf test` remains relevant for checking
>     event encoding and actual metric use.

Do we still require the code to "resolve metrics" in resolve_metric()? 
But I'm not sure it even ever had any users.

> 
> 2) The conversion to a string from the tree can minimize the metric's
>     string size, for example, preferring 1e6 over 1000000, avoiding
>     multiplication by 1 and removing unnecessary whitespace. On x86
>     this reduces the string size by 3,050bytes (0.07%).

Out of curiosity, did you try the exponent change on its own (to see the 
impact on size)?

Nit:

Unrelated, really, I notice that sometimes we lose the parenthesis and 
sometimes never had them, like:

/* offset=11526 */ "\000\000metrics\000Ave [...] 0\000( 1000000000 * ( 
UNC_CHA
/* offset=11207 */ "\000\000metrics\000Ave [...] 0\0001e9 * (UNC_CHA_TOR

To me, it seems neater to have the expression contained within (a 
parenthesis) ever since we moved to this "big string". This seems to be 
a preexisting feature.

Thanks,
John


> 
> In future changes it would be possible to programmatically
> generate the json expressions (a single line of text and so a
> pain to write manually) for an architecture using the expression
> tree. This could avoid copy-pasting metrics for all architecture
> variants.
> 
> Signed-off-by: Ian Rogers<irogers@google.com>


  parent reply	other threads:[~2022-12-05 15:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-01  3:41 [PATCH v3] perf jevents: Parse metrics during conversion Ian Rogers
2022-12-05 12:53 ` Arnaldo Carvalho de Melo
2022-12-05 12:56   ` John Garry
2022-12-05 15:24 ` John Garry [this message]
2022-12-06 19:29   ` Ian Rogers
2022-12-07 11:21     ` John Garry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=61e2f5b7-e332-08d1-7ffb-404cecd23ecb@oracle.com \
    --to=john.g.garry@oracle.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=eranian@google.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=sumanthk@linux.ibm.com \
    --cc=tmricht@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.