From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
James Clark <james.clark@linaro.org>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] perf python: Fix ilist TreeNode string type drift
Date: Sun, 13 Sep 2026 18:40:14 -0300 [thread overview]
Message-ID: <aqcYPoBOEKNk0WMc@x2> (raw)
In-Reply-To: <20260910211539.3157204-1-irogers@google.com>
On Thu, Sep 10, 2026 at 02:15:39PM -0700, Ian Rogers wrote:
> Updates Tree type hints to correctly use TreeNode[TreeValue]
> across all assignments preventing mypy strictness mismatches.
>
> Address some other minor stylistic issues reported by mypy, pylint and
> Sashiko.
Thanks, applied to perf-tools-next, for v7.4.
- Arnaldo
> Assisted-by: Antigravity:gemini-3.1-pro
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> v2: Tweak the imports and type annotations for better 3.9
> compatibility and to be more pythonic.
> ---
> tools/perf/python/ilist.py | 78 ++++++++++++++++++++------------------
> 1 file changed, 42 insertions(+), 36 deletions(-)
>
> diff --git a/tools/perf/python/ilist.py b/tools/perf/python/ilist.py
> index 0d757ddb4795..ebff0a843b7a 100755
> --- a/tools/perf/python/ilist.py
> +++ b/tools/perf/python/ilist.py
> @@ -2,11 +2,13 @@
> # SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> """Interactive perf list."""
>
> -from abc import ABC, abstractmethod
> +from __future__ import annotations
> +
> import argparse
> +from abc import ABC, abstractmethod
> from dataclasses import dataclass
> import math
> -from typing import Any, Dict, Optional, Tuple
> +from typing import Any, Optional
> import perf
> from textual import on
> from textual.app import App, ComposeResult
> @@ -19,7 +21,7 @@ from textual.widgets import Button, Footer, Header, Input, Label, Sparkline, Sta
> from textual.widgets.tree import TreeNode
>
>
> -def get_info(info: Dict[str, str], key: str):
> +def get_info(info: dict[str, Any], key: str):
> return (info[key] + "\n") if key in info else ""
>
>
> @@ -140,7 +142,7 @@ class ErrorScreen(ModalScreen[bool]):
> def compose(self) -> ComposeResult:
> yield Button(f"Error: {self.error}", variant="primary", id="error")
>
> - def on_button_pressed(self, event: Button.Pressed) -> None:
> + def on_button_pressed(self) -> None:
> self.dismiss(True)
>
>
> @@ -234,7 +236,7 @@ class IListApp(App):
>
> def __init__(self, interval: float) -> None:
> self.interval = interval
> - self.evlist = None
> + self.evlist: Optional[perf.evlist] = None
> self.selected: Optional[TreeValue] = None
> self.search_results: list[TreeNode[TreeValue]] = []
> self.cur_search_result: TreeNode[TreeValue] | None = None
> @@ -284,18 +286,18 @@ class IListApp(App):
> """Sets the focus after the SearchScreen is dismissed."""
>
> search_label = self.query_one("#active_search", Label)
> - search_label.display = True if event else False
> + search_label.display = bool(event)
> if not event:
> return
> event = event.lower()
> search_label.update(f'Searching for events matching "{event}"')
>
> - tree: Tree[str] = self.query_one("#root", Tree)
> + tree: Tree[TreeValue] = self.query_one("#root", Tree)
>
> - def find_search_results(event: str, node: TreeNode[str],
> + def find_search_results(event: str, node: TreeNode[TreeValue],
> cursor_seen: bool = False,
> - match_after_cursor: Optional[TreeNode[str]] = None
> - ) -> Tuple[bool, Optional[TreeNode[str]]]:
> + match_after_cursor: Optional[TreeNode[TreeValue]] = None
> + ) -> tuple[bool, Optional[TreeNode[TreeValue]]]:
> """Find nodes that match the search remembering the one after the cursor."""
> if not cursor_seen and node == tree.cursor_node:
> cursor_seen = True
> @@ -332,7 +334,7 @@ class IListApp(App):
>
> def action_collapse(self) -> None:
> """Collapse the part of the tree currently on."""
> - tree: Tree[str] = self.query_one("#root", Tree)
> + tree: Tree[TreeValue] = self.query_one("#root", Tree)
> node = tree.cursor_node
> if node and node.parent:
> node.parent.collapse_all()
> @@ -343,31 +345,34 @@ class IListApp(App):
> if not self.selected or not self.evlist:
> return
>
> - def update_count(cpu: int, count: int):
> + def update_count(cpu: int, count: int | float):
> # Update the raw count display.
> - counter: Label = self.query(f"#counter_cpu{cpu}" if cpu >= 0 else "#counter_total")
> - if not counter:
> + counter_query = self.query(f"#counter_cpu{cpu}" if cpu >= 0 else "#counter_total")
> + if not counter_query:
> return
> - counter = counter.first(Label)
> + counter: Label = counter_query.first(Label)
> counter.update(str(count))
>
> # Update the sparkline.
> - line: Sparkline = self.query(f"#sparkline_cpu{cpu}" if cpu >= 0 else "#sparkline_total")
> - if not line:
> + line_query = self.query(f"#sparkline_cpu{cpu}" if cpu >= 0 else "#sparkline_total")
> + if not line_query:
> return
> - line = line.first(Sparkline)
> + line: Sparkline = line_query.first(Sparkline)
> # If there are more events than the width, remove the front event.
> - if len(line.data) > line.size.width:
> - line.data.pop(0)
> - line.data.append(count)
> - line.mutate_reactive(Sparkline.data)
> + if line.data is not None:
> + if len(line.data) > line.size.width:
> + line.data = line.data[1:]
> + line.data = list(line.data) + [float(count)]
> + else:
> + line.data = [float(count)]
> + line.refresh()
>
> # Update the total and each CPU counts, assume there's just 1 evsel.
> - total = 0
> + total: float = 0.0
> self.evlist.disable()
> for evsel in self.evlist:
> for cpu in evsel.cpus():
> - aggr = 0
> + aggr: float = 0.0
> for thread in evsel.threads():
> aggr += self.selected.value(self.evlist, evsel, cpu, thread)
> update_count(cpu, aggr)
> @@ -424,16 +429,16 @@ class IListApp(App):
> # Add spark lines for all the CPUs. Note, must be done after
> # open so that the evlist CPUs have been computed by propagate
> # maps.
> - line = CounterSparkline(cpu=-1)
> - lines.mount(line)
> + line_sp = CounterSparkline(cpu=-1)
> + lines.mount(line_sp)
> for cpu in self.evlist.all_cpus():
> - line = CounterSparkline(cpu)
> - lines.mount(line)
> - line = Counter(cpu=-1)
> - lines.mount(line)
> + c_sp = CounterSparkline(cpu)
> + lines.mount(c_sp)
> + c_val = Counter(cpu=-1)
> + lines.mount(c_val)
> for cpu in self.evlist.all_cpus():
> - line = Counter(cpu)
> - lines.mount(line)
> + c_val2 = Counter(cpu)
> + lines.mount(c_val2)
>
> def compose(self) -> ComposeResult:
> """Draws the app."""
> @@ -459,17 +464,18 @@ class IListApp(App):
> # Reading events may fail with EPERM, ignore.
> pass
> metrics = tree.root.add("Metrics")
> - groups = set()
> + groups: set[str] = set()
> for metric in perf.metrics():
> groups.update(metric["MetricGroup"])
>
> - def add_metrics_to_tree(node: TreeNode[TreeValue], parent: str, pmu: str = None):
> + def add_metrics_to_tree(node: TreeNode[TreeValue], parent: str, pmu: Optional[str] = None):
> for metric in sorted(perf.metrics(), key=lambda x: x["MetricName"]):
> - metric_pmu = metric.get('PMU')
> + metric_pmu_raw = metric.get('PMU')
> + metric_pmu = str(metric_pmu_raw) if metric_pmu_raw else ''
> if pmu and metric_pmu and metric_pmu != pmu:
> continue
> if parent in metric["MetricGroup"]:
> - name = metric["MetricName"]
> + name = str(metric["MetricName"])
> display_name = name
> if metric_pmu:
> display_name += f" ({metric_pmu})"
> --
> 2.55.0.1007.g17ff1f9808-goog
prev parent reply other threads:[~2026-09-13 21:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-10 17:44 [PATCH v1] " Ian Rogers
2026-09-10 21:15 ` [PATCH v2] " Ian Rogers
2026-09-13 21:40 ` Arnaldo Carvalho de Melo [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aqcYPoBOEKNk0WMc@x2 \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®