Re: [PATCH v6 33/59] perf check-perf-trace: Port check-perf-trace to use python module

sashiko-bot
2026-04-25T18:08:37Z

Sashiko AI review found 3 potential issue(s):
- [High] The script will crash with an unhandled TypeError if a thread is not found by session.find_thread().
- [Medium] Incorrect thread lookup using sample_pid instead of sample_tid leads to inaccurate event attribution.
- [Medium] Type hinting with collections.defaultdict[str, int] uses PEP 585 syntax, which crashes on module load in Python versions older than 3.9.
--

commit 4e0bca6abdc96a8b064133ab4674a7ddb85d72dd
Author: Ian Rogers <irogers@google.com>

perf check-perf-trace: Port check-perf-trace to use python module

This commit ports the check-perf-trace script to directly use the perf python
module. This avoids creating intermediate dictionaries for all event fields,
making execution significantly faster than using perf script callbacks.

> diff --git a/tools/perf/python/check-perf-trace.py b/tools/perf/python/check-perf-trace.py
> new file mode 100755
> index 000000000000..7c1c7632a091
> --- /dev/null
> +++ b/tools/perf/python/check-perf-trace.py
> @@ -0,0 +1,113 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +"""
> +Basic test of Python scripting support for perf.
> +Ported from tools/perf/scripts/python/check-perf-trace.py
> +"""
> +
> +import argparse
> +import collections
> +import perf
> +
> +unhandled: collections.defaultdict[str, int] = collections.defaultdict(int)

Will this syntax cause a module load crash on older Python versions?

Subscripting standard collections directly for type hints was introduced
in Python 3.9. On older versions, like Python 3.6 or 3.8, this raises a
TypeError indicating the type object is not subscriptable, which prevents
the script from running.

[ ... ]

> +def print_header(event_name: str, sample: perf.sample_event) -> None:
> + """Prints common header for events."""
> + secs = sample.sample_time // 1000000000
> + nsecs = sample.sample_time % 1000000000
> + comm = session.find_thread(sample.sample_pid).comm() if session else "[unknown]"

Are we looking up the correct thread for the event here? By passing
sample.sample_pid, which corresponds to the process ID, instead of
sample.sample_tid, we might systematically look up the main thread
instead of the specific secondary thread that triggered the event.

How does the script handle threads that are not found?

Looking at pyrf_session__find_thread() in tools/perf/util/python.c,
failing to locate a thread results in a Python TypeError being raised.
Since there is no try/except block around the find_thread() call,
encountering an unknown thread (which can happen with ring buffer
overruns or dropped events) could cause the script to immediately crash.

> + print(f"{event_name:<20} {sample.sample_cpu:5} {secs:05}.{nsecs:09} "

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