Re: [PATCH 1/3] perf sort: Keep output fields in the same level

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Mar 07, 2025 at 12:08:27AM -0800, Namhyung Kim wrote:
> This is useful for hierarchy output mode where the first level is
> considered as output fields.  We want them in the same level so that it
> can show only the remaining groups in the hierarchy.
> 
> Before:
>   $ perf report -s overhead,sample,period,comm,dso -H --stdio
>   ...
>   #          Overhead  Samples / Period / Command / Shared Object
>   # .................  ..........................................
>   #
>      100.00%           4035
>         100.00%           3835883066
>            100.00%           perf
>                99.37%           perf
>                 0.50%           ld-linux-x86-64.so.2
>                 0.06%           [unknown]
>                 0.04%           libc.so.6
>                 0.02%           libLLVM-16.so.1
> 
> After:
>   $ perf report -s overhead,sample,period,comm,dso -H --stdio
>   ...
>   #    Overhead       Samples        Period  Command / Shared Object
>   # .......................................  .......................
>   #
>      100.00%          4035    3835883066     perf
>          99.37%          4005    3811826223     perf
>           0.50%            19      19210014     ld-linux-x86-64.so.2
>           0.06%             8       2367089     [unknown]
>           0.04%             2       1720336     libc.so.6
>           0.02%             1        759404     libLLVM-16.so.1
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Ping!  Anybody interested in this change? :)

Thanks,
Namhyung

> ---
>  tools/perf/util/sort.c | 44 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index f08fbc4bf0a2ce29..6b49d64854f5f986 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -3720,6 +3720,34 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok,
>  	return -ESRCH;
>  }
>  
> +/* This should match with sort_dimension__add() above */
> +static bool is_hpp_sort_key(const char *key)
> +{
> +	unsigned i;
> +
> +	for (i = 0; i < ARRAY_SIZE(arch_specific_sort_keys); i++) {
> +		if (!strcmp(arch_specific_sort_keys[i], key) &&
> +		    !arch_support_sort_key(key)) {
> +			return false;
> +		}
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(common_sort_dimensions); i++) {
> +		struct sort_dimension *sd = &common_sort_dimensions[i];
> +
> +		if (sd->name && !strncasecmp(key, sd->name, strlen(key)))
> +			return false;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(hpp_sort_dimensions); i++) {
> +		struct hpp_dimension *hd = &hpp_sort_dimensions[i];
> +
> +		if (!strncasecmp(key, hd->name, strlen(key)))
> +			return true;
> +	}
> +	return false;
> +}
> +
>  static int setup_sort_list(struct perf_hpp_list *list, char *str,
>  			   struct evlist *evlist)
>  {
> @@ -3727,7 +3755,9 @@ static int setup_sort_list(struct perf_hpp_list *list, char *str,
>  	int ret = 0;
>  	int level = 0;
>  	int next_level = 1;
> +	int prev_level = 0;
>  	bool in_group = false;
> +	bool prev_was_hpp = false;
>  
>  	do {
>  		tok = str;
> @@ -3748,6 +3778,19 @@ static int setup_sort_list(struct perf_hpp_list *list, char *str,
>  		}
>  
>  		if (*tok) {
> +			if (is_hpp_sort_key(tok)) {
> +				/* keep output (hpp) sort keys in the same level */
> +				if (prev_was_hpp) {
> +					bool next_same = (level == next_level);
> +
> +					level = prev_level;
> +					next_level = next_same ? level : level+1;
> +				}
> +				prev_was_hpp = true;
> +			} else {
> +				prev_was_hpp = false;
> +			}
> +
>  			ret = sort_dimension__add(list, tok, evlist, level);
>  			if (ret == -EINVAL) {
>  				if (!cacheline_size() && !strncasecmp(tok, "dcacheline", strlen(tok)))
> @@ -3759,6 +3802,7 @@ static int setup_sort_list(struct perf_hpp_list *list, char *str,
>  				ui__error("Unknown --sort key: `%s'", tok);
>  				break;
>  			}
> +			prev_level = level;
>  		}
>  
>  		level = next_level;
> -- 
> 2.49.0.rc0.332.g42c0ae87b1-goog
>