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

sashiko-bot
2026-04-20T00:38:00Z

Sashiko AI review found 3 potential issue(s):
- [High] Fatal TypeError due to invalid type hinting syntax.
- [Medium] Unbounded memory growth leading to OOM risks on large trace files.
- [Medium] Hardcoded input file drops support for custom trace files.
--

commit 1b6a7eb3db688a2d738e2a6cae094deea81591b4
Author: Ian Rogers <irogers@google.com>

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

This commit rewrites tools/perf/scripts/python/futex-contention.py to use
the python module. It aims to improve performance by avoiding the overheads
in the perf script execution.

> diff --git a/tools/perf/python/futex-contention.py b/tools/perf/python/futex-contention.py
> new file mode 100755
> index 0000000000000..153583de9cdee
> --- /dev/null
> +++ b/tools/perf/python/futex-contention.py
> @@ -0,0 +1,54 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +# Measures futex contention.
> +from collections import defaultdict
> +from typing import Dict
> +import perf
> +
> +process_names: Dict[int, str] = {}
> +start_times: Dict[int, [int, int]] = {}
> +session = None
> +durations: Dict[[int, int], list] = defaultdict(list)

Will this raise a TypeError at module import time? typing.Dict requires
types as arguments, not list instances. Should this use Tuple[int, int]
instead?

[ ... ]

> + def handle_end(tid: int, end_time: int) -> None:
> + if tid not in start_times:
> + return
> + (uaddr, start_time) = start_times[tid]
> + del start_times[tid]
> + durations[(tid, uaddr)].append(end_time - start_time)

Does storing every single duration in a list risk unbounded memory growth
and potential Out-Of-Memory crashes on large traces with high-frequency
futex workloads?

The original perf script based utility function maintained a running O(1)
memory aggregate (min, max, avg, count) for each lock.

[ ... ]

> +if __name__ == "__main__":
> + session = perf.session(perf.data("perf.data"), sample=process_event)

Does this hardcode the input file to perf.data in the current working
directory?

Unlike other ported scripts that use argparse and sys.argv, this script
drops support for custom trace files. This breaks compatibility with
workflows where users specify custom trace locations (e.g., perf script -i
my_trace.data).

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