Re: [PATCH v9 18/18] perf symbol: Lazily compute idle

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unnecessary out-of-line function call `perf_env__e_machine` before the fast-path cache check in `symbol__is_idle` introduces measurable overhead on the `perf top` hot path.
--

commit 786c2d40f855657d57b68d17dcb788ab5ff93929
Author: Ian Rogers <irogers@google.com>

perf symbol: Lazily compute idle

This commit switches from an idle boolean to a helper function,
symbol__is_idle, to lazily compute whether a symbol is an idle function.
The computation now takes the machine's architecture and kernel version into
consideration.

Link: https://lore.kernel.org/lkml/20260219113850.354271-1-tmricht@linux.ibm.com/
Link: https://lore.kernel.org/lkml/20260323085255.98173-1-jameshongleiwang@126.com/

[ ... ]

> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 4702b8989354..2caa6b8b8609 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c

[ ... ]

> @@ -760,55 +747,90 @@ int modules__parse(const char *filename, void *arg,
>  	return err;
>  }
>  
> -bool symbol__is_idle(const struct symbol *sym)
> -{
> -	uint16_t flags = atomic_load(&sym->flags);
> -	uint16_t idle_val = (flags & SYMBOL_FLAG_IDLE_MASK) >> SYMBOL_FLAG_IDLE_SHIFT;
> -
> -	return idle_val == SYMBOL_IDLE__IDLE;
> -}
> -
>  /*
>   * These are symbols in the kernel image, so make sure that
>   * sym is from a kernel DSO.
>   */
> -static bool symbol__compute_is_idle(const char *name)
> +static int sym_name_cmp(const void *a, const void *b)
> +{
> +	const char *name = a;
> +	const char *const *sym = b;
> +
> +	return strcmp(name, *sym);
> +}
> +
> +bool symbol__is_idle(struct symbol *sym, const struct dso *dso, struct perf_env *env)
>  {

[ ... ]

> +	const char *name = sym->name;
> +	uint16_t e_machine = perf_env__e_machine(env, /*e_flags=*/NULL);

Does calling perf_env__e_machine() before checking the cached idle_val
introduce unnecessary overhead on the perf top hot path?

Delaying the e_machine initialization until after the cache check below
might help avoid this out-of-line function call when the idle state is
already known.

> +
> +	{
> +		uint16_t flags = atomic_load(&sym->flags);
> +		uint16_t idle_val = (flags & SYMBOL_FLAG_IDLE_MASK) >> SYMBOL_FLAG_IDLE_SHIFT;
> +
> +		if (idle_val != SYMBOL_IDLE__UNKNOWN)
> +			return idle_val == SYMBOL_IDLE__IDLE;
> +	}
> +
> +	if (!dso || dso__kernel(dso) == DSO_SPACE__USER) {
> +		symbol__set_idle(sym, /*idle=*/false);
> +		return false;
> +	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260503002248.1040791-1-irogers@google.com?part=18