Re: [PATCH v3 2/7] perf report: Add parallelism sort key

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Jan 27, 2025 at 10:58:49AM +0100, Dmitry Vyukov wrote:
> Show parallelism level in profiles if requested by user.
> 
> Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Ian Rogers <irogers@google.com>
> Cc: linux-perf-users@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  tools/perf/builtin-report.c | 11 +++++++++++
>  tools/perf/util/hist.c      |  2 ++
>  tools/perf/util/hist.h      |  3 +++
>  tools/perf/util/session.c   | 12 ++++++++++++
>  tools/perf/util/session.h   |  1 +
>  tools/perf/util/sort.c      | 23 +++++++++++++++++++++++
>  tools/perf/util/sort.h      |  1 +
>  7 files changed, 53 insertions(+)
> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 0d9bd090eda71..14d49f0625881 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -1720,6 +1720,17 @@ int cmd_report(int argc, const char **argv)
>  		symbol_conf.annotate_data_sample = true;
>  	}
>  
> +	if (report.disable_order || !perf_session__has_switch_events(session)) {
> +		if ((sort_order && strstr(sort_order, "parallelism")) ||
> +				(field_order && strstr(field_order, "parallelism"))) {
> +			if (report.disable_order)
> +				ui__error("Use of parallelism is incompatible with --disable-order.\n");
> +			else
> +				ui__error("Use of parallelism requires --switch-events during record.\n");
> +			return -1;
> +		}
> +	}
> +
>  	if (sort_order && strstr(sort_order, "ipc")) {
>  		parse_options_usage(report_usage, options, "s", 1);
>  		goto error;
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 0f30f843c566d..cafd693568189 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -207,6 +207,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>  
>  	hists__new_col_len(hists, HISTC_CGROUP, 6);
>  	hists__new_col_len(hists, HISTC_CGROUP_ID, 20);
> +	hists__new_col_len(hists, HISTC_PARALLELISM, 11);
>  	hists__new_col_len(hists, HISTC_CPU, 3);
>  	hists__new_col_len(hists, HISTC_SOCKET, 6);
>  	hists__new_col_len(hists, HISTC_MEM_LOCKED, 6);
> @@ -741,6 +742,7 @@ __hists__add_entry(struct hists *hists,
>  		.ip	 = al->addr,
>  		.level	 = al->level,
>  		.code_page_size = sample->code_page_size,
> +		.parallelism	= al->parallelism,
>  		.stat = {
>  			.nr_events = 1,
>  			.period	= sample->period,
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 46c8373e31465..a6e662d77dc24 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -42,6 +42,7 @@ enum hist_column {
>  	HISTC_CGROUP_ID,
>  	HISTC_CGROUP,
>  	HISTC_PARENT,
> +	HISTC_PARALLELISM,
>  	HISTC_CPU,
>  	HISTC_SOCKET,
>  	HISTC_SRCLINE,
> @@ -228,6 +229,7 @@ struct hist_entry {
>  	u64			transaction;
>  	s32			socket;
>  	s32			cpu;
> +	int			parallelism;

Can you make it u16 and move to around cpumode to remove paddings?

Thanks,
Namhyung


>  	u64			code_page_size;
>  	u64			weight;
>  	u64			ins_lat;
> @@ -580,6 +582,7 @@ bool perf_hpp__is_thread_entry(struct perf_hpp_fmt *fmt);
>  bool perf_hpp__is_comm_entry(struct perf_hpp_fmt *fmt);
>  bool perf_hpp__is_dso_entry(struct perf_hpp_fmt *fmt);
>  bool perf_hpp__is_sym_entry(struct perf_hpp_fmt *fmt);
> +bool perf_hpp__is_parallelism_entry(struct perf_hpp_fmt *fmt);
>  
>  struct perf_hpp_fmt *perf_hpp_fmt__dup(struct perf_hpp_fmt *fmt);