The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v1 0/3]  perf jevents: Deterministic build fix and mypy cleanliness
@ 2026-07-07  3:40 Ian Rogers
  2026-07-07  3:40 ` [PATCH v1 1/3] perf jevents: Add more components to the metric sorting order Ian Rogers
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Ian Rogers @ 2026-07-07  3:40 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
	Sandipan Das, Chun-Tse Shao, John Garry, linux-perf-users,
	linux-kernel, Nazar Kazakov

Nazar Kazakov reported non-deterministic builds due to the metrics
being reordered in the jevents.py output. Fix by sorting metrics on
more than just their name.

Checking the code with mypy showed a large range of type warnings. Fix
by largely adding asserts that values aren't None and by adding type
annotations.

Ian Rogers (3):
  perf jevents: Add more components to the metric sorting order
  perf jevents: Add python type annotations
  perf jevents metric: Add python type annotations

 tools/perf/pmu-events/jevents.py | 82 +++++++++++++++++++-------------
 tools/perf/pmu-events/metric.py  | 23 +++++----
 2 files changed, 62 insertions(+), 43 deletions(-)

-- 
2.55.0.795.g602f6c329a-goog


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 1/3] perf jevents: Add more components to the metric sorting order
  2026-07-07  3:40 [PATCH v1 0/3] perf jevents: Deterministic build fix and mypy cleanliness Ian Rogers
@ 2026-07-07  3:40 ` Ian Rogers
  2026-07-15 11:22   ` Nazar Kazakov
  2026-07-07  3:40 ` [PATCH v1 2/3] perf jevents: Add python type annotations Ian Rogers
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Ian Rogers @ 2026-07-07  3:40 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
	Sandipan Das, Chun-Tse Shao, John Garry, linux-perf-users,
	linux-kernel, Nazar Kazakov

Nazar Kazakov reported non-deterministic builds due to the metrics
being reordered in the jevents.py output. The metrics were largely
only being sorted by name, add in the expressions and descriptions.

Reported-by: Nazar Kazakov <nazar.kazakov@codethink.co.uk>
Closes: https://lore.kernel.org/linux-perf-users/20260706175624.692736-1-nazar.kazakov@codethink.co.uk/
Fixes: 40769665b63d ("perf jevents: Parse metrics during conversion")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/pmu-events/jevents.py | 5 +++--
 tools/perf/pmu-events/metric.py  | 6 +++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 376dc2d24162..3c6cfeefbd5d 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -570,13 +570,14 @@ static const struct pmu_table_entry {_pending_events_tblname}[] = {{
 def print_pending_metrics() -> None:
   """Optionally close metrics table."""
 
-  def metric_cmp_key(j: JsonEvent) -> Tuple[bool, str, str]:
+  def metric_cmp_key(j: JsonEvent) -> Tuple[str, str, str, str]:
     def fix_none(s: Optional[str]) -> str:
       if s is None:
         return ''
       return s
 
-    return (j.desc is not None, fix_none(j.pmu), fix_none(j.metric_name))
+    return (fix_none(j.pmu), fix_none(j.metric_name), j.metric_expr.ToPerfJson(),
+            fix_none(j.desc))
 
   global _pending_metrics
   if not _pending_metrics:
diff --git a/tools/perf/pmu-events/metric.py b/tools/perf/pmu-events/metric.py
index a91ccb5977f0..11c7162825f4 100644
--- a/tools/perf/pmu-events/metric.py
+++ b/tools/perf/pmu-events/metric.py
@@ -623,7 +623,11 @@ class Metric:
 
   def __lt__(self, other):
     """Sort order."""
-    return self.name < other.name
+    if self.name != other.name:
+      return self.name < other.name
+    if not self.expr.Equals(other.expr):
+      return self.expr.ToPerfJson() < other.expr.ToPerfJson()
+    return self.description < other.description
 
   def AddToMetricGroup(self, group):
     """Callback used when being added to a MetricGroup."""
-- 
2.55.0.795.g602f6c329a-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v1 2/3] perf jevents: Add python type annotations
  2026-07-07  3:40 [PATCH v1 0/3] perf jevents: Deterministic build fix and mypy cleanliness Ian Rogers
  2026-07-07  3:40 ` [PATCH v1 1/3] perf jevents: Add more components to the metric sorting order Ian Rogers
@ 2026-07-07  3:40 ` Ian Rogers
  2026-07-16 16:56   ` Namhyung Kim
  2026-07-07  3:40 ` [PATCH v1 3/3] perf jevents metric: " Ian Rogers
  2026-07-14 16:39 ` [PATCH v1 0/3] perf jevents: Deterministic build fix and mypy cleanliness Ian Rogers
  3 siblings, 1 reply; 11+ messages in thread
From: Ian Rogers @ 2026-07-07  3:40 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
	Sandipan Das, Chun-Tse Shao, John Garry, linux-perf-users,
	linux-kernel, Nazar Kazakov

Make mypy clean.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/pmu-events/jevents.py | 77 +++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 32 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 3c6cfeefbd5d..83a239dd69f5 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -14,27 +14,27 @@ import collections
 # Global command line arguments.
 _args = None
 # List of regular event tables.
-_event_tables = []
+_event_tables: list[str] = []
 # List of event tables generated from "/sys" directories.
-_sys_event_tables = []
+_sys_event_tables: list[str] = []
 # List of regular metric tables.
-_metric_tables = []
+_metric_tables: list[str] = []
 # List of metric tables generated from "/sys" directories.
-_sys_metric_tables = []
+_sys_metric_tables: list[str] = []
 # Mapping between sys event table names and sys metric table names.
 _sys_event_table_to_metric_table_mapping = {}
 # Map from an event name to an architecture standard
 # JsonEvent. Architecture standard events are in json files in the top
 # f'{_args.starting_dir}/{_args.arch}' directory.
-_arch_std_events = {}
+_arch_std_events: Dict[str, "JsonEvent"] = {}
 # Events to write out when the table is closed
-_pending_events = []
+_pending_events: list["JsonEvent"] = []
 # Name of events table to be written out
-_pending_events_tblname = None
+_pending_events_tblname: Optional[str] = None
 # Metrics to write out when the table is closed
-_pending_metrics = []
+_pending_metrics: list["JsonEvent"] = []
 # Name of metrics table to be written out
-_pending_metrics_tblname = None
+_pending_metrics_tblname: Optional[str] = None
 # Global BigCString shared by all structures.
 _bcs = None
 # Map from the name of a metric group to a description of the group.
@@ -210,7 +210,7 @@ class JsonEvent:
       """Convert an int to a string similar to a printf modifier of %#llx."""
       return str(x) if x >= 0 and x < 10 else hex(x)
 
-    def fixdesc(s: str) -> str:
+    def fixdesc(s: Optional[str]) -> Optional[str]:
       """Fix formatting issue for the desc string."""
       if s is None:
         return None
@@ -218,7 +218,7 @@ class JsonEvent:
                                        '. '), '.').replace('\n', '\\n').replace(
                                            '\"', '\\"').replace('\r', '\\r')
 
-    def convert_aggr_mode(aggr_mode: str) -> Optional[str]:
+    def convert_aggr_mode(aggr_mode: Optional[str]) -> Optional[str]:
       """Returns the aggr_mode_class enum value associated with the JSON string."""
       if not aggr_mode:
         return None
@@ -228,7 +228,7 @@ class JsonEvent:
       }
       return aggr_mode_to_enum[aggr_mode]
 
-    def convert_metric_constraint(metric_constraint: str) -> Optional[str]:
+    def convert_metric_constraint(metric_constraint: Optional[str]) -> Optional[str]:
       """Returns the metric_event_groups enum value associated with the JSON string."""
       if not metric_constraint:
         return None
@@ -241,7 +241,7 @@ class JsonEvent:
       }
       return metric_constraint_to_enum[metric_constraint]
 
-    def lookup_msr(num: str) -> Optional[str]:
+    def lookup_msr(num: Optional[str]) -> Optional[str]:
       """Converts the msr number, or first in a list to the appropriate event field."""
       if not num:
         return None
@@ -253,7 +253,7 @@ class JsonEvent:
       }
       return msrmap[int(num.split(',', 1)[0], 0)]
 
-    def real_event(name: str, event: str) -> Optional[str]:
+    def real_event(name: Optional[str], event: Optional[str]) -> Optional[str]:
       """Convert well known event names to an event string otherwise use the event argument."""
       fixed = {
           'inst_retired.any': 'event=0xc0,period=2000003',
@@ -269,7 +269,7 @@ class JsonEvent:
         return fixed[name.lower()]
       return event
 
-    def unit_to_pmu(unit: str) -> Optional[str]:
+    def unit_to_pmu(unit: Optional[str]) -> Optional[str]:
       """Convert a JSON Unit to Linux PMU name."""
       if not unit:
         return 'default_core'
@@ -304,21 +304,21 @@ class JsonEvent:
       return table[unit] if unit in table else f'uncore_{unit.lower()}'
 
     def is_zero(val: str) -> bool:
-        try:
-            if val.startswith('0x'):
-                return int(val, 16) == 0
-            else:
-                return int(val) == 0
-        except e:
-            return False
+      try:
+        if val.startswith('0x'):
+          return int(val, 16) == 0
+        else:
+          return int(val) == 0
+      except:
+        return False
 
     def canonicalize_value(val: str) -> str:
-        try:
-            if val.startswith('0x'):
-                return llx(int(val, 16))
-            return str(int(val))
-        except e:
-            return val
+      try:
+        if val.startswith('0x'):
+          return llx(int(val, 16))
+        return str(int(val))
+      except:
+        return val
 
     eventcode = 0
     if 'EventCode' in jd:
@@ -370,7 +370,8 @@ class JsonEvent:
     if precise and self.desc and '(Precise Event)' not in self.desc:
       extra_desc += ' (Must be precise)' if precise == '2' else (' (Precise '
                                                                  'event)')
-    event = None
+    self.event: Optional[str] = None
+    event: Optional[str] = None
     if configcode is not None:
       event = f'config={llx(configcode)}'
     elif eventidcode is not None:
@@ -455,6 +456,7 @@ class JsonEvent:
         return f'\t/* {s} */\n' if len(s) < 80 else f'\t/* {s[0:80]}... */\n'
 
     s = self.build_c_string(metric)
+    assert _bcs is not None
     return f'{make_comment(s)}\t{{ { _bcs.offsets[s] } }},\n'
 
 
@@ -524,6 +526,7 @@ def print_pending_events() -> None:
     return
 
   global _pending_events_tblname
+  assert _pending_events_tblname is not None
   if _pending_events_tblname.endswith('_sys'):
     global _sys_event_tables
     _sys_event_tables.append(_pending_events_tblname)
@@ -534,7 +537,8 @@ def print_pending_events() -> None:
   first = True
   last_pmu = None
   last_name = None
-  pmus = set()
+  pmus: Set[Tuple[str, str]] = set()
+  assert _args is not None
   for event in sorted(_pending_events, key=event_cmp_key):
     if last_pmu and last_pmu == event.pmu:
       assert event.name != last_name, f"Duplicate event: {last_pmu}/{last_name}/ in {_pending_events_tblname}"
@@ -576,6 +580,7 @@ def print_pending_metrics() -> None:
         return ''
       return s
 
+    assert j.metric_expr is not None
     return (fix_none(j.pmu), fix_none(j.metric_name), j.metric_expr.ToPerfJson(),
             fix_none(j.desc))
 
@@ -584,6 +589,7 @@ def print_pending_metrics() -> None:
     return
 
   global _pending_metrics_tblname
+  assert _pending_metrics_tblname is not None
   if _pending_metrics_tblname.endswith('_sys'):
     global _sys_metric_tables
     _sys_metric_tables.append(_pending_metrics_tblname)
@@ -593,7 +599,8 @@ def print_pending_metrics() -> None:
 
   first = True
   last_pmu = None
-  pmus = set()
+  pmus: Set[Tuple[str, str]] = set()
+  assert _args is not None
   for metric in sorted(_pending_metrics, key=metric_cmp_key):
     if metric.pmu != last_pmu:
       if not first:
@@ -629,7 +636,6 @@ def get_topic(topic: str) -> str:
   return removesuffix(topic, '.json').replace('-', ' ')
 
 def preprocess_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
-
   if item.is_dir():
     return
 
@@ -643,6 +649,7 @@ def preprocess_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
   if not item.is_file() or not item.name.endswith('.json'):
     return
 
+  assert _bcs is not None
   if item.name.endswith('metricgroups.json'):
     metricgroup_descriptions = json.load(open(item.path))
     for mgroup in metricgroup_descriptions:
@@ -704,6 +711,7 @@ def process_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
 
 def print_mapping_table(archs: Sequence[str]) -> None:
   """Read the mapfile and generate the struct from cpuid string to event table."""
+  assert _args is not None
   _args.output_file.write("""
 /* Struct used to make the PMU event table implementation opaque to callers. */
 struct pmu_events_table {
@@ -821,6 +829,7 @@ static const struct pmu_events_map pmu_events_map[] = {
 
 
 def print_metric_table_functions() -> None:
+  assert _args is not None
   _args.output_file.write("""
 const char *pmu_metrics_table__name(const struct pmu_metrics_table *table)
 {
@@ -864,6 +873,7 @@ int pmu_metrics_table__iterate_tables(pmu_metrics_table_iter_t fn, void *data)
 
 
 def print_system_mapping_table() -> None:
+  assert _args is not None
   """C struct mapping table array for tables from /sys directories."""
   _args.output_file.write("""
 struct pmu_sys_events {
@@ -1400,6 +1410,7 @@ int pmu_for_each_sys_metric(pmu_metric_iter_fn fn, void *data)
 """)
 
 def print_metricgroups() -> None:
+  assert _args is not None
   _args.output_file.write("""
 static const int metricgroups[][2] = {
 """)
@@ -1443,6 +1454,7 @@ def main() -> None:
   def ftw(path: str, parents: Sequence[str],
           action: Callable[[Sequence[str], os.DirEntry], None]) -> None:
     """Replicate the directory/file walking behavior of C's file tree walk."""
+    assert _args is not None
     for item in sorted(os.scandir(path), key=lambda e: e.name):
       if _args.model != 'all' and item.is_dir():
         # Check if the model matches one in _args.model.
@@ -1513,6 +1525,7 @@ struct pmu_table_entry {
     preprocess_arch_std_files(arch_path)
     ftw(arch_path, [], preprocess_one_file)
 
+  assert _bcs is not None
   _bcs.compute()
   _args.output_file.write('/* clang-format off */\n')
   if not _args.output_string_file:
-- 
2.55.0.795.g602f6c329a-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v1 3/3] perf jevents metric: Add python type annotations
  2026-07-07  3:40 [PATCH v1 0/3] perf jevents: Deterministic build fix and mypy cleanliness Ian Rogers
  2026-07-07  3:40 ` [PATCH v1 1/3] perf jevents: Add more components to the metric sorting order Ian Rogers
  2026-07-07  3:40 ` [PATCH v1 2/3] perf jevents: Add python type annotations Ian Rogers
@ 2026-07-07  3:40 ` Ian Rogers
  2026-07-14 16:39 ` [PATCH v1 0/3] perf jevents: Deterministic build fix and mypy cleanliness Ian Rogers
  3 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2026-07-07  3:40 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Ian Rogers, Adrian Hunter, James Clark,
	Sandipan Das, Chun-Tse Shao, John Garry, linux-perf-users,
	linux-kernel, Nazar Kazakov

Make mypy clean.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/pmu-events/metric.py | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/perf/pmu-events/metric.py b/tools/perf/pmu-events/metric.py
index 11c7162825f4..ce025675898c 100644
--- a/tools/perf/pmu-events/metric.py
+++ b/tools/perf/pmu-events/metric.py
@@ -276,7 +276,7 @@ class Operator(Expression):
     lhs = self.lhs.Simplify()
     rhs = self.rhs.Simplify()
     if isinstance(lhs, Constant) and isinstance(rhs, Constant):
-      return Constant(ast.literal_eval(lhs + self.operator + rhs))
+      return Constant(ast.literal_eval(lhs.value + self.operator + rhs.value))
 
     if isinstance(self.lhs, Constant):
       if self.operator in ('+', '|') and lhs.value == '0':
@@ -298,7 +298,7 @@ class Operator(Expression):
       if self.operator == '*' and rhs.value == '0':
         return Constant(0)
 
-      if self.operator == '*' and self.rhs.value == '1':
+      if self.operator == '*' and rhs.value == '1':
         return lhs
 
     return Operator(self.operator, lhs, rhs)
@@ -316,9 +316,7 @@ class Operator(Expression):
     if self.Equals(expression):
       return Event(name)
     lhs = self.lhs.Substitute(name, expression)
-    rhs = None
-    if self.rhs:
-      rhs = self.rhs.Substitute(name, expression)
+    rhs = self.rhs.Substitute(name, expression)
     return Operator(self.operator, lhs, rhs)
 
 
@@ -382,7 +380,9 @@ class Function(Expression):
                rhs: Optional[Union[int, float, Expression]] = None):
     self.fn = fn
     self.lhs = _Constify(lhs)
-    self.rhs = _Constify(rhs)
+    self.rhs = None
+    if rhs is not None:
+      self.rhs = _Constify(rhs)
 
   def ToPerfJson(self):
     if self.rhs:
@@ -407,7 +407,8 @@ class Function(Expression):
     return Function(self.fn, lhs, rhs)
 
   def HasExperimentalEvents(self) -> bool:
-    return self.lhs.HasExperimentalEvents() or (self.rhs and self.rhs.HasExperimentalEvents())
+    return (self.lhs.HasExperimentalEvents() or
+            (self.rhs is not None and self.rhs.HasExperimentalEvents()))
 
   def Equals(self, other: Expression) -> bool:
     if isinstance(other, Function):
@@ -683,7 +684,7 @@ class MetricGroup:
 
   def Flatten(self) -> Set[Metric]:
     """Returns a set of all leaf metrics."""
-    result = set()
+    result: Set[Metric] = set()
     for x in self.metric_list:
       result = result.union(x.Flatten())
 
-- 
2.55.0.795.g602f6c329a-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 0/3] perf jevents: Deterministic build fix and mypy cleanliness
  2026-07-07  3:40 [PATCH v1 0/3] perf jevents: Deterministic build fix and mypy cleanliness Ian Rogers
                   ` (2 preceding siblings ...)
  2026-07-07  3:40 ` [PATCH v1 3/3] perf jevents metric: " Ian Rogers
@ 2026-07-14 16:39 ` Ian Rogers
  3 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2026-07-14 16:39 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim, Nazar Kazakov
  Cc: Peter Zijlstra, Ingo Molnar, Jiri Olsa, Adrian Hunter, John Garry,
	James Clark, Sandipan Das, Chun-Tse Shao, linux-perf-users,
	linux-kernel

On Mon, Jul 6, 2026 at 8:40 PM Ian Rogers <irogers@google.com> wrote:
>
> Nazar Kazakov reported non-deterministic builds due to the metrics
> being reordered in the jevents.py output. Fix by sorting metrics on
> more than just their name.
>
> Checking the code with mypy showed a large range of type warnings. Fix
> by largely adding asserts that values aren't None and by adding type
> annotations.

Ping. There's Sashiko feedback on patch 1:
https://sashiko.dev/#/patchset/20260707034019.241762-1-irogers%40google.com
but it basically says that having a metric name with no metric
expression will cause a crash, which is expected behavior.

Nazar, it would be great if you could provide a Tested-by tag :-)

Thanks,
Ian

> Ian Rogers (3):
>   perf jevents: Add more components to the metric sorting order
>   perf jevents: Add python type annotations
>   perf jevents metric: Add python type annotations
>
>  tools/perf/pmu-events/jevents.py | 82 +++++++++++++++++++-------------
>  tools/perf/pmu-events/metric.py  | 23 +++++----
>  2 files changed, 62 insertions(+), 43 deletions(-)
>
> --
> 2.55.0.795.g602f6c329a-goog
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/3] perf jevents: Add more components to the metric sorting order
  2026-07-07  3:40 ` [PATCH v1 1/3] perf jevents: Add more components to the metric sorting order Ian Rogers
@ 2026-07-15 11:22   ` Nazar Kazakov
  2026-07-15 17:49     ` Ian Rogers
  0 siblings, 1 reply; 11+ messages in thread
From: Nazar Kazakov @ 2026-07-15 11:22 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Adrian Hunter, James Clark, Sandipan Das,
	Chun-Tse Shao, John Garry, linux-perf-users, linux-kernel

On 2026-07-07 04:40, Ian Rogers wrote:
> Nazar Kazakov reported non-deterministic builds due to the metrics
> being reordered in the jevents.py output. The metrics were largely
> only being sorted by name, add in the expressions and descriptions.
> 
> Reported-by: Nazar Kazakov <nazar.kazakov@codethink.co.uk>
> Closes: 
> https://lore.kernel.org/linux-perf-users/20260706175624.692736-1-nazar.kazakov@codethink.co.uk/
> Fixes: 40769665b63d ("perf jevents: Parse metrics during conversion")
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/pmu-events/jevents.py | 5 +++--
>  tools/perf/pmu-events/metric.py  | 6 +++++-
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/pmu-events/jevents.py 
> b/tools/perf/pmu-events/jevents.py
> index 376dc2d24162..3c6cfeefbd5d 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -570,13 +570,14 @@ static const struct pmu_table_entry 
> {_pending_events_tblname}[] = {{
>  def print_pending_metrics() -> None:
>    """Optionally close metrics table."""
> 
> -  def metric_cmp_key(j: JsonEvent) -> Tuple[bool, str, str]:
> +  def metric_cmp_key(j: JsonEvent) -> Tuple[str, str, str, str]:
>      def fix_none(s: Optional[str]) -> str:
>        if s is None:
>          return ''
>        return s
> 
> -    return (j.desc is not None, fix_none(j.pmu), 
> fix_none(j.metric_name))
> +    return (fix_none(j.pmu), fix_none(j.metric_name), 
> j.metric_expr.ToPerfJson(),
> +            fix_none(j.desc))
> 
>    global _pending_metrics
>    if not _pending_metrics:
> diff --git a/tools/perf/pmu-events/metric.py 
> b/tools/perf/pmu-events/metric.py
> index a91ccb5977f0..11c7162825f4 100644
> --- a/tools/perf/pmu-events/metric.py
> +++ b/tools/perf/pmu-events/metric.py
> @@ -623,7 +623,11 @@ class Metric:
> 
>    def __lt__(self, other):
>      """Sort order."""
> -    return self.name < other.name
> +    if self.name != other.name:
> +      return self.name < other.name
> +    if not self.expr.Equals(other.expr):
> +      return self.expr.ToPerfJson() < other.expr.ToPerfJson()
> +    return self.description < other.description
> 
>    def AddToMetricGroup(self, group):
>      """Callback used when being added to a MetricGroup."""

Tested-by: Nazar Kazakov <nazar.kazakov@codethink.co.uk>

This fixes the non-reproducibility indeed, thank you!
I haven't tested mypy patches as I've just cherry-picked this one.

Thanks,
Nazar Kazakov

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/3] perf jevents: Add more components to the metric sorting order
  2026-07-15 11:22   ` Nazar Kazakov
@ 2026-07-15 17:49     ` Ian Rogers
  0 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2026-07-15 17:49 UTC (permalink / raw)
  To: Nazar Kazakov
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Jiri Olsa, Adrian Hunter, James Clark, Sandipan Das,
	Chun-Tse Shao, John Garry, linux-perf-users, linux-kernel

On Wed, Jul 15, 2026 at 4:22 AM Nazar Kazakov
<nazar.kazakov@codethink.co.uk> wrote:
>
> On 2026-07-07 04:40, Ian Rogers wrote:
> > Nazar Kazakov reported non-deterministic builds due to the metrics
> > being reordered in the jevents.py output. The metrics were largely
> > only being sorted by name, add in the expressions and descriptions.
> >
> > Reported-by: Nazar Kazakov <nazar.kazakov@codethink.co.uk>
> > Closes:
> > https://lore.kernel.org/linux-perf-users/20260706175624.692736-1-nazar.kazakov@codethink.co.uk/
> > Fixes: 40769665b63d ("perf jevents: Parse metrics during conversion")
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/pmu-events/jevents.py | 5 +++--
> >  tools/perf/pmu-events/metric.py  | 6 +++++-
> >  2 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/pmu-events/jevents.py
> > b/tools/perf/pmu-events/jevents.py
> > index 376dc2d24162..3c6cfeefbd5d 100755
> > --- a/tools/perf/pmu-events/jevents.py
> > +++ b/tools/perf/pmu-events/jevents.py
> > @@ -570,13 +570,14 @@ static const struct pmu_table_entry
> > {_pending_events_tblname}[] = {{
> >  def print_pending_metrics() -> None:
> >    """Optionally close metrics table."""
> >
> > -  def metric_cmp_key(j: JsonEvent) -> Tuple[bool, str, str]:
> > +  def metric_cmp_key(j: JsonEvent) -> Tuple[str, str, str, str]:
> >      def fix_none(s: Optional[str]) -> str:
> >        if s is None:
> >          return ''
> >        return s
> >
> > -    return (j.desc is not None, fix_none(j.pmu),
> > fix_none(j.metric_name))
> > +    return (fix_none(j.pmu), fix_none(j.metric_name),
> > j.metric_expr.ToPerfJson(),
> > +            fix_none(j.desc))
> >
> >    global _pending_metrics
> >    if not _pending_metrics:
> > diff --git a/tools/perf/pmu-events/metric.py
> > b/tools/perf/pmu-events/metric.py
> > index a91ccb5977f0..11c7162825f4 100644
> > --- a/tools/perf/pmu-events/metric.py
> > +++ b/tools/perf/pmu-events/metric.py
> > @@ -623,7 +623,11 @@ class Metric:
> >
> >    def __lt__(self, other):
> >      """Sort order."""
> > -    return self.name < other.name
> > +    if self.name != other.name:
> > +      return self.name < other.name
> > +    if not self.expr.Equals(other.expr):
> > +      return self.expr.ToPerfJson() < other.expr.ToPerfJson()
> > +    return self.description < other.description
> >
> >    def AddToMetricGroup(self, group):
> >      """Callback used when being added to a MetricGroup."""
>
> Tested-by: Nazar Kazakov <nazar.kazakov@codethink.co.uk>
>
> This fixes the non-reproducibility indeed, thank you!
> I haven't tested mypy patches as I've just cherry-picked this one.

Great, thanks!
Ian

> Thanks,
> Nazar Kazakov

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 2/3] perf jevents: Add python type annotations
  2026-07-07  3:40 ` [PATCH v1 2/3] perf jevents: Add python type annotations Ian Rogers
@ 2026-07-16 16:56   ` Namhyung Kim
  2026-07-16 17:25     ` Ian Rogers
  0 siblings, 1 reply; 11+ messages in thread
From: Namhyung Kim @ 2026-07-16 16:56 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Jiri Olsa,
	Adrian Hunter, James Clark, Sandipan Das, Chun-Tse Shao,
	John Garry, linux-perf-users, linux-kernel, Nazar Kazakov

On Mon, Jul 06, 2026 at 08:40:18PM -0700, Ian Rogers wrote:
> Make mypy clean.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/pmu-events/jevents.py | 77 +++++++++++++++++++-------------
>  1 file changed, 45 insertions(+), 32 deletions(-)
> 
> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 3c6cfeefbd5d..83a239dd69f5 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -14,27 +14,27 @@ import collections
>  # Global command line arguments.
>  _args = None
>  # List of regular event tables.
> -_event_tables = []
> +_event_tables: list[str] = []

I got this from one of my test setup.

    GEN     /build/pmu-events/test-empty-pmu-events.c                             
  Traceback (most recent call last):                                              
    File "pmu-events/jevents.py", line 17, in <module>                            
      _event_tables: list[str] = []                                               
  TypeError: 'type' object is not subscriptable                                   
  make[3]: *** [pmu-events/Build:176: /build/pmu-events/test-empty-pmu-events.c] Error 1
  make[2]: *** [Makefile.perf:550: /build/pmu-events/pmu-events-in.o] Error 2     
  make[2]: *** Waiting for unfinished jobs....                                    
  make[1]: *** [Makefile.perf:288: sub-make] Error 2                              
  make: *** [Makefile:76: all] Error 2

Python version was 3.6.15.

I'll process patch 1 for now.

Thanks,
Namhyung


>  # List of event tables generated from "/sys" directories.
> -_sys_event_tables = []
> +_sys_event_tables: list[str] = []
>  # List of regular metric tables.
> -_metric_tables = []
> +_metric_tables: list[str] = []
>  # List of metric tables generated from "/sys" directories.
> -_sys_metric_tables = []
> +_sys_metric_tables: list[str] = []
>  # Mapping between sys event table names and sys metric table names.
>  _sys_event_table_to_metric_table_mapping = {}
>  # Map from an event name to an architecture standard
>  # JsonEvent. Architecture standard events are in json files in the top
>  # f'{_args.starting_dir}/{_args.arch}' directory.
> -_arch_std_events = {}
> +_arch_std_events: Dict[str, "JsonEvent"] = {}
>  # Events to write out when the table is closed
> -_pending_events = []
> +_pending_events: list["JsonEvent"] = []
>  # Name of events table to be written out
> -_pending_events_tblname = None
> +_pending_events_tblname: Optional[str] = None
>  # Metrics to write out when the table is closed
> -_pending_metrics = []
> +_pending_metrics: list["JsonEvent"] = []
>  # Name of metrics table to be written out
> -_pending_metrics_tblname = None
> +_pending_metrics_tblname: Optional[str] = None
>  # Global BigCString shared by all structures.
>  _bcs = None
>  # Map from the name of a metric group to a description of the group.
> @@ -210,7 +210,7 @@ class JsonEvent:
>        """Convert an int to a string similar to a printf modifier of %#llx."""
>        return str(x) if x >= 0 and x < 10 else hex(x)
>  
> -    def fixdesc(s: str) -> str:
> +    def fixdesc(s: Optional[str]) -> Optional[str]:
>        """Fix formatting issue for the desc string."""
>        if s is None:
>          return None
> @@ -218,7 +218,7 @@ class JsonEvent:
>                                         '. '), '.').replace('\n', '\\n').replace(
>                                             '\"', '\\"').replace('\r', '\\r')
>  
> -    def convert_aggr_mode(aggr_mode: str) -> Optional[str]:
> +    def convert_aggr_mode(aggr_mode: Optional[str]) -> Optional[str]:
>        """Returns the aggr_mode_class enum value associated with the JSON string."""
>        if not aggr_mode:
>          return None
> @@ -228,7 +228,7 @@ class JsonEvent:
>        }
>        return aggr_mode_to_enum[aggr_mode]
>  
> -    def convert_metric_constraint(metric_constraint: str) -> Optional[str]:
> +    def convert_metric_constraint(metric_constraint: Optional[str]) -> Optional[str]:
>        """Returns the metric_event_groups enum value associated with the JSON string."""
>        if not metric_constraint:
>          return None
> @@ -241,7 +241,7 @@ class JsonEvent:
>        }
>        return metric_constraint_to_enum[metric_constraint]
>  
> -    def lookup_msr(num: str) -> Optional[str]:
> +    def lookup_msr(num: Optional[str]) -> Optional[str]:
>        """Converts the msr number, or first in a list to the appropriate event field."""
>        if not num:
>          return None
> @@ -253,7 +253,7 @@ class JsonEvent:
>        }
>        return msrmap[int(num.split(',', 1)[0], 0)]
>  
> -    def real_event(name: str, event: str) -> Optional[str]:
> +    def real_event(name: Optional[str], event: Optional[str]) -> Optional[str]:
>        """Convert well known event names to an event string otherwise use the event argument."""
>        fixed = {
>            'inst_retired.any': 'event=0xc0,period=2000003',
> @@ -269,7 +269,7 @@ class JsonEvent:
>          return fixed[name.lower()]
>        return event
>  
> -    def unit_to_pmu(unit: str) -> Optional[str]:
> +    def unit_to_pmu(unit: Optional[str]) -> Optional[str]:
>        """Convert a JSON Unit to Linux PMU name."""
>        if not unit:
>          return 'default_core'
> @@ -304,21 +304,21 @@ class JsonEvent:
>        return table[unit] if unit in table else f'uncore_{unit.lower()}'
>  
>      def is_zero(val: str) -> bool:
> -        try:
> -            if val.startswith('0x'):
> -                return int(val, 16) == 0
> -            else:
> -                return int(val) == 0
> -        except e:
> -            return False
> +      try:
> +        if val.startswith('0x'):
> +          return int(val, 16) == 0
> +        else:
> +          return int(val) == 0
> +      except:
> +        return False
>  
>      def canonicalize_value(val: str) -> str:
> -        try:
> -            if val.startswith('0x'):
> -                return llx(int(val, 16))
> -            return str(int(val))
> -        except e:
> -            return val
> +      try:
> +        if val.startswith('0x'):
> +          return llx(int(val, 16))
> +        return str(int(val))
> +      except:
> +        return val
>  
>      eventcode = 0
>      if 'EventCode' in jd:
> @@ -370,7 +370,8 @@ class JsonEvent:
>      if precise and self.desc and '(Precise Event)' not in self.desc:
>        extra_desc += ' (Must be precise)' if precise == '2' else (' (Precise '
>                                                                   'event)')
> -    event = None
> +    self.event: Optional[str] = None
> +    event: Optional[str] = None
>      if configcode is not None:
>        event = f'config={llx(configcode)}'
>      elif eventidcode is not None:
> @@ -455,6 +456,7 @@ class JsonEvent:
>          return f'\t/* {s} */\n' if len(s) < 80 else f'\t/* {s[0:80]}... */\n'
>  
>      s = self.build_c_string(metric)
> +    assert _bcs is not None
>      return f'{make_comment(s)}\t{{ { _bcs.offsets[s] } }},\n'
>  
>  
> @@ -524,6 +526,7 @@ def print_pending_events() -> None:
>      return
>  
>    global _pending_events_tblname
> +  assert _pending_events_tblname is not None
>    if _pending_events_tblname.endswith('_sys'):
>      global _sys_event_tables
>      _sys_event_tables.append(_pending_events_tblname)
> @@ -534,7 +537,8 @@ def print_pending_events() -> None:
>    first = True
>    last_pmu = None
>    last_name = None
> -  pmus = set()
> +  pmus: Set[Tuple[str, str]] = set()
> +  assert _args is not None
>    for event in sorted(_pending_events, key=event_cmp_key):
>      if last_pmu and last_pmu == event.pmu:
>        assert event.name != last_name, f"Duplicate event: {last_pmu}/{last_name}/ in {_pending_events_tblname}"
> @@ -576,6 +580,7 @@ def print_pending_metrics() -> None:
>          return ''
>        return s
>  
> +    assert j.metric_expr is not None
>      return (fix_none(j.pmu), fix_none(j.metric_name), j.metric_expr.ToPerfJson(),
>              fix_none(j.desc))
>  
> @@ -584,6 +589,7 @@ def print_pending_metrics() -> None:
>      return
>  
>    global _pending_metrics_tblname
> +  assert _pending_metrics_tblname is not None
>    if _pending_metrics_tblname.endswith('_sys'):
>      global _sys_metric_tables
>      _sys_metric_tables.append(_pending_metrics_tblname)
> @@ -593,7 +599,8 @@ def print_pending_metrics() -> None:
>  
>    first = True
>    last_pmu = None
> -  pmus = set()
> +  pmus: Set[Tuple[str, str]] = set()
> +  assert _args is not None
>    for metric in sorted(_pending_metrics, key=metric_cmp_key):
>      if metric.pmu != last_pmu:
>        if not first:
> @@ -629,7 +636,6 @@ def get_topic(topic: str) -> str:
>    return removesuffix(topic, '.json').replace('-', ' ')
>  
>  def preprocess_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
> -
>    if item.is_dir():
>      return
>  
> @@ -643,6 +649,7 @@ def preprocess_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
>    if not item.is_file() or not item.name.endswith('.json'):
>      return
>  
> +  assert _bcs is not None
>    if item.name.endswith('metricgroups.json'):
>      metricgroup_descriptions = json.load(open(item.path))
>      for mgroup in metricgroup_descriptions:
> @@ -704,6 +711,7 @@ def process_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
>  
>  def print_mapping_table(archs: Sequence[str]) -> None:
>    """Read the mapfile and generate the struct from cpuid string to event table."""
> +  assert _args is not None
>    _args.output_file.write("""
>  /* Struct used to make the PMU event table implementation opaque to callers. */
>  struct pmu_events_table {
> @@ -821,6 +829,7 @@ static const struct pmu_events_map pmu_events_map[] = {
>  
>  
>  def print_metric_table_functions() -> None:
> +  assert _args is not None
>    _args.output_file.write("""
>  const char *pmu_metrics_table__name(const struct pmu_metrics_table *table)
>  {
> @@ -864,6 +873,7 @@ int pmu_metrics_table__iterate_tables(pmu_metrics_table_iter_t fn, void *data)
>  
>  
>  def print_system_mapping_table() -> None:
> +  assert _args is not None
>    """C struct mapping table array for tables from /sys directories."""
>    _args.output_file.write("""
>  struct pmu_sys_events {
> @@ -1400,6 +1410,7 @@ int pmu_for_each_sys_metric(pmu_metric_iter_fn fn, void *data)
>  """)
>  
>  def print_metricgroups() -> None:
> +  assert _args is not None
>    _args.output_file.write("""
>  static const int metricgroups[][2] = {
>  """)
> @@ -1443,6 +1454,7 @@ def main() -> None:
>    def ftw(path: str, parents: Sequence[str],
>            action: Callable[[Sequence[str], os.DirEntry], None]) -> None:
>      """Replicate the directory/file walking behavior of C's file tree walk."""
> +    assert _args is not None
>      for item in sorted(os.scandir(path), key=lambda e: e.name):
>        if _args.model != 'all' and item.is_dir():
>          # Check if the model matches one in _args.model.
> @@ -1513,6 +1525,7 @@ struct pmu_table_entry {
>      preprocess_arch_std_files(arch_path)
>      ftw(arch_path, [], preprocess_one_file)
>  
> +  assert _bcs is not None
>    _bcs.compute()
>    _args.output_file.write('/* clang-format off */\n')
>    if not _args.output_string_file:
> -- 
> 2.55.0.795.g602f6c329a-goog
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 2/3] perf jevents: Add python type annotations
  2026-07-16 16:56   ` Namhyung Kim
@ 2026-07-16 17:25     ` Ian Rogers
  2026-07-16 18:23       ` Namhyung Kim
  0 siblings, 1 reply; 11+ messages in thread
From: Ian Rogers @ 2026-07-16 17:25 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Jiri Olsa,
	Adrian Hunter, James Clark, Sandipan Das, Chun-Tse Shao,
	John Garry, linux-perf-users, linux-kernel, Nazar Kazakov

On Thu, Jul 16, 2026 at 9:56 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Mon, Jul 06, 2026 at 08:40:18PM -0700, Ian Rogers wrote:
> > Make mypy clean.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/pmu-events/jevents.py | 77 +++++++++++++++++++-------------
> >  1 file changed, 45 insertions(+), 32 deletions(-)
> >
> > diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> > index 3c6cfeefbd5d..83a239dd69f5 100755
> > --- a/tools/perf/pmu-events/jevents.py
> > +++ b/tools/perf/pmu-events/jevents.py
> > @@ -14,27 +14,27 @@ import collections
> >  # Global command line arguments.
> >  _args = None
> >  # List of regular event tables.
> > -_event_tables = []
> > +_event_tables: list[str] = []
>
> I got this from one of my test setup.
>
>     GEN     /build/pmu-events/test-empty-pmu-events.c
>   Traceback (most recent call last):
>     File "pmu-events/jevents.py", line 17, in <module>
>       _event_tables: list[str] = []
>   TypeError: 'type' object is not subscriptable
>   make[3]: *** [pmu-events/Build:176: /build/pmu-events/test-empty-pmu-events.c] Error 1
>   make[2]: *** [Makefile.perf:550: /build/pmu-events/pmu-events-in.o] Error 2
>   make[2]: *** Waiting for unfinished jobs....
>   make[1]: *** [Makefile.perf:288: sub-make] Error 2
>   make: *** [Makefile:76: all] Error 2
>
> Python version was 3.6.15.

I believe the minimum Python version should be 3.9.x:
https://docs.kernel.org/process/changes.html
That version will resolve this issue.

Thanks,
Ian

> I'll process patch 1 for now.
>
> Thanks,
> Namhyung
>
>
> >  # List of event tables generated from "/sys" directories.
> > -_sys_event_tables = []
> > +_sys_event_tables: list[str] = []
> >  # List of regular metric tables.
> > -_metric_tables = []
> > +_metric_tables: list[str] = []
> >  # List of metric tables generated from "/sys" directories.
> > -_sys_metric_tables = []
> > +_sys_metric_tables: list[str] = []
> >  # Mapping between sys event table names and sys metric table names.
> >  _sys_event_table_to_metric_table_mapping = {}
> >  # Map from an event name to an architecture standard
> >  # JsonEvent. Architecture standard events are in json files in the top
> >  # f'{_args.starting_dir}/{_args.arch}' directory.
> > -_arch_std_events = {}
> > +_arch_std_events: Dict[str, "JsonEvent"] = {}
> >  # Events to write out when the table is closed
> > -_pending_events = []
> > +_pending_events: list["JsonEvent"] = []
> >  # Name of events table to be written out
> > -_pending_events_tblname = None
> > +_pending_events_tblname: Optional[str] = None
> >  # Metrics to write out when the table is closed
> > -_pending_metrics = []
> > +_pending_metrics: list["JsonEvent"] = []
> >  # Name of metrics table to be written out
> > -_pending_metrics_tblname = None
> > +_pending_metrics_tblname: Optional[str] = None
> >  # Global BigCString shared by all structures.
> >  _bcs = None
> >  # Map from the name of a metric group to a description of the group.
> > @@ -210,7 +210,7 @@ class JsonEvent:
> >        """Convert an int to a string similar to a printf modifier of %#llx."""
> >        return str(x) if x >= 0 and x < 10 else hex(x)
> >
> > -    def fixdesc(s: str) -> str:
> > +    def fixdesc(s: Optional[str]) -> Optional[str]:
> >        """Fix formatting issue for the desc string."""
> >        if s is None:
> >          return None
> > @@ -218,7 +218,7 @@ class JsonEvent:
> >                                         '. '), '.').replace('\n', '\\n').replace(
> >                                             '\"', '\\"').replace('\r', '\\r')
> >
> > -    def convert_aggr_mode(aggr_mode: str) -> Optional[str]:
> > +    def convert_aggr_mode(aggr_mode: Optional[str]) -> Optional[str]:
> >        """Returns the aggr_mode_class enum value associated with the JSON string."""
> >        if not aggr_mode:
> >          return None
> > @@ -228,7 +228,7 @@ class JsonEvent:
> >        }
> >        return aggr_mode_to_enum[aggr_mode]
> >
> > -    def convert_metric_constraint(metric_constraint: str) -> Optional[str]:
> > +    def convert_metric_constraint(metric_constraint: Optional[str]) -> Optional[str]:
> >        """Returns the metric_event_groups enum value associated with the JSON string."""
> >        if not metric_constraint:
> >          return None
> > @@ -241,7 +241,7 @@ class JsonEvent:
> >        }
> >        return metric_constraint_to_enum[metric_constraint]
> >
> > -    def lookup_msr(num: str) -> Optional[str]:
> > +    def lookup_msr(num: Optional[str]) -> Optional[str]:
> >        """Converts the msr number, or first in a list to the appropriate event field."""
> >        if not num:
> >          return None
> > @@ -253,7 +253,7 @@ class JsonEvent:
> >        }
> >        return msrmap[int(num.split(',', 1)[0], 0)]
> >
> > -    def real_event(name: str, event: str) -> Optional[str]:
> > +    def real_event(name: Optional[str], event: Optional[str]) -> Optional[str]:
> >        """Convert well known event names to an event string otherwise use the event argument."""
> >        fixed = {
> >            'inst_retired.any': 'event=0xc0,period=2000003',
> > @@ -269,7 +269,7 @@ class JsonEvent:
> >          return fixed[name.lower()]
> >        return event
> >
> > -    def unit_to_pmu(unit: str) -> Optional[str]:
> > +    def unit_to_pmu(unit: Optional[str]) -> Optional[str]:
> >        """Convert a JSON Unit to Linux PMU name."""
> >        if not unit:
> >          return 'default_core'
> > @@ -304,21 +304,21 @@ class JsonEvent:
> >        return table[unit] if unit in table else f'uncore_{unit.lower()}'
> >
> >      def is_zero(val: str) -> bool:
> > -        try:
> > -            if val.startswith('0x'):
> > -                return int(val, 16) == 0
> > -            else:
> > -                return int(val) == 0
> > -        except e:
> > -            return False
> > +      try:
> > +        if val.startswith('0x'):
> > +          return int(val, 16) == 0
> > +        else:
> > +          return int(val) == 0
> > +      except:
> > +        return False
> >
> >      def canonicalize_value(val: str) -> str:
> > -        try:
> > -            if val.startswith('0x'):
> > -                return llx(int(val, 16))
> > -            return str(int(val))
> > -        except e:
> > -            return val
> > +      try:
> > +        if val.startswith('0x'):
> > +          return llx(int(val, 16))
> > +        return str(int(val))
> > +      except:
> > +        return val
> >
> >      eventcode = 0
> >      if 'EventCode' in jd:
> > @@ -370,7 +370,8 @@ class JsonEvent:
> >      if precise and self.desc and '(Precise Event)' not in self.desc:
> >        extra_desc += ' (Must be precise)' if precise == '2' else (' (Precise '
> >                                                                   'event)')
> > -    event = None
> > +    self.event: Optional[str] = None
> > +    event: Optional[str] = None
> >      if configcode is not None:
> >        event = f'config={llx(configcode)}'
> >      elif eventidcode is not None:
> > @@ -455,6 +456,7 @@ class JsonEvent:
> >          return f'\t/* {s} */\n' if len(s) < 80 else f'\t/* {s[0:80]}... */\n'
> >
> >      s = self.build_c_string(metric)
> > +    assert _bcs is not None
> >      return f'{make_comment(s)}\t{{ { _bcs.offsets[s] } }},\n'
> >
> >
> > @@ -524,6 +526,7 @@ def print_pending_events() -> None:
> >      return
> >
> >    global _pending_events_tblname
> > +  assert _pending_events_tblname is not None
> >    if _pending_events_tblname.endswith('_sys'):
> >      global _sys_event_tables
> >      _sys_event_tables.append(_pending_events_tblname)
> > @@ -534,7 +537,8 @@ def print_pending_events() -> None:
> >    first = True
> >    last_pmu = None
> >    last_name = None
> > -  pmus = set()
> > +  pmus: Set[Tuple[str, str]] = set()
> > +  assert _args is not None
> >    for event in sorted(_pending_events, key=event_cmp_key):
> >      if last_pmu and last_pmu == event.pmu:
> >        assert event.name != last_name, f"Duplicate event: {last_pmu}/{last_name}/ in {_pending_events_tblname}"
> > @@ -576,6 +580,7 @@ def print_pending_metrics() -> None:
> >          return ''
> >        return s
> >
> > +    assert j.metric_expr is not None
> >      return (fix_none(j.pmu), fix_none(j.metric_name), j.metric_expr.ToPerfJson(),
> >              fix_none(j.desc))
> >
> > @@ -584,6 +589,7 @@ def print_pending_metrics() -> None:
> >      return
> >
> >    global _pending_metrics_tblname
> > +  assert _pending_metrics_tblname is not None
> >    if _pending_metrics_tblname.endswith('_sys'):
> >      global _sys_metric_tables
> >      _sys_metric_tables.append(_pending_metrics_tblname)
> > @@ -593,7 +599,8 @@ def print_pending_metrics() -> None:
> >
> >    first = True
> >    last_pmu = None
> > -  pmus = set()
> > +  pmus: Set[Tuple[str, str]] = set()
> > +  assert _args is not None
> >    for metric in sorted(_pending_metrics, key=metric_cmp_key):
> >      if metric.pmu != last_pmu:
> >        if not first:
> > @@ -629,7 +636,6 @@ def get_topic(topic: str) -> str:
> >    return removesuffix(topic, '.json').replace('-', ' ')
> >
> >  def preprocess_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
> > -
> >    if item.is_dir():
> >      return
> >
> > @@ -643,6 +649,7 @@ def preprocess_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
> >    if not item.is_file() or not item.name.endswith('.json'):
> >      return
> >
> > +  assert _bcs is not None
> >    if item.name.endswith('metricgroups.json'):
> >      metricgroup_descriptions = json.load(open(item.path))
> >      for mgroup in metricgroup_descriptions:
> > @@ -704,6 +711,7 @@ def process_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
> >
> >  def print_mapping_table(archs: Sequence[str]) -> None:
> >    """Read the mapfile and generate the struct from cpuid string to event table."""
> > +  assert _args is not None
> >    _args.output_file.write("""
> >  /* Struct used to make the PMU event table implementation opaque to callers. */
> >  struct pmu_events_table {
> > @@ -821,6 +829,7 @@ static const struct pmu_events_map pmu_events_map[] = {
> >
> >
> >  def print_metric_table_functions() -> None:
> > +  assert _args is not None
> >    _args.output_file.write("""
> >  const char *pmu_metrics_table__name(const struct pmu_metrics_table *table)
> >  {
> > @@ -864,6 +873,7 @@ int pmu_metrics_table__iterate_tables(pmu_metrics_table_iter_t fn, void *data)
> >
> >
> >  def print_system_mapping_table() -> None:
> > +  assert _args is not None
> >    """C struct mapping table array for tables from /sys directories."""
> >    _args.output_file.write("""
> >  struct pmu_sys_events {
> > @@ -1400,6 +1410,7 @@ int pmu_for_each_sys_metric(pmu_metric_iter_fn fn, void *data)
> >  """)
> >
> >  def print_metricgroups() -> None:
> > +  assert _args is not None
> >    _args.output_file.write("""
> >  static const int metricgroups[][2] = {
> >  """)
> > @@ -1443,6 +1454,7 @@ def main() -> None:
> >    def ftw(path: str, parents: Sequence[str],
> >            action: Callable[[Sequence[str], os.DirEntry], None]) -> None:
> >      """Replicate the directory/file walking behavior of C's file tree walk."""
> > +    assert _args is not None
> >      for item in sorted(os.scandir(path), key=lambda e: e.name):
> >        if _args.model != 'all' and item.is_dir():
> >          # Check if the model matches one in _args.model.
> > @@ -1513,6 +1525,7 @@ struct pmu_table_entry {
> >      preprocess_arch_std_files(arch_path)
> >      ftw(arch_path, [], preprocess_one_file)
> >
> > +  assert _bcs is not None
> >    _bcs.compute()
> >    _args.output_file.write('/* clang-format off */\n')
> >    if not _args.output_string_file:
> > --
> > 2.55.0.795.g602f6c329a-goog
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 2/3] perf jevents: Add python type annotations
  2026-07-16 17:25     ` Ian Rogers
@ 2026-07-16 18:23       ` Namhyung Kim
  2026-07-16 21:36         ` Ian Rogers
  0 siblings, 1 reply; 11+ messages in thread
From: Namhyung Kim @ 2026-07-16 18:23 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Jiri Olsa,
	Adrian Hunter, James Clark, Sandipan Das, Chun-Tse Shao,
	John Garry, linux-perf-users, linux-kernel, Nazar Kazakov

On Thu, Jul 16, 2026 at 10:25:01AM -0700, Ian Rogers wrote:
> On Thu, Jul 16, 2026 at 9:56 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Mon, Jul 06, 2026 at 08:40:18PM -0700, Ian Rogers wrote:
> > > Make mypy clean.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > >  tools/perf/pmu-events/jevents.py | 77 +++++++++++++++++++-------------
> > >  1 file changed, 45 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> > > index 3c6cfeefbd5d..83a239dd69f5 100755
> > > --- a/tools/perf/pmu-events/jevents.py
> > > +++ b/tools/perf/pmu-events/jevents.py
> > > @@ -14,27 +14,27 @@ import collections
> > >  # Global command line arguments.
> > >  _args = None
> > >  # List of regular event tables.
> > > -_event_tables = []
> > > +_event_tables: list[str] = []
> >
> > I got this from one of my test setup.
> >
> >     GEN     /build/pmu-events/test-empty-pmu-events.c
> >   Traceback (most recent call last):
> >     File "pmu-events/jevents.py", line 17, in <module>
> >       _event_tables: list[str] = []
> >   TypeError: 'type' object is not subscriptable
> >   make[3]: *** [pmu-events/Build:176: /build/pmu-events/test-empty-pmu-events.c] Error 1
> >   make[2]: *** [Makefile.perf:550: /build/pmu-events/pmu-events-in.o] Error 2
> >   make[2]: *** Waiting for unfinished jobs....
> >   make[1]: *** [Makefile.perf:288: sub-make] Error 2
> >   make: *** [Makefile:76: all] Error 2
> >
> > Python version was 3.6.15.
> 
> I believe the minimum Python version should be 3.9.x:
> https://docs.kernel.org/process/changes.html
> That version will resolve this issue.

Hmm.. ok.  I'll re-add them then.

Thanks,
Namhyung


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 2/3] perf jevents: Add python type annotations
  2026-07-16 18:23       ` Namhyung Kim
@ 2026-07-16 21:36         ` Ian Rogers
  0 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2026-07-16 21:36 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Jiri Olsa,
	Adrian Hunter, James Clark, Sandipan Das, Chun-Tse Shao,
	John Garry, linux-perf-users, linux-kernel, Nazar Kazakov

On Thu, Jul 16, 2026 at 11:23 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, Jul 16, 2026 at 10:25:01AM -0700, Ian Rogers wrote:
> > On Thu, Jul 16, 2026 at 9:56 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > On Mon, Jul 06, 2026 at 08:40:18PM -0700, Ian Rogers wrote:
> > > > Make mypy clean.
> > > >
> > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > > ---
> > > >  tools/perf/pmu-events/jevents.py | 77 +++++++++++++++++++-------------
> > > >  1 file changed, 45 insertions(+), 32 deletions(-)
> > > >
> > > > diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> > > > index 3c6cfeefbd5d..83a239dd69f5 100755
> > > > --- a/tools/perf/pmu-events/jevents.py
> > > > +++ b/tools/perf/pmu-events/jevents.py
> > > > @@ -14,27 +14,27 @@ import collections
> > > >  # Global command line arguments.
> > > >  _args = None
> > > >  # List of regular event tables.
> > > > -_event_tables = []
> > > > +_event_tables: list[str] = []
> > >
> > > I got this from one of my test setup.
> > >
> > >     GEN     /build/pmu-events/test-empty-pmu-events.c
> > >   Traceback (most recent call last):
> > >     File "pmu-events/jevents.py", line 17, in <module>
> > >       _event_tables: list[str] = []
> > >   TypeError: 'type' object is not subscriptable
> > >   make[3]: *** [pmu-events/Build:176: /build/pmu-events/test-empty-pmu-events.c] Error 1
> > >   make[2]: *** [Makefile.perf:550: /build/pmu-events/pmu-events-in.o] Error 2
> > >   make[2]: *** Waiting for unfinished jobs....
> > >   make[1]: *** [Makefile.perf:288: sub-make] Error 2
> > >   make: *** [Makefile:76: all] Error 2
> > >
> > > Python version was 3.6.15.
> >
> > I believe the minimum Python version should be 3.9.x:
> > https://docs.kernel.org/process/changes.html
> > That version will resolve this issue.
>
> Hmm.. ok.  I'll re-add them then.

Thanks! Fwiw, Sashiko should also keep us honest here by checking the
minimum tool version as we instruct it to read that from the
documentation:
https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/build.md

Thanks,
Ian

> Thanks,
> Namhyung
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-07-16 21:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07  3:40 [PATCH v1 0/3] perf jevents: Deterministic build fix and mypy cleanliness Ian Rogers
2026-07-07  3:40 ` [PATCH v1 1/3] perf jevents: Add more components to the metric sorting order Ian Rogers
2026-07-15 11:22   ` Nazar Kazakov
2026-07-15 17:49     ` Ian Rogers
2026-07-07  3:40 ` [PATCH v1 2/3] perf jevents: Add python type annotations Ian Rogers
2026-07-16 16:56   ` Namhyung Kim
2026-07-16 17:25     ` Ian Rogers
2026-07-16 18:23       ` Namhyung Kim
2026-07-16 21:36         ` Ian Rogers
2026-07-07  3:40 ` [PATCH v1 3/3] perf jevents metric: " Ian Rogers
2026-07-14 16:39 ` [PATCH v1 0/3] perf jevents: Deterministic build fix and mypy cleanliness Ian Rogers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox