Re: [PATCH v5 22/58] perf python: Add perf.pyi stubs file

sashiko-bot
2026-04-24T17:13:21Z

Sashiko AI review found 2 potential issue(s):
- [Medium] Missing API definitions in the `.pyi` type stub file
- [Medium] Undefined constant `COUNT_HW_REF_CPU_CYCLES` in type stub
--

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

perf python: Add perf.pyi stubs file

This commit introduces Python type stubs for the perf module to improve IDE
support and static analysis. It adds docstrings and type definitions for
classes, methods, and constants derived from the C source.

> diff --git a/tools/perf/python/perf.pyi b/tools/perf/python/perf.pyi
> new file mode 100644
> index 00000000000000..7ab0dfc44d1cb9
> --- /dev/null
> +++ b/tools/perf/python/perf.pyi
> @@ -0,0 +1,579 @@

[ ... ]

> +def parse_metrics(metrics_string: str) -> 'evlist':
> + """Parse a string of metrics or metric groups and return an evlist."""
> + ...

Should this include the optional pmu, cpus, and threads arguments? It
looks like the C extension parses them using "s|sOO".

> +def pmus() -> Iterator[Any]:
> + """Returns a sequence of pmus."""
> + ...

Is the tracepoint() function missing from these module-level definitions?
It appears to be exported by the C module but omitted from this stub.

[ ... ]

> +class evsel:
> + """Event selector, represents a single event being monitored."""
> + def __init__(
> + self,
> + type: int = ...,
> + config: int = ...,
> + sample_freq: int = ...,
> + sample_period: int = ...,
> + sample_type: int = ...,
> + read_format: int = ...,
> + disabled: bool = ...,
> + inherit: bool = ...,
> + pinned: bool = ...,
> + exclusive: bool = ...,
> + exclude_user: bool = ...,
> + exclude_kernel: bool = ...,
> + exclude_hv: bool = ...,
> + exclude_idle: bool = ...,
> + mmap: bool = ...,
> + context_switch: bool = ...,
> + comm: bool = ...,
> + freq: bool = ...,
> + idx: int = ...,
> + ) -> None: ...

Does this constructor need the remaining keyword arguments defined in the
C extension?

The underlying kwlist defines 29 distinct arguments, but this stub only
lists 19. Missing arguments like inherit_stat, enable_on_exec, task,
watermark, and precise_ip might cause static analysis tools and IDEs to flag
valid code.

[ ... ]

> +class sample_event:
> + """Represents a sample event from perf."""
> + evsel: evsel
> + sample_cpu: int
> + sample_time: int
> + sample_pid: int
> + type: int
> + brstack: Optional['branch_stack']
> + callchain: Optional['callchain']
> + def __getattr__(self, name: str) -> Any: ...

Should this class also define attributes like sample_tid, sample_ip,
sample_addr, and sample_period, along with methods like srccode() and
insn()?

The C extension exports these, so their omission might negatively affect
static analysis.

[ ... ]

> +COUNT_HW_STALLED_CYCLES_BACKEND: int
> +"""Stalled cycles during retirement [This event is an alias of idle-cycles-backend]."""
> +
> +COUNT_HW_REF_CPU_CYCLES: int
> +"""Total cycles; not affected by CPU frequency scaling."""
> +
> +
> +# Cache Counters

Could this cause an AttributeError at runtime? It looks like
COUNT_HW_REF_CPU_CYCLES is not actually exported in the perf__constants array
in tools/perf/util/python.c, which would cause issues for anyone relying on
the stub's declaration.

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