linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] perf flamegraph: Fix minor pylint/type hint issues
@ 2025-07-16  0:46 Ian Rogers
  2025-07-16 17:43 ` Namhyung Kim
  2025-07-18  0:06 ` Namhyung Kim
  0 siblings, 2 replies; 3+ messages in thread
From: Ian Rogers @ 2025-07-16  0:46 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Ian Rogers, Adrian Hunter, Kan Liang, Wangyang Guo, Tim Chen,
	Zhiguo Zhou, Tianyou Li, linux-perf-users, linux-kernel

Switch to assuming python3. Fix minor pylint issues on line length,
repeated compares, not using f-strings and variable case. Add type
hints and check with mypy.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/scripts/python/flamegraph.py | 61 +++++++++++++------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/tools/perf/scripts/python/flamegraph.py b/tools/perf/scripts/python/flamegraph.py
index e49ff242b779..ad735990c5be 100755
--- a/tools/perf/scripts/python/flamegraph.py
+++ b/tools/perf/scripts/python/flamegraph.py
@@ -18,7 +18,6 @@
 # pylint: disable=missing-class-docstring
 # pylint: disable=missing-function-docstring
 
-from __future__ import print_function
 import argparse
 import hashlib
 import io
@@ -26,9 +25,10 @@ import json
 import os
 import subprocess
 import sys
+from typing import Dict, Optional, Union
 import urllib.request
 
-minimal_html = """<head>
+MINIMAL_HTML = """<head>
   <link rel="stylesheet" type="text/css" href="https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/d3-flamegraph.css">
 </head>
 <body>
@@ -50,20 +50,20 @@ minimal_html = """<head>
 
 # pylint: disable=too-few-public-methods
 class Node:
-    def __init__(self, name, libtype):
+    def __init__(self, name: str, libtype: str):
         self.name = name
         # "root" | "kernel" | ""
         # "" indicates user space
         self.libtype = libtype
-        self.value = 0
-        self.children = []
+        self.value: int = 0
+        self.children: list[Node] = []
 
-    def to_json(self):
+    def to_json(self) -> Dict[str, Union[str, int, list[Dict]]]:
         return {
             "n": self.name,
             "l": self.libtype,
             "v": self.value,
-            "c": self.children
+            "c": [x.to_json() for x in self.children]
         }
 
 
@@ -73,7 +73,7 @@ class FlameGraphCLI:
         self.stack = Node("all", "root")
 
     @staticmethod
-    def get_libtype_from_dso(dso):
+    def get_libtype_from_dso(dso: Optional[str]) -> str:
         """
         when kernel-debuginfo is installed,
         dso points to /usr/lib/debug/lib/modules/*/vmlinux
@@ -84,7 +84,7 @@ class FlameGraphCLI:
         return ""
 
     @staticmethod
-    def find_or_create_node(node, name, libtype):
+    def find_or_create_node(node: Node, name: str, libtype: str) -> Node:
         for child in node.children:
             if child.name == name:
                 return child
@@ -93,7 +93,7 @@ class FlameGraphCLI:
         node.children.append(child)
         return child
 
-    def process_event(self, event):
+    def process_event(self, event) -> None:
         # ignore events where the event name does not match
         # the one specified by the user
         if self.args.event_name and event.get("ev_name") != self.args.event_name:
@@ -106,7 +106,7 @@ class FlameGraphCLI:
             comm = event["comm"]
             libtype = "kernel"
         else:
-            comm = "{} ({})".format(event["comm"], pid)
+            comm = f"{event['comm']} ({pid})"
             libtype = ""
         node = self.find_or_create_node(self.stack, comm, libtype)
 
@@ -121,7 +121,7 @@ class FlameGraphCLI:
             node = self.find_or_create_node(node, name, libtype)
         node.value += 1
 
-    def get_report_header(self):
+    def get_report_header(self) -> str:
         if self.args.input == "-":
             # when this script is invoked with "perf script flamegraph",
             # no perf.data is created and we cannot read the header of it
@@ -131,7 +131,8 @@ class FlameGraphCLI:
             # if the file name other than perf.data is given,
             # we read the header of that file
             if self.args.input:
-                output = subprocess.check_output(["perf", "report", "--header-only", "-i", self.args.input])
+                output = subprocess.check_output(["perf", "report", "--header-only",
+                                                  "-i", self.args.input])
             else:
                 output = subprocess.check_output(["perf", "report", "--header-only"])
 
@@ -140,10 +141,10 @@ class FlameGraphCLI:
                 result += "\nFocused event: " + self.args.event_name
             return result
         except Exception as err:  # pylint: disable=broad-except
-            print("Error reading report header: {}".format(err), file=sys.stderr)
+            print(f"Error reading report header: {err}", file=sys.stderr)
             return ""
 
-    def trace_end(self):
+    def trace_end(self) -> None:
         stacks_json = json.dumps(self.stack, default=lambda x: x.to_json())
 
         if self.args.format == "html":
@@ -167,7 +168,8 @@ graph template (--template PATH) or use another output format (--format
 FORMAT).""",
                               file=sys.stderr)
                         if self.args.input == "-":
-                            print("""Not attempting to download Flame Graph template as script command line
+                            print(
+"""Not attempting to download Flame Graph template as script command line
 input is disabled due to using live mode. If you want to download the
 template retry without live mode. For example, use 'perf record -a -g
 -F 99 sleep 60' and 'perf script report flamegraph'. Alternatively,
@@ -176,37 +178,40 @@ https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/templates/d3-flamegraph-b
 and place it at:
 /usr/share/d3-flame-graph/d3-flamegraph-base.html""",
                                   file=sys.stderr)
-                            quit()
+                            sys.exit(1)
                         s = None
-                        while s != "y" and s != "n":
-                            s = input("Do you wish to download a template from cdn.jsdelivr.net? (this warning can be suppressed with --allow-download) [yn] ").lower()
+                        while s not in ["y", "n"]:
+                            s = input("Do you wish to download a template from cdn.jsdelivr.net?" +
+                                      "(this warning can be suppressed with --allow-download) [yn] "
+                                      ).lower()
                         if s == "n":
-                            quit()
-                    template = "https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/templates/d3-flamegraph-base.html"
+                            sys.exit(1)
+                    template = ("https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/templates/"
+                                "d3-flamegraph-base.html")
                     template_md5sum = "143e0d06ba69b8370b9848dcd6ae3f36"
 
             try:
-                with urllib.request.urlopen(template) as template:
+                with urllib.request.urlopen(template) as url_template:
                     output_str = "".join([
-                        l.decode("utf-8") for l in template.readlines()
+                        l.decode("utf-8") for l in url_template.readlines()
                     ])
             except Exception as err:
                 print(f"Error reading template {template}: {err}\n"
                       "a minimal flame graph will be generated", file=sys.stderr)
-                output_str = minimal_html
+                output_str = MINIMAL_HTML
                 template_md5sum = None
 
             if template_md5sum:
                 download_md5sum = hashlib.md5(output_str.encode("utf-8")).hexdigest()
                 if download_md5sum != template_md5sum:
                     s = None
-                    while s != "y" and s != "n":
+                    while s not in ["y", "n"]:
                         s = input(f"""Unexpected template md5sum.
 {download_md5sum} != {template_md5sum}, for:
 {output_str}
 continue?[yn] """).lower()
                     if s == "n":
-                        quit()
+                        sys.exit(1)
 
             output_str = output_str.replace("/** @options_json **/", options_json)
             output_str = output_str.replace("/** @flamegraph_json **/", stacks_json)
@@ -220,12 +225,12 @@ continue?[yn] """).lower()
             with io.open(sys.stdout.fileno(), "w", encoding="utf-8", closefd=False) as out:
                 out.write(output_str)
         else:
-            print("dumping data to {}".format(output_fn))
+            print(f"dumping data to {output_fn}")
             try:
                 with io.open(output_fn, "w", encoding="utf-8") as out:
                     out.write(output_str)
             except IOError as err:
-                print("Error writing output file: {}".format(err), file=sys.stderr)
+                print(f"Error writing output file: {err}", file=sys.stderr)
                 sys.exit(1)
 
 
-- 
2.50.0.727.gbf7dc18ff4-goog


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

* Re: [PATCH v1] perf flamegraph: Fix minor pylint/type hint issues
  2025-07-16  0:46 [PATCH v1] perf flamegraph: Fix minor pylint/type hint issues Ian Rogers
@ 2025-07-16 17:43 ` Namhyung Kim
  2025-07-18  0:06 ` Namhyung Kim
  1 sibling, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2025-07-16 17:43 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Wangyang Guo, Tim Chen, Zhiguo Zhou, Tianyou Li,
	linux-perf-users, linux-kernel

On Tue, Jul 15, 2025 at 05:46:35PM -0700, Ian Rogers wrote:
> Switch to assuming python3. Fix minor pylint issues on line length,
> repeated compares, not using f-strings and variable case. Add type
> hints and check with mypy.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>

I've confirmed that it generated the exactly same output.

Tested-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  tools/perf/scripts/python/flamegraph.py | 61 +++++++++++++------------
>  1 file changed, 33 insertions(+), 28 deletions(-)
> 
> diff --git a/tools/perf/scripts/python/flamegraph.py b/tools/perf/scripts/python/flamegraph.py
> index e49ff242b779..ad735990c5be 100755
> --- a/tools/perf/scripts/python/flamegraph.py
> +++ b/tools/perf/scripts/python/flamegraph.py
> @@ -18,7 +18,6 @@
>  # pylint: disable=missing-class-docstring
>  # pylint: disable=missing-function-docstring
>  
> -from __future__ import print_function
>  import argparse
>  import hashlib
>  import io
> @@ -26,9 +25,10 @@ import json
>  import os
>  import subprocess
>  import sys
> +from typing import Dict, Optional, Union
>  import urllib.request
>  
> -minimal_html = """<head>
> +MINIMAL_HTML = """<head>
>    <link rel="stylesheet" type="text/css" href="https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/d3-flamegraph.css">
>  </head>
>  <body>
> @@ -50,20 +50,20 @@ minimal_html = """<head>
>  
>  # pylint: disable=too-few-public-methods
>  class Node:
> -    def __init__(self, name, libtype):
> +    def __init__(self, name: str, libtype: str):
>          self.name = name
>          # "root" | "kernel" | ""
>          # "" indicates user space
>          self.libtype = libtype
> -        self.value = 0
> -        self.children = []
> +        self.value: int = 0
> +        self.children: list[Node] = []
>  
> -    def to_json(self):
> +    def to_json(self) -> Dict[str, Union[str, int, list[Dict]]]:
>          return {
>              "n": self.name,
>              "l": self.libtype,
>              "v": self.value,
> -            "c": self.children
> +            "c": [x.to_json() for x in self.children]
>          }
>  
>  
> @@ -73,7 +73,7 @@ class FlameGraphCLI:
>          self.stack = Node("all", "root")
>  
>      @staticmethod
> -    def get_libtype_from_dso(dso):
> +    def get_libtype_from_dso(dso: Optional[str]) -> str:
>          """
>          when kernel-debuginfo is installed,
>          dso points to /usr/lib/debug/lib/modules/*/vmlinux
> @@ -84,7 +84,7 @@ class FlameGraphCLI:
>          return ""
>  
>      @staticmethod
> -    def find_or_create_node(node, name, libtype):
> +    def find_or_create_node(node: Node, name: str, libtype: str) -> Node:
>          for child in node.children:
>              if child.name == name:
>                  return child
> @@ -93,7 +93,7 @@ class FlameGraphCLI:
>          node.children.append(child)
>          return child
>  
> -    def process_event(self, event):
> +    def process_event(self, event) -> None:
>          # ignore events where the event name does not match
>          # the one specified by the user
>          if self.args.event_name and event.get("ev_name") != self.args.event_name:
> @@ -106,7 +106,7 @@ class FlameGraphCLI:
>              comm = event["comm"]
>              libtype = "kernel"
>          else:
> -            comm = "{} ({})".format(event["comm"], pid)
> +            comm = f"{event['comm']} ({pid})"
>              libtype = ""
>          node = self.find_or_create_node(self.stack, comm, libtype)
>  
> @@ -121,7 +121,7 @@ class FlameGraphCLI:
>              node = self.find_or_create_node(node, name, libtype)
>          node.value += 1
>  
> -    def get_report_header(self):
> +    def get_report_header(self) -> str:
>          if self.args.input == "-":
>              # when this script is invoked with "perf script flamegraph",
>              # no perf.data is created and we cannot read the header of it
> @@ -131,7 +131,8 @@ class FlameGraphCLI:
>              # if the file name other than perf.data is given,
>              # we read the header of that file
>              if self.args.input:
> -                output = subprocess.check_output(["perf", "report", "--header-only", "-i", self.args.input])
> +                output = subprocess.check_output(["perf", "report", "--header-only",
> +                                                  "-i", self.args.input])
>              else:
>                  output = subprocess.check_output(["perf", "report", "--header-only"])
>  
> @@ -140,10 +141,10 @@ class FlameGraphCLI:
>                  result += "\nFocused event: " + self.args.event_name
>              return result
>          except Exception as err:  # pylint: disable=broad-except
> -            print("Error reading report header: {}".format(err), file=sys.stderr)
> +            print(f"Error reading report header: {err}", file=sys.stderr)
>              return ""
>  
> -    def trace_end(self):
> +    def trace_end(self) -> None:
>          stacks_json = json.dumps(self.stack, default=lambda x: x.to_json())
>  
>          if self.args.format == "html":
> @@ -167,7 +168,8 @@ graph template (--template PATH) or use another output format (--format
>  FORMAT).""",
>                                file=sys.stderr)
>                          if self.args.input == "-":
> -                            print("""Not attempting to download Flame Graph template as script command line
> +                            print(
> +"""Not attempting to download Flame Graph template as script command line
>  input is disabled due to using live mode. If you want to download the
>  template retry without live mode. For example, use 'perf record -a -g
>  -F 99 sleep 60' and 'perf script report flamegraph'. Alternatively,
> @@ -176,37 +178,40 @@ https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/templates/d3-flamegraph-b
>  and place it at:
>  /usr/share/d3-flame-graph/d3-flamegraph-base.html""",
>                                    file=sys.stderr)
> -                            quit()
> +                            sys.exit(1)
>                          s = None
> -                        while s != "y" and s != "n":
> -                            s = input("Do you wish to download a template from cdn.jsdelivr.net? (this warning can be suppressed with --allow-download) [yn] ").lower()
> +                        while s not in ["y", "n"]:
> +                            s = input("Do you wish to download a template from cdn.jsdelivr.net?" +
> +                                      "(this warning can be suppressed with --allow-download) [yn] "
> +                                      ).lower()
>                          if s == "n":
> -                            quit()
> -                    template = "https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/templates/d3-flamegraph-base.html"
> +                            sys.exit(1)
> +                    template = ("https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/templates/"
> +                                "d3-flamegraph-base.html")
>                      template_md5sum = "143e0d06ba69b8370b9848dcd6ae3f36"
>  
>              try:
> -                with urllib.request.urlopen(template) as template:
> +                with urllib.request.urlopen(template) as url_template:
>                      output_str = "".join([
> -                        l.decode("utf-8") for l in template.readlines()
> +                        l.decode("utf-8") for l in url_template.readlines()
>                      ])
>              except Exception as err:
>                  print(f"Error reading template {template}: {err}\n"
>                        "a minimal flame graph will be generated", file=sys.stderr)
> -                output_str = minimal_html
> +                output_str = MINIMAL_HTML
>                  template_md5sum = None
>  
>              if template_md5sum:
>                  download_md5sum = hashlib.md5(output_str.encode("utf-8")).hexdigest()
>                  if download_md5sum != template_md5sum:
>                      s = None
> -                    while s != "y" and s != "n":
> +                    while s not in ["y", "n"]:
>                          s = input(f"""Unexpected template md5sum.
>  {download_md5sum} != {template_md5sum}, for:
>  {output_str}
>  continue?[yn] """).lower()
>                      if s == "n":
> -                        quit()
> +                        sys.exit(1)
>  
>              output_str = output_str.replace("/** @options_json **/", options_json)
>              output_str = output_str.replace("/** @flamegraph_json **/", stacks_json)
> @@ -220,12 +225,12 @@ continue?[yn] """).lower()
>              with io.open(sys.stdout.fileno(), "w", encoding="utf-8", closefd=False) as out:
>                  out.write(output_str)
>          else:
> -            print("dumping data to {}".format(output_fn))
> +            print(f"dumping data to {output_fn}")
>              try:
>                  with io.open(output_fn, "w", encoding="utf-8") as out:
>                      out.write(output_str)
>              except IOError as err:
> -                print("Error writing output file: {}".format(err), file=sys.stderr)
> +                print(f"Error writing output file: {err}", file=sys.stderr)
>                  sys.exit(1)
>  
>  
> -- 
> 2.50.0.727.gbf7dc18ff4-goog
> 

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

* Re: [PATCH v1] perf flamegraph: Fix minor pylint/type hint issues
  2025-07-16  0:46 [PATCH v1] perf flamegraph: Fix minor pylint/type hint issues Ian Rogers
  2025-07-16 17:43 ` Namhyung Kim
@ 2025-07-18  0:06 ` Namhyung Kim
  1 sibling, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2025-07-18  0:06 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Kan Liang, Wangyang Guo, Tim Chen, Zhiguo Zhou, Tianyou Li,
	linux-perf-users, linux-kernel, Ian Rogers

On Tue, 15 Jul 2025 17:46:35 -0700, Ian Rogers wrote:
> Switch to assuming python3. Fix minor pylint issues on line length,
> repeated compares, not using f-strings and variable case. Add type
> hints and check with mypy.
> 
> 
Applied to perf-tools-next, thanks!

Best regards,
Namhyung



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

end of thread, other threads:[~2025-07-18  0:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-16  0:46 [PATCH v1] perf flamegraph: Fix minor pylint/type hint issues Ian Rogers
2025-07-16 17:43 ` Namhyung Kim
2025-07-18  0:06 ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).