linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	James Clark <james.clark@linaro.org>, Xu Yang <xu.yang_2@nxp.com>,
	"Masami Hiramatsu (Google)" <mhiramat@kernel.org>,
	Collin Funk <collin.funk1@gmail.com>,
	Howard Chu <howardchu95@gmail.com>,
	Weilin Wang <weilin.wang@intel.com>,
	Andi Kleen <ak@linux.intel.com>,
	"Dr. David Alan Gilbert" <linux@treblig.org>,
	Thomas Richter <tmricht@linux.ibm.com>,
	Tiezhu Yang <yangtiezhu@loongson.cn>,
	Gautam Menghani <gautam@linux.ibm.com>,
	Thomas Falcon <thomas.falcon@intel.com>,
	Chun-Tse Shao <ctshao@google.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: Re: [PATCH v10 08/11] perf python: Add evlist compute_metric
Date: Wed, 3 Sep 2025 10:35:57 -0300	[thread overview]
Message-ID: <aLhEPS6T-Sxnujrp@x1> (raw)
In-Reply-To: <20250819013941.209033-9-irogers@google.com>

On Mon, Aug 18, 2025 at 06:39:38PM -0700, Ian Rogers wrote:
> Add a compute_metric function that computes a metric double value for a
> given evlist, metric name, CPU and thread. For example:
> ```
> >>> import perf
> >>> x = perf.parse_metrics("TopdownL1")
> >>> x.open()
> >>> x.enable()
> >>> x.disable()
> >>> x.metrics()
> ['tma_bad_speculation', 'tma_frontend_bound', 'tma_backend_bound', 'tma_retiring']
> >>> x.compute_metric('tma_bad_speculation', 0, -1)
> 0.08605342847131037
> ```

Added the following to fix the build on the still not EOLed OpenSUSE
15, ok?

- Arnaldo

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 56102034d5b8c469..47178404802f4069 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1386,7 +1386,7 @@ static int prepare_metric(const struct metric_expr *mexp,
 static PyObject *pyrf_evlist__compute_metric(struct pyrf_evlist *pevlist,
 					     PyObject *args, PyObject *kwargs)
 {
-	int ret, cpu = 0, cpu_idx, thread = 0, thread_idx;
+	int ret, cpu = 0, cpu_idx = 0, thread = 0, thread_idx = 0;
 	const char *metric;
 	struct rb_node *node;
 	struct metric_expr *mexp = NULL;

Committer notes:

Initialize thread_idx and cpu_idx to zero as albeit them not possibly
coming out unitialized from the loop as mexp would be not NULL only if
they were initialized, some older compilers don't notice that and error
with:

    GEN     /tmp/build/perf/python/perf.cpython-36m-x86_64-linux-gnu.so
  /git/perf-6.17.0-rc3/tools/perf/util/python.c: In function ‘pyrf_evlist__compute_metric’:
  /git/perf-6.17.0-rc3/tools/perf/util/python.c:1363:3: error: ‘thread_idx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     evsel__read_counter(metric_events[i], cpu_idx, thread_idx);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /git/perf-6.17.0-rc3/tools/perf/util/python.c:1389:41: note: ‘thread_idx’ was declared here
    int ret, cpu = 0, cpu_idx, thread = 0, thread_idx;
                                           ^~~~~~~~~~
  /git/perf-6.17.0-rc3/tools/perf/util/python.c:1363:3: error: ‘cpu_idx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     evsel__read_counter(metric_events[i], cpu_idx, thread_idx);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /git/perf-6.17.0-rc3/tools/perf/util/python.c:1389:20: note: ‘cpu_idx’ was declared here
    int ret, cpu = 0, cpu_idx, thread = 0, thread_idx;
                      ^~~~~~~
  /git/perf-6.17.0-rc3/tools/perf/util/python.c: At top level:
  cc1: error: unrecognized command line option ‘-Wno-cast-function-type’ [-Werror]
  cc1: all warnings being treated as errors
  error: command 'gcc' failed with exit status 1
  cp: cannot stat '/tmp/build/perf/python_ext_build/lib/perf*.so': No such file or directory

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Reviewed-by: Howard Chu <howardchu95@gmail.com>
> ---
>  tools/perf/util/python.c | 125 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 125 insertions(+)
> 
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 31089f8e5519..e0769538b8d9 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -14,6 +14,7 @@
>  #include "evlist.h"
>  #include "evsel.h"
>  #include "event.h"
> +#include "expr.h"
>  #include "print_binary.h"
>  #include "record.h"
>  #include "strbuf.h"
> @@ -1330,6 +1331,124 @@ static PyObject *pyrf_evlist__metrics(struct pyrf_evlist *pevlist)
>  	return list;
>  }
>  
> +static int prepare_metric(const struct metric_expr *mexp,
> +			  const struct evsel *evsel,
> +			  struct expr_parse_ctx *pctx,
> +			  int cpu_idx, int thread_idx)
> +{
> +	struct evsel * const *metric_events = mexp->metric_events;
> +	struct metric_ref *metric_refs = mexp->metric_refs;
> +
> +	for (int i = 0; metric_events[i]; i++) {
> +		char *n = strdup(evsel__metric_id(metric_events[i]));
> +		double val, ena, run;
> +		int source_count = evsel__source_count(metric_events[i]);
> +		int ret;
> +		struct perf_counts_values *old_count, *new_count;
> +
> +		if (!n)
> +			return -ENOMEM;
> +
> +		if (source_count == 0)
> +			source_count = 1;
> +
> +		ret = evsel__ensure_counts(metric_events[i]);
> +		if (ret)
> +			return ret;
> +
> +		/* Set up pointers to the old and newly read counter values. */
> +		old_count = perf_counts(metric_events[i]->prev_raw_counts, cpu_idx, thread_idx);
> +		new_count = perf_counts(metric_events[i]->counts, cpu_idx, thread_idx);
> +		/* Update the value in metric_events[i]->counts. */
> +		evsel__read_counter(metric_events[i], cpu_idx, thread_idx);
> +
> +		val = new_count->val - old_count->val;
> +		ena = new_count->ena - old_count->ena;
> +		run = new_count->run - old_count->run;
> +
> +		if (ena != run && run != 0)
> +			val = val * ena / run;
> +		ret = expr__add_id_val_source_count(pctx, n, val, source_count);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (int i = 0; metric_refs && metric_refs[i].metric_name; i++) {
> +		int ret = expr__add_ref(pctx, &metric_refs[i]);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static PyObject *pyrf_evlist__compute_metric(struct pyrf_evlist *pevlist,
> +					     PyObject *args, PyObject *kwargs)
> +{
> +	int ret, cpu = 0, cpu_idx, thread = 0, thread_idx;
> +	const char *metric;
> +	struct rb_node *node;
> +	struct metric_expr *mexp = NULL;
> +	struct expr_parse_ctx *pctx;
> +	double result = 0;
> +
> +	if (!PyArg_ParseTuple(args, "sii", &metric, &cpu, &thread))
> +		return NULL;
> +
> +	for (node = rb_first_cached(&pevlist->evlist.metric_events.entries);
> +	     mexp == NULL && node;
> +	     node = rb_next(node)) {
> +		struct metric_event *me = container_of(node, struct metric_event, nd);
> +		struct list_head *pos;
> +
> +		list_for_each(pos, &me->head) {
> +			struct metric_expr *e = container_of(pos, struct metric_expr, nd);
> +
> +			if (strcmp(e->metric_name, metric))
> +				continue;
> +
> +			if (e->metric_events[0] == NULL)
> +				continue;
> +
> +			cpu_idx = perf_cpu_map__idx(e->metric_events[0]->core.cpus,
> +						    (struct perf_cpu){.cpu = cpu});
> +			if (cpu_idx < 0)
> +				continue;
> +
> +			thread_idx = perf_thread_map__idx(e->metric_events[0]->core.threads,
> +							  thread);
> +			if (thread_idx < 0)
> +				continue;
> +
> +			mexp = e;
> +			break;
> +		}
> +	}
> +	if (!mexp) {
> +		PyErr_Format(PyExc_TypeError, "Unknown metric '%s' for CPU '%d' and thread '%d'",
> +			     metric, cpu, thread);
> +		return NULL;
> +	}
> +
> +	pctx = expr__ctx_new();
> +	if (!pctx)
> +		return PyErr_NoMemory();
> +
> +	ret = prepare_metric(mexp, mexp->metric_events[0], pctx, cpu_idx, thread_idx);
> +	if (ret) {
> +		expr__ctx_free(pctx);
> +		errno = -ret;
> +		PyErr_SetFromErrno(PyExc_OSError);
> +		return NULL;
> +	}
> +	if (expr__parse(&result, pctx, mexp->metric_expr))
> +		result = 0.0;
> +
> +	expr__ctx_free(pctx);
> +	return PyFloat_FromDouble(result);
> +}
> +
>  static PyObject *pyrf_evlist__mmap(struct pyrf_evlist *pevlist,
>  				   PyObject *args, PyObject *kwargs)
>  {
> @@ -1564,6 +1683,12 @@ static PyMethodDef pyrf_evlist__methods[] = {
>  		.ml_flags = METH_NOARGS,
>  		.ml_doc	  = PyDoc_STR("List of metric names within the evlist.")
>  	},
> +	{
> +		.ml_name  = "compute_metric",
> +		.ml_meth  = (PyCFunction)pyrf_evlist__compute_metric,
> +		.ml_flags = METH_VARARGS | METH_KEYWORDS,
> +		.ml_doc	  = PyDoc_STR("compute metric for given name, cpu and thread")
> +	},
>  	{
>  		.ml_name  = "mmap",
>  		.ml_meth  = (PyCFunction)pyrf_evlist__mmap,
> -- 
> 2.51.0.rc1.167.g924127e9c0-goog

  reply	other threads:[~2025-09-03 13:36 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-19  1:39 [PATCH v10 00/11] New perf ilist app Ian Rogers
2025-08-19  1:39 ` [PATCH v10 01/11] perf python: Add more exceptions on error paths Ian Rogers
2025-08-19  1:39 ` [PATCH v10 02/11] perf python: Improve the tracepoint function if no libtraceevent Ian Rogers
2025-08-19  1:39 ` [PATCH v10 03/11] perf python: Add basic PMU abstraction and pmus sequence Ian Rogers
2025-08-19  1:39 ` [PATCH v10 04/11] perf python: Add function returning dictionary of all events on a PMU Ian Rogers
2025-08-19  1:39 ` [PATCH v10 05/11] perf ilist: Add new python ilist command Ian Rogers
2025-08-19  1:39 ` [PATCH v10 06/11] perf python: Add parse_metrics function Ian Rogers
2025-08-19  1:39 ` [PATCH v10 07/11] perf python: Add evlist metrics function Ian Rogers
2025-08-19  1:39 ` [PATCH v10 08/11] perf python: Add evlist compute_metric Ian Rogers
2025-09-03 13:35   ` Arnaldo Carvalho de Melo [this message]
2025-09-03 15:56     ` Ian Rogers
2025-08-19  1:39 ` [PATCH v10 09/11] perf python: Add metrics function Ian Rogers
2025-08-19  1:39 ` [PATCH v10 10/11] perf ilist: Add support for metrics Ian Rogers
2025-08-19  1:39 ` [PATCH v10 11/11] perf tp_pmu: Remove unnecessary check Ian Rogers
2025-09-02 20:06 ` [PATCH v10 00/11] New perf ilist app Arnaldo Carvalho de Melo

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=aLhEPS6T-Sxnujrp@x1 \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=collin.funk1@gmail.com \
    --cc=ctshao@google.com \
    --cc=gautam@linux.ibm.com \
    --cc=howardchu95@gmail.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux@treblig.org \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=thomas.falcon@intel.com \
    --cc=tmricht@linux.ibm.com \
    --cc=weilin.wang@intel.com \
    --cc=xu.yang_2@nxp.com \
    --cc=yangtiezhu@loongson.cn \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).