All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	James Clark <james.clark@linaro.org>, Xu Yang <xu.yang_2@nxp.com>,
	Chun-Tse Shao <ctshao@google.com>,
	Thomas Richter <tmricht@linux.ibm.com>,
	Sumanth Korikkar <sumanthk@linux.ibm.com>,
	Collin Funk <collin.funk1@gmail.com>,
	Thomas Falcon <thomas.falcon@intel.com>,
	Howard Chu <howardchu95@gmail.com>,
	Dapeng Mi <dapeng1.mi@linux.intel.com>,
	Levi Yun <yeoreum.yun@arm.com>,
	Yang Li <yang.lee@linux.alibaba.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	Andi Kleen <ak@linux.intel.com>,
	Weilin Wang <weilin.wang@intel.com>
Subject: Re: [PATCH v3 06/18] perf script: Change metric format to use json metrics
Date: Mon, 10 Nov 2025 22:59:22 -0800	[thread overview]
Message-ID: <aRLeyoCQVG46KKEv@google.com> (raw)
In-Reply-To: <20251111040417.270945-7-irogers@google.com>

On Mon, Nov 10, 2025 at 08:04:05PM -0800, Ian Rogers wrote:
> The metric format option isn't properly supported. This change
> improves that by making the sample events update the counts of an
> evsel, where the shadow metric code expects to read the values.  To
> support printing metrics, metrics need to be found. This is done on
> the first attempt to print a metric. Every metric is parsed and then
> the evsels in the metric's evlist compared to those in perf script
> using the perf_event_attr type and config. If the metric matches then
> it is added for printing. As an event in the perf script's evlist may
> have >1 metric id, or different leader for aggregation, the first
> metric matched will be displayed in those cases.
> 
> An example use is:
> ```
> $ perf record -a -e '{instructions,cpu-cycles}:S' -a -- sleep 1
> $ perf script -F period,metric
> ...
>      867817
>          metric:    0.30  insn per cycle
>      125394
>          metric:    0.04  insn per cycle
>      313516
>          metric:    0.11  insn per cycle
>          metric:    1.00  insn per cycle
> ```
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
[SNIP]
> @@ -2150,23 +2296,72 @@ static void perf_sample__fprint_metric(struct perf_script *script,
>  			 },
>  		.force_header = false,
>  	};
> -	struct evsel *ev2;
> -	u64 val;
> +	struct perf_counts_values *count, *old_count;
> +	int cpu_map_idx, thread_map_idx, aggr_idx;
> +	struct evsel *pos;
> +
> +	if (!init_metrics) {
> +		/* One time initialization of stat_config and metric data. */
> +		struct script_find_metrics_args args = {
> +			.evlist = evsel->evlist,
> +			/* TODO: Determine system-wide based on evlist.. */
> +			.system_wide = true,

Probably you can check if the thread_map has an entry for -1.


> +		};
> +		if (!stat_config.output)
> +			stat_config.output = stdout;
> +
> +		if (!stat_config.aggr_map) {
> +			/* TODO: currently only global aggregation is supported. */
> +			assert(stat_config.aggr_mode == AGGR_GLOBAL);

IIUC there's no option or config to set different aggregation mode for
perf script.

Thanks,
Namhyung


> +			stat_config.aggr_get_id = script_aggr_cpu_id_get;
> +			stat_config.aggr_map =
> +				cpu_aggr_map__new(evsel->evlist->core.user_requested_cpus,
> +						  aggr_cpu_id__global, /*data=*/NULL,
> +						  /*needs_sort=*/false);
> +		}
>  
> -	if (!evsel->stats)
> -		evlist__alloc_stats(&stat_config, script->session->evlist, /*alloc_raw=*/false);
> -	if (evsel_script(leader)->gnum++ == 0)
> -		perf_stat__reset_shadow_stats();
> -	val = sample->period * evsel->scale;
> -	evsel_script(evsel)->val = val;
> -	if (evsel_script(leader)->gnum == leader->core.nr_members) {
> -		for_each_group_member (ev2, leader) {
> -			perf_stat__print_shadow_stats(&stat_config, ev2,
> -						      evsel_script(ev2)->val,
> -						      sample->cpu,
> -						      &ctx);
> +		metricgroup__for_each_metric(pmu_metrics_table__find(), script_find_metrics, &args);
> +		init_metrics = true;
> +	}
> +
> +	if (!evsel->stats) {
> +		if (evlist__alloc_stats(&stat_config, evsel->evlist, /*alloc_raw=*/true) < 0)
> +			return;
> +	}
> +	if (!evsel->stats->aggr) {
> +		if (evlist__alloc_aggr_stats(evsel->evlist, stat_config.aggr_map->nr) < 0)
> +			return;
> +	}
> +
> +	/* Update the evsel's count using the sample's data. */
> +	cpu_map_idx = perf_cpu_map__idx(evsel->core.cpus, (struct perf_cpu){sample->cpu});
> +	thread_map_idx = perf_thread_map__idx(evsel->core.threads, sample->tid);
> +	if (thread_map_idx < 0) {
> +		/* Missing thread, check for any thread. */
> +		if (perf_thread_map__pid(evsel->core.threads, /*idx=*/0) == -1) {
> +			thread_map_idx = 0;
> +		} else {
> +			pr_info("Missing thread map entry for thread %d\n", sample->tid);
> +			return;
> +		}
> +	}
> +	count = perf_counts(evsel->counts, cpu_map_idx, thread_map_idx);
> +	old_count = perf_counts(evsel->prev_raw_counts, cpu_map_idx, thread_map_idx);
> +	count->val = old_count->val + sample->period;
> +	count->run = old_count->run + 1;
> +	count->ena = old_count->ena + 1;
> +
> +	/* Update the aggregated stats. */
> +	perf_stat_process_counter(&stat_config, evsel);
> +
> +	/* Display all metrics. */
> +	evlist__for_each_entry(evsel->evlist, pos) {
> +		cpu_aggr_map__for_each_idx(aggr_idx, stat_config.aggr_map) {
> +			perf_stat__print_shadow_stats(&stat_config, pos,
> +						count->val,
> +						aggr_idx,
> +						&ctx);
>  		}
> -		evsel_script(leader)->gnum = 0;
>  	}
>  }
>  
> @@ -2348,7 +2543,7 @@ static void process_event(struct perf_script *script,
>  	}
>  
>  	if (PRINT_FIELD(METRIC))
> -		perf_sample__fprint_metric(script, thread, evsel, sample, fp);
> +		perf_sample__fprint_metric(thread, evsel, sample, fp);
>  
>  	if (verbose > 0)
>  		fflush(fp);
> -- 
> 2.51.2.1041.gc1ab5b90ca-goog
> 

  reply	other threads:[~2025-11-11  6:59 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-11  4:03 [PATCH v3 00/18] Switch the default perf stat metrics to json Ian Rogers
2025-11-11  4:04 ` [PATCH v3 01/18] perf metricgroup: Add care to picking the evsel for displaying a metric Ian Rogers
2025-11-11  8:15   ` Mi, Dapeng
2025-11-11 17:20     ` Ian Rogers
2025-11-11 19:05       ` Namhyung Kim
2025-11-12  8:14         ` Mi, Dapeng
2025-11-11  4:04 ` [PATCH v3 02/18] perf expr: Add #target_cpu literal Ian Rogers
2025-11-11  4:04 ` [PATCH v3 03/18] perf jevents: Add set of common metrics based on default ones Ian Rogers
2025-11-11  6:37   ` Namhyung Kim
2025-11-11 18:38     ` Ian Rogers
2025-11-11  4:04 ` [PATCH v3 04/18] perf jevents: Add metric DefaultShowEvents Ian Rogers
2025-11-11  4:04 ` [PATCH v3 05/18] perf stat: Add detail -d,-dd,-ddd metrics Ian Rogers
2025-11-11  4:04 ` [PATCH v3 06/18] perf script: Change metric format to use json metrics Ian Rogers
2025-11-11  6:59   ` Namhyung Kim [this message]
2025-11-11 20:52     ` Ian Rogers
2025-11-11  4:04 ` [PATCH v3 07/18] perf stat: Remove hard coded shadow metrics Ian Rogers
2025-11-11  7:02   ` Namhyung Kim
2025-11-11 17:23     ` Ian Rogers
2025-11-11 19:03       ` Namhyung Kim
2025-11-11  4:04 ` [PATCH v3 08/18] perf stat: Fix default metricgroup display on hybrid Ian Rogers
2025-11-11  4:04 ` [PATCH v3 09/18] perf stat: Sort default events/metrics Ian Rogers
2025-11-11  4:04 ` [PATCH v3 10/18] perf stat: Remove "unit" workarounds for metric-only Ian Rogers
2025-11-11  4:04 ` [PATCH v3 11/18] perf test stat+json: Improve metric-only testing Ian Rogers
2025-11-11  4:04 ` [PATCH v3 12/18] perf test stat: Ignore failures in Default[234] metricgroups Ian Rogers
2025-11-11  4:04 ` [PATCH v3 13/18] perf test stat: Update std_output testing metric expectations Ian Rogers
2025-11-11  4:04 ` [PATCH v3 14/18] perf test metrics: Update all metrics for possibly failing default metrics Ian Rogers
2025-11-11  4:04 ` [PATCH v3 15/18] perf test stat: Update shadow test to use metrics Ian Rogers
2025-11-11  4:04 ` [PATCH v3 16/18] perf test stat: Update test expectations and events Ian Rogers
2025-11-11  4:04 ` [PATCH v3 17/18] perf test stat csv: " Ian Rogers
2025-11-11  4:04 ` [PATCH v3 18/18] perf tool_pmu: Make core_wide and target_cpu json events 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=aRLeyoCQVG46KKEv@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --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=dapeng1.mi@linux.intel.com \
    --cc=howardchu95@gmail.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sumanthk@linux.ibm.com \
    --cc=thomas.falcon@intel.com \
    --cc=tmricht@linux.ibm.com \
    --cc=weilin.wang@intel.com \
    --cc=xu.yang_2@nxp.com \
    --cc=yang.lee@linux.alibaba.com \
    --cc=yeoreum.yun@arm.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.