Re: [PATCH v6 22/59] perf python: Add perf.pyi stubs file

sashiko-bot
2026-04-25T18:11:14Z

Sashiko AI review found 4 potential issue(s):
- [Medium] Missing module-level functions and classes in perf.pyi
- [Medium] Incomplete and missing attributes in perf event classes
- [Medium] Missing properties and incorrect attribute names in auxiliary perf classes
- [Medium] Incorrect method signatures and missing sequence methods
--

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

perf python: Add perf.pyi stubs file

Add Python type stubs for the perf module to improve IDE support and
static analysis. Includes docstrings for classes, methods, and
constants derived from C source and JSON definitions.

> diff --git a/tools/perf/python/perf.pyi b/tools/perf/python/perf.pyi
> new file mode 100644
> index 000000000000..91e19704e595
> --- /dev/null
> +++ b/tools/perf/python/perf.pyi
> @@ -0,0 +1,581 @@
> +"""Type stubs for the perf Python module."""
> +from typing import Callable, Dict, List, Optional, Any, Iterator
> +

[ ... ]

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

Could the tracepoint() function exported by perf__methods in
tools/perf/util/python.c be missing from this stub?

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

Would pmus() be more accurately typed as returning an iterator over pmu
objects rather than Iterator[Any]?

Are the pmu class and its methods like events() and name() missing?

> +class data:

[ ... ]

> +class counts_values:
> + """Raw counter values."""
> + val: int
> + ena: int
> + run: int
> +

Are the id and lost fields, as well as the values property, missing from
this class?

[ ... ]

> +class evsel:
> + """Event selector, represents a single event being monitored."""
> + def __init__(
> + self,

[ ... ]

> + idx: int = ...,
> + ) -> None: ...
> + def __str__(self) -> str:
> + """Return string representation of the event."""
> + ...
> + def open(self) -> None:

Could open() be missing optional arguments like cpus, threads, group, and
inherit?

Should properties like tracking, config, read_format, sample_period,
sample_type, size, type, and wakeup_events be included in the evsel class?

> + """Open the event selector file descriptor table."""
> + ...
> + def read(self, cpu: int, thread: int) -> counts_values:

[ ... ]

> +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']

Is this class missing specialized methods like srccode() and insn()?

Also, shouldn't sample_members properties like sample_tid, sample_id,
sample_stream_id, and sample_period be present here and across other
event classes?

> + def __getattr__(self, name: str) -> Any: ...
> +

[ ... ]

> +class switch_event:
> + """Represents a SWITCH or SWITCH_CPU_WIDE record."""
> + type: int
> +

Could the next_prev_pid and next_prev_tid properties be missing from this
class?

[ ... ]

> +class callchain_node:
> + """Represents a frame in the callchain."""
> + ip: int
> + sym: Optional[Any]
> + map: Optional[Any]

The C extension exposes these properties as symbol and dso via
pyrf_callchain_node__getset.

Will the use of sym and map cause code that passes static analysis to fail
with AttributeError at runtime?

> +
> +class callchain:

[ ... ]

> +class evlist:

Since the evlist class functions as a Python sequence in C, should __len__
and __getitem__ be defined here?

> + def __init__(self, cpus: cpu_map, threads: thread_map) -> None: ...
> + def open(self) -> None:
> + """Open the events in the list."""
> + ...
> + def close(self) -> None:
> + """Close the events in the list."""
> + ...
> + def mmap(self) -> None:

Are optional arguments like pages and overwrite missing from mmap()?

> + """Memory map the event buffers."""
> + ...
> + def poll(self, timeout: int) -> int:

Is timeout incorrectly marked as required here? It appears to be optional
and defaults to -1 in the C implementation.

> + """Poll for events.

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