mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	 Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	 Mark Rutland <mark.rutland@arm.com>,
	 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,  Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	 Kan Liang <kan.liang@linux.intel.com>,
	Kajol Jain <kjain@linux.ibm.com>,
	 John Garry <john.g.garry@oracle.com>, Kaige Ye <ye@kaige.org>,
	 K Prateek Nayak <kprateek.nayak@amd.com>,
	linux-perf-users@vger.kernel.org,  linux-kernel@vger.kernel.org
Cc: Stephane Eranian <eranian@google.com>
Subject: [PATCH v1 1/3] perf stat: Pass fewer metric arguments
Date: Thu,  1 Feb 2024 18:25:10 -0800	[thread overview]
Message-ID: <20240202022512.467636-1-irogers@google.com> (raw)

Pass metric_expr and evsel rather than specific variables from the
struct, thereby reducing the number of arguments. This will enable
later fixes.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/stat-shadow.c | 75 +++++++++++++++--------------------
 1 file changed, 33 insertions(+), 42 deletions(-)

diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index e31426167852..f6c9d2f98835 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -355,23 +355,22 @@ static void print_nsecs(struct perf_stat_config *config,
 		print_metric(config, ctxp, NULL, NULL, "CPUs utilized", 0);
 }
 
-static int prepare_metric(struct evsel **metric_events,
-			  struct metric_ref *metric_refs,
+static int prepare_metric(const struct metric_expr *mexp,
 			  struct expr_parse_ctx *pctx,
 			  int aggr_idx)
 {
 	int i;
 
-	for (i = 0; metric_events[i]; i++) {
+	for (i = 0; mexp->metric_events[i]; i++) {
 		char *n;
 		double val;
 		int source_count = 0;
 
-		if (evsel__is_tool(metric_events[i])) {
+		if (evsel__is_tool(mexp->metric_events[i])) {
 			struct stats *stats;
 			double scale;
 
-			switch (metric_events[i]->tool_event) {
+			switch (mexp->metric_events[i]->tool_event) {
 			case PERF_TOOL_DURATION_TIME:
 				stats = &walltime_nsecs_stats;
 				scale = 1e-9;
@@ -391,19 +390,20 @@ static int prepare_metric(struct evsel **metric_events,
 				pr_err("Invalid tool event 'max'");
 				abort();
 			default:
-				pr_err("Unknown tool event '%s'", evsel__name(metric_events[i]));
+				pr_err("Unknown tool event '%s'",
+				       evsel__name(mexp->metric_events[i]));
 				abort();
 			}
 			val = avg_stats(stats) * scale;
 			source_count = 1;
 		} else {
-			struct perf_stat_evsel *ps = metric_events[i]->stats;
+			struct perf_stat_evsel *ps = mexp->metric_events[i]->stats;
 			struct perf_stat_aggr *aggr = &ps->aggr[aggr_idx];
 
 			if (!aggr)
 				break;
 
-                        if (!metric_events[i]->supported) {
+			if (!mexp->metric_events[i]->supported) {
 				/*
 				 * Not supported events will have a count of 0,
 				 * which can be confusing in a
@@ -419,19 +419,19 @@ static int prepare_metric(struct evsel **metric_events,
 				 * reverse the scale before computing the
 				 * metric.
 				 */
-				val = aggr->counts.val * (1.0 / metric_events[i]->scale);
-				source_count = evsel__source_count(metric_events[i]);
+				val = aggr->counts.val * (1.0 / mexp->metric_events[i]->scale);
+				source_count = evsel__source_count(mexp->metric_events[i]);
 			}
 		}
-		n = strdup(evsel__metric_id(metric_events[i]));
+		n = strdup(evsel__metric_id(mexp->metric_events[i]));
 		if (!n)
 			return -ENOMEM;
 
 		expr__add_id_val_source_count(pctx, n, val, source_count);
 	}
 
-	for (int j = 0; metric_refs && metric_refs[j].metric_name; j++) {
-		int ret = expr__add_ref(pctx, &metric_refs[j]);
+	for (int j = 0; mexp->metric_refs && mexp->metric_refs[j].metric_name; j++) {
+		int ret = expr__add_ref(pctx, &mexp->metric_refs[j]);
 
 		if (ret)
 			return ret;
@@ -441,14 +441,8 @@ static int prepare_metric(struct evsel **metric_events,
 }
 
 static void generic_metric(struct perf_stat_config *config,
-			   const char *metric_expr,
-			   const char *metric_threshold,
-			   struct evsel **metric_events,
-			   struct metric_ref *metric_refs,
-			   char *name,
-			   const char *metric_name,
-			   const char *metric_unit,
-			   int runtime,
+			struct metric_expr *mexp,
+			struct evsel *evsel,
 			   int aggr_idx,
 			   struct perf_stat_output_ctx *out)
 {
@@ -465,55 +459,55 @@ static void generic_metric(struct perf_stat_config *config,
 
 	if (config->user_requested_cpu_list)
 		pctx->sctx.user_requested_cpu_list = strdup(config->user_requested_cpu_list);
-	pctx->sctx.runtime = runtime;
+	pctx->sctx.runtime = mexp->runtime;
 	pctx->sctx.system_wide = config->system_wide;
-	i = prepare_metric(metric_events, metric_refs, pctx, aggr_idx);
+	i = prepare_metric(mexp, pctx, aggr_idx);
 	if (i < 0) {
 		expr__ctx_free(pctx);
 		return;
 	}
-	if (!metric_events[i]) {
-		if (expr__parse(&ratio, pctx, metric_expr) == 0) {
+	if (!mexp->metric_events[i]) {
+		if (expr__parse(&ratio, pctx, mexp->metric_expr) == 0) {
 			char *unit;
 			char metric_bf[64];
 
-			if (metric_threshold &&
-			    expr__parse(&threshold, pctx, metric_threshold) == 0 &&
+			if (mexp->metric_threshold &&
+			    expr__parse(&threshold, pctx, mexp->metric_threshold) == 0 &&
 			    !isnan(threshold)) {
 				color = fpclassify(threshold) == FP_ZERO
 					? PERF_COLOR_GREEN : PERF_COLOR_RED;
 			}
 
-			if (metric_unit && metric_name) {
-				if (perf_pmu__convert_scale(metric_unit,
+			if (mexp->metric_unit && mexp->metric_name) {
+				if (perf_pmu__convert_scale(mexp->metric_unit,
 					&unit, &scale) >= 0) {
 					ratio *= scale;
 				}
-				if (strstr(metric_expr, "?"))
+				if (strstr(mexp->metric_expr, "?"))
 					scnprintf(metric_bf, sizeof(metric_bf),
-					  "%s  %s_%d", unit, metric_name, runtime);
+					  "%s  %s_%d", unit, mexp->metric_name, mexp->runtime);
 				else
 					scnprintf(metric_bf, sizeof(metric_bf),
-					  "%s  %s", unit, metric_name);
+					  "%s  %s", unit, mexp->metric_name);
 
 				print_metric(config, ctxp, color, "%8.1f",
 					     metric_bf, ratio);
 			} else {
 				print_metric(config, ctxp, color, "%8.2f",
-					metric_name ?
-					metric_name :
-					out->force_header ?  name : "",
+					mexp->metric_name ?
+					mexp->metric_name :
+					out->force_header ?  evsel->name : "",
 					ratio);
 			}
 		} else {
 			print_metric(config, ctxp, color, /*unit=*/NULL,
 				     out->force_header ?
-				     (metric_name ? metric_name : name) : "", 0);
+				     (mexp->metric_name ?: evsel->name) : "", 0);
 		}
 	} else {
 		print_metric(config, ctxp, color, /*unit=*/NULL,
 			     out->force_header ?
-			     (metric_name ? metric_name : name) : "", 0);
+			     (mexp->metric_name ?: evsel->name) : "", 0);
 	}
 
 	expr__ctx_free(pctx);
@@ -528,7 +522,7 @@ double test_generic_metric(struct metric_expr *mexp, int aggr_idx)
 	if (!pctx)
 		return NAN;
 
-	if (prepare_metric(mexp->metric_events, mexp->metric_refs, pctx, aggr_idx) < 0)
+	if (prepare_metric(mexp, pctx, aggr_idx) < 0)
 		goto out;
 
 	if (expr__parse(&ratio, pctx, mexp->metric_expr))
@@ -630,10 +624,7 @@ void *perf_stat__print_shadow_stats_metricgroup(struct perf_stat_config *config,
 
 		if ((*num)++ > 0)
 			out->new_line(config, ctxp);
-		generic_metric(config, mexp->metric_expr, mexp->metric_threshold,
-			       mexp->metric_events, mexp->metric_refs, evsel->name,
-			       mexp->metric_name, mexp->metric_unit, mexp->runtime,
-			       aggr_idx, out);
+		generic_metric(config, mexp, evsel, aggr_idx, out);
 	}
 
 	return NULL;
-- 
2.43.0.594.gd9cf4e227d-goog


             reply	other threads:[~2024-02-02  2:25 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-02  2:25 Ian Rogers [this message]
2024-02-02  2:25 ` [PATCH v1 2/3] perf metrics: Compute unmerged uncore metrics individually Ian Rogers
2024-02-06  2:02   ` Namhyung Kim
2024-02-06  2:21     ` Ian Rogers
2024-02-02  2:25 ` [PATCH v1 3/3] perf stat: Fix metric-only aggregation index Ian Rogers
2024-02-06  1:59 ` [PATCH v1 1/3] perf stat: Pass fewer metric arguments Namhyung Kim
2024-02-06  2:23   ` Ian Rogers

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=20240202022512.467636-1-irogers@google.com \
    --to=irogers@google.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=eranian@google.com \
    --cc=john.g.garry@oracle.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=kjain@linux.ibm.com \
    --cc=kprateek.nayak@amd.com \
    --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=ye@kaige.org \
    /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

all inboxes | Powered by JetHome®