Re: [PATCH v2 29/58] perf futex-contention: Port futex-contention to use python module

sashiko-bot
2026-04-23T04:13:38Z

Sashiko AI review found 1 potential issue(s):
- [Medium] The script silently drops futex wait events and causes a performance regression when a thread's process name cannot be resolved.
--

commit 3b120818238ad05dcc6b4174bcbcdb6273b4f67b
Author: Ian Rogers <irogers@google.com>

perf futex-contention: Port futex-contention to use python module

This commit ports the futex-contention.py script to use the perf Python module,
along with several style changes. The rewrite avoids the overhead of perf
script execution, resulting in significant performance improvements.

> diff --git a/tools/perf/python/futex-contention.py b/tools/perf/python/futex-contention.py
> new file mode 100755
> index 0000000000000..7c5c3d0ca60a2
> --- /dev/null
> +++ b/tools/perf/python/futex-contention.py

[ ... ]

> +def process_event(sample: perf.sample_event) -> None:
> + """Process a single sample event."""
> + def handle_start(tid: int, uaddr: int, op: int, start_time: int) -> None:
> + if (op & FUTEX_CMD_MASK) != FUTEX_WAIT:
> + return
> + if tid not in process_names:
> + try:
> + if session:
> + process = session.process(tid)
> + if process:
> + process_names[tid] = process.comm()
> + except (TypeError, AttributeError):
> + return

If session.process(tid) throws an exception, the script catches it and returns
early. Does this cause the event to be dropped since start_times[tid] is not
recorded?

Also, if session.process(tid) returns None or throws an exception,
process_names[tid] isn't populated with a fallback value. Since the thread
remains missing from process_names, won't this force the script to repeat the
expensive lookup for every subsequent event from this thread, hurting
performance?

> + start_times[tid] = (uaddr, start_time)

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