Re: [PATCH v1] perf script flamegraph: Avoid d3-flame-graph package dependency

From: Andreas Gerstmayr
Date: Tue Jan 10 2023 - 13:06:26 EST

On 05.01.23 10:24, Ian Rogers wrote:
On Wed, Jan 4, 2023 at 7:04 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:

Currently flame graph generation requires a d3-flame-graph template to
be installed. Unfortunately this is hard to come by for things like
Debian [1]. If the template isn't installed warn and download it from
jsdelivr CDN. If downloading fails generate a minimal flame graph
again with the javascript coming from jsdelivr CDN.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996839

Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>

I'm not entirely convinced that this perf script should make a network request. In my opinion

* best would be if this template gets packaged in Debian
* alternatively print instructions how to install the template:

sudo mkdir /usr/share/d3-flame-graph
sudo wget -O /usr/share/d3-flame-graph/d3-flamegraph-base.html https://cdn.jsdelivr.net/npm/d3-flame-graph@4/dist/templates/d3-flamegraph-base.html

* or eventually just embed the entire template (66 KB) in the Python file (not sure if this is permissible though, it's basically a minified HTML/JS blob)?
* if the above options don't work, I'd recommend asking the user before making the network request, and eventually persist the template somewhere so it doesn't need to be downloaded every time

What do you think?


Cheers,
Andreas

---
tools/perf/scripts/python/flamegraph.py | 63 ++++++++++++++++++-------
1 file changed, 45 insertions(+), 18 deletions(-)

diff --git a/tools/perf/scripts/python/flamegraph.py b/tools/perf/scripts/python/flamegraph.py
index b6af1dd5f816..808b0e1c9be5 100755
--- a/tools/perf/scripts/python/flamegraph.py
+++ b/tools/perf/scripts/python/flamegraph.py
@@ -25,6 +25,27 @@ import io
import argparse
import json
import subprocess
+import urllib.request
+
+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";>

(hopefully fixed Martin Spier's e-mail address)

The @4.1.3 comes from the README.md:
https://github.com/spiermar/d3-flame-graph/blob/master/README.md
Does it make sense just to drop it or use @latest ? It'd be nice not
to patch this file for every d3-flame-graph update.

Thanks,
Ian

+</head>
+<body>
+ <div id="chart"></div>
+ <script type="text/javascript" src="https://d3js.org/d3.v7.js";></script>
+ <script type="text/javascript" src="https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/d3-flamegraph.min.js";></script>
+ <script type="text/javascript">
+ const stacks = [/** @flamegraph_json **/];
+ // Note, options is unused.
+ const options = [/** @options_json **/];
+
+ var chart = flamegraph();
+ d3.select("#chart")
+ .datum(stacks[0])
+ .call(chart);
+ </script>
+</body>
+"""

# pylint: disable=too-few-public-methods
class Node:
@@ -50,15 +71,18 @@ class FlameGraphCLI:
self.args = args
self.stack = Node("all", "root")

- if self.args.format == "html" and \
- not os.path.isfile(self.args.template):
- print("Flame Graph template {} does not exist. Please install "
- "the js-d3-flame-graph (RPM) or libjs-d3-flame-graph (deb) "
- "package, specify an existing flame graph template "
- "(--template PATH) or another output format "
- "(--format FORMAT).".format(self.args.template),
- file=sys.stderr)
- sys.exit(1)
+ if self.args.format == "html":
+ if os.path.isfile(self.args.template):
+ self.template = f"file://{self.args.template}"
+ else:
+ print(f"""
+Warning: Flame Graph template '{self.args.template}'
+does not exist, a template will be downloaded via http. To avoid this
+please install a package such as the js-d3-flame-graph or
+libjs-d3-flame-graph, specify an existing flame graph template
+(--template PATH) or another output format (--format FORMAT).
+""", file=sys.stderr)
+ self.template = "https://cdn.jsdelivr.net/npm/d3-flame-graph@4.1.3/dist/templates/d3-flamegraph-base.html";

@staticmethod
def get_libtype_from_dso(dso):
@@ -129,15 +153,18 @@ class FlameGraphCLI:
options_json = json.dumps(options)

try:
- with io.open(self.args.template, encoding="utf-8") as template:
- output_str = (
- template.read()
- .replace("/** @options_json **/", options_json)
- .replace("/** @flamegraph_json **/", stacks_json)
- )
- except IOError as err:
- print("Error reading template file: {}".format(err), file=sys.stderr)
- sys.exit(1)
+ with urllib.request.urlopen(self.template) as template:
+ output_str = '\n'.join([
+ l.decode('utf-8') for l in template.readlines()
+ ])
+ except Exception as err:
+ print(f"Error reading template {self.template}: {err}\n"
+ "a minimal flame graph will be generated", file=sys.stderr)
+ output_str = minimal_html
+
+ output_str = output_str.replace("/** @options_json **/", options_json)
+ output_str = output_str.replace("/** @flamegraph_json **/", stacks_json)
+
output_fn = self.args.output or "flamegraph.html"
else:
output_str = stacks_json
--
2.39.0.314.g84b9a713c41-goog



--
Red Hat Austria GmbH, Registered seat: A-1200 Vienna, Millennium Tower, 26.floor, Handelskai 94-96, Austria
Commercial register: Commercial Court Vienna, FN 479668w