[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fXRr_cTYWhO4=HicrddbwVt=DpmEnvAQMY0sTsgcSx4jQ@mail.gmail.com>
Date: Mon, 9 Jan 2023 11:42:42 -0800
From: Ian Rogers <irogers@...gle.com>
To: Martin Spier <spiermar@...il.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
996839@...s.debian.org, Andreas Gerstmayr <agerstmayr@...hat.com>,
Brendan Gregg <brendan@...el.com>
Subject: Re: [PATCH v1] perf script flamegraph: Avoid d3-flame-graph package dependency
On Fri, Jan 6, 2023 at 7:32 AM Martin Spier <spiermar@...il.com> wrote:
>
> On Thu, Jan 5, 2023 at 1:25 AM Ian Rogers <irogers@...gle.com> wrote:
> >
> > On Wed, Jan 4, 2023 at 7:04 PM Ian Rogers <irogers@...gle.com> 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@...gle.com>
> > > ---
> > > 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
>
> Yes, that's the right email.
>
> Using @latest is an option, but it might be better to just use @4 to
> avoid breaking changes. Not expecting any major releases in the near
> future.
>
> Thanks,
> Martin
Thanks Martin! I'll leave it using @4 then. Could I trouble you for an
Acked-by or Reviewed-by for the change? It would be nice to resolve
the Debian bug.
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
> > >
Powered by blists - more mailing lists