Re: [PATCH 4/4] perf script: Document and test --lazy-load-symbols and --max-symbol-bytes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] perf test: missing err=2 assignments cause skips to be reported as success
- [Medium] perf test: strict byte comparison fails on platforms with differing debug output
--

commit 152590a92c1dd38a6f7f70f13ced82231e7560bb
Author: Alireza Haghdoost <haghdoost@uber.com>

perf script: Document and test --lazy-load-symbols and --max-symbol-bytes

Document both new options in perf-script.txt detailing their interaction,
the memory tradeoffs, and output behavior for certain targets. Add a shell
test that records a small profile and asserts that lazy loading produces
identical output to the eager loader, and that the max symbol budget is
enforced.

> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index 200ea25891d88..b5a90ff22342f 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -412,6 +412,30 @@ include::itrace.txt[]
>  
>          Default: 127
>  
> +--lazy-load-symbols::
> +	Resolve symbols lazily instead of eagerly loading the full
> +	symbol table of every DSO that appears in a sample. A compact
> +	sorted index is built per DSO and only the addresses that appear
> +	in samples are materialized into symbols, with names read from the
> +	file's string table at lookup time. This sharply reduces memory
> +	(and usually time) for profiles of large binaries where only a
> +	small fraction of the symbol table is referenced. This applies only
> +	to userspace ELF DSOs; kernel DSOs and modules always load eagerly.
> +	Output may differ from the default loader for some targets
> +	(e.g. PPC64 .opd, .gnu_debugdata, or split debuginfo). Default: off.

[ ... ]

> diff --git a/tools/perf/tests/shell/script_lazy_load_symbols.sh b/tools/perf/tests/shell/script_lazy_load_symbols.sh
> new file mode 100755
> index 0000000000000..799c61e2f3e87
> --- /dev/null
> +++ b/tools/perf/tests/shell/script_lazy_load_symbols.sh
> @@ -0,0 +1,120 @@
[ ... ]
> +test_lazy_load_identical() {
> +	echo "Lazy-load output matches eager loader"
> +
> +	# Record a small profile with callchains so symbol resolution runs.
> +	if ! perf record -o "${perfdata}" -g -- perf test -w thloop 2> /dev/null
> +	then
> +		echo "Lazy-load identical [Skipped record not supported]"
> +		return

[Severity: Medium]
Will this script falsely report success when it skips tests? It looks like
the function returns here without setting err=2, which would cause the test
script to exit with 0 (pass) instead of 2 (skip) when perf record fails.

> +	fi
> +
> +	if ! perf script -i "${perfdata}" 2> /dev/null > "${eager_out}" || \
> +	   ! perf script --lazy-load-symbols -i "${perfdata}" 2> /dev/null > "${lazy_out}"
> +	then
> +		echo "Lazy-load identical [Failed perf script error]"
> +		err=1
> +		return
> +	fi
> +
> +	# The comparison is only meaningful if something actually resolved;
> +	# two all-[unknown] outputs would also match.
> +	if ! grep -q "${testsym}" "${eager_out}"
> +	then
> +		echo "Lazy-load identical [Skipped no ${testsym} resolved]"
> +		return
> +	fi
> +
> +	if ! cmp -s "${eager_out}" "${lazy_out}"
> +	then
> +		echo "Lazy-load identical [Failed output differs]"
> +		err=1
> +		return

[Severity: Medium]
Since the documentation mentions that output may differ from the default loader
for some targets (e.g., PPC64, .gnu_debugdata, or split debuginfo), will this
strict byte-identical comparison fail spuriously on those platforms?

> +	fi
> +	echo "Lazy-load identical [Success]"
> +}
> +
> +test_max_symbol_bytes() {
> +	echo "--max-symbol-bytes budget enforcement"
> +
> +	# Depends on ${perfdata} from test_lazy_load_identical.
> +	if [ ! -s "${perfdata}" ]
> +	then
> +		echo "--max-symbol-bytes budget [Skipped record not supported]"
> +		return

[Severity: Medium]
Similar to the skip path in test_lazy_load_identical(), should err=2 be set
here so the script correctly returns the skip exit code rather than success?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-perf-symbol-memory-send-v1-0-1d3360e21f07@uber.com?part=4