Message-ID: <CAP-5=fX1j_MdzjqLTYeNGFTWLaQp8Dmbi093QN98b2pNgFcpBA@mail.gmail.com>
Date: Wed, 23 Jul 2025 11:15:29 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
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>, 
	Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>, 
	James Clark <james.clark@...aro.org>, Xu Yang <xu.yang_2@....com>, 
	"Masami Hiramatsu (Google)" <mhiramat@...nel.org>, Collin Funk <collin.funk1@...il.com>, 
	Howard Chu <howardchu95@...il.com>, Weilin Wang <weilin.wang@...el.com>, 
	Andi Kleen <ak@...ux.intel.com>, "Dr. David Alan Gilbert" <linux@...blig.org>, 
	Thomas Richter <tmricht@...ux.ibm.com>, Tiezhu Yang <yangtiezhu@...ngson.cn>, 
	Gautam Menghani <gautam@...ux.ibm.com>, Thomas Falcon <thomas.falcon@...el.com>, 
	Chun-Tse Shao <ctshao@...gle.com>, linux-kernel@...r.kernel.org, 
	linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v7 00/16] New perf ilist app

On Wed, Jul 23, 2025 at 11:00 AM Namhyung Kim <namhyung@...nel.org> wrote:
>
> Hi Ian,
>
> On Wed, Jul 23, 2025 at 08:32:33AM -0700, Ian Rogers wrote:
> > On Mon, Jul 14, 2025 at 9:44 AM Ian Rogers <irogers@...gle.com> wrote:
> > >
> > > This patch series builds up to the addition of a new ilist app written
> > > in python using textual [1] for the UI. The app presents perf PMUs and
> > > events, displays the event information as in `perf list` while at the
> > > bottom of the console showing recent activity of the event in total
> > > and across all CPUs. It also displays metrics, placed in a tree
> > > through their metric group, again with counts being displayed in the
> > > bottom panel.
> > >
> > > The first ground work patches of fixes, cleanup and refactoring were
> > > separated into their own series here:
> > > https://lore.kernel.org/lkml/20250709214029.1769089-1-irogers@google.com/
> > >
> > > The second part of the patches adds event json for the software PMU
> > > and makes the tracepoint PMU support iteration of events and the
> > > like. Without these improvements the tracepoint and software PMUs will
> > > appear to have no events in the ilist app. As the software PMU moves
> > > parsing to json, the legacy hard coded parsing is removed. This has
> > > proven controversial for hardware events and so that cleanup isn't
> > > done here.
> > >
> > > The final patches expand the perf python APIs and add the ilist
> > > command. To run it you need the updated perf.cpython.so in your
> > > PYTHONPATH and then execute the script. Expanding PMUs and then
> > > selecting events will cause event informatin to be displayed in the
> > > top-right and the counters values to be displayed as sparklines and
> > > counts in the bottom half of the screen.
> > >
> > > [1] https://textual.textualize.io/
> > >
> > > v7: Better handle errors in the python code and ignore errors when
> > >     scanning PMU/events in ilist.py, improving the behavior when not
> > >     root. Add a tp_pmu/python clean up. Minor kernel coding style
> > >     clean up. Fix behavior of ilist if a search result isn't found but
> > >     then next is chosen.
> > >
> > > v6: For metrics on hybrid systems don't purely match by name, also
> > >     match the CPU and thread so that if the same metric exists for
> > >     different PMUs the appropriate one is selected and counters may be
> > >     read. Likewise use evsel maps and not the evlists.
> > >
> > > v5: Split the series in two. Add metric support. Various clean ups and
> > >     tweaks to the app in particular around the handling of searches.
> > >
> > > v4: No conflict rebase. Picks up perf-tools-next DRM PMU which
> > >     displays as expected.
> > >
> > > v3: Add a search dialog to the ilist app with 'n'ext and 'p'revious
> > >     keys. No changes in the ground work first 14 patches.
> > >
> > > v2: In the jevents event description duplication, some minor changes
> > >     accidentally missed from v1 meaning that in v1 the descriptions
> > >     were still duplicated. Expand the cover letter with some thoughts
> > >     on the series.
> > >
> > > Ian Rogers (16):
> > >   perf python: Add more exceptions on error paths
> > >   perf jevents: Add common software event json
> > >   perf parse-events: Remove non-json software events
> > >   perf tp_pmu: Factor existing tracepoint logic to new file
> > >   perf tp_pmu: Add event APIs
> > >   perf list: Remove tracepoint printing code
> > >   perf list: Skip ABI PMUs when printing pmu values
> > >   perf python: Improve the tracepoint function if no libtraceevent
> > >   perf python: Add basic PMU abstraction and pmus sequence
> > >   perf python: Add function returning dictionary of all events on a PMU
> > >   perf ilist: Add new python ilist command
> > >   perf python: Add parse_metrics function
> > >   perf python: Add evlist metrics function
> > >   perf python: Add evlist compute_metric
> > >   perf python: Add metrics function
> > >   perf ilist: Add support for metrics
> >
> > Hi,
> >
> > Is there any more I can do to get this series landed? I appreciate having:
> >
> > Tested-by: Gautam Menghani <gautam@...ux.ibm.com>
> >
> > I think there is some follow up for "make install" for scripts like
> > these, but I'm keen for the python API to move forward.
>
> I'll review the series today so that we can get some part of it, at
> least.  Basically I think we need a wrapper script like perf-ilist to
> run this easily (maybe with documentation).

Sgtm, I'll hold off on a new version, Arnaldo has caught some useful
nits to clean up in it.

Something related are the scripting engine scripts like gecko and
flamegraph. Something I've discovered is that those scripts work less
well than standalone apps like the ilist.py here. One issue there is
that the scripting engine tends to throw lots of data into a
dictionary in case it is useful, which means there is a higher
overhead than in the standalone app. Another issue is that the
scripting engine wants perf to drive python to drive the script. This
conflicts with python wanting to own the main thread, things like
asyncio, etc. For textual it isn't possible for the app to run on
anything other than the main thread, but it'd be useful to
incrementally load data while displaying it. With the scripting engine
it is only possible to run a textual app at trace_end (i.e. not
incrementally). Something we found in the gecko work was that it was
hard to handle command line arguments in a scripting engine style
script, what options are for record, report and live mode.

What does this mean? I think we want to move the scripting engine into
the python.c and perf python .so and expose it with something like the
session/tool API. I think we should migrate the existing scripts to
use this and then I think we can deprecate the python interpreter
script engine integration. Perhaps we can deprecate perl scripting
engine support at the same time. Then things like ilist.py can be run
with "perf script ilist", similar to how the scripting engine scripts
run today. Your proposal would avoid the need for the script keyword,
but perhaps that'd make it clear why the UI changes. I don't mind
having or not having "script" in the command line :-)

Thanks,
Ian

> Thanks,
> Namhyung
>
> > >
> > >  tools/perf/builtin-list.c                     |  65 ++-
> > >  .../arch/common/common/software.json          |  92 ++++
> > >  tools/perf/pmu-events/empty-pmu-events.c      | 266 +++++----
> > >  tools/perf/pmu-events/jevents.py              |  15 +-
> > >  tools/perf/python/ilist.py                    | 491 +++++++++++++++++
> > >  tools/perf/util/Build                         |   1 +
> > >  tools/perf/util/evsel.c                       |  21 +-
> > >  tools/perf/util/parse-events.c                | 198 ++-----
> > >  tools/perf/util/parse-events.h                |   1 -
> > >  tools/perf/util/parse-events.l                |  38 +-
> > >  tools/perf/util/parse-events.y                |  29 +-
> > >  tools/perf/util/pfm.c                         |   2 +
> > >  tools/perf/util/pmu.c                         |   7 +
> > >  tools/perf/util/pmus.c                        |   2 +
> > >  tools/perf/util/print-events.c                | 100 +---
> > >  tools/perf/util/print-events.h                |   4 +-
> > >  tools/perf/util/python.c                      | 519 +++++++++++++++++-
> > >  tools/perf/util/tp_pmu.c                      | 209 +++++++
> > >  tools/perf/util/tp_pmu.h                      |  19 +
> > >  19 files changed, 1638 insertions(+), 441 deletions(-)
> > >  create mode 100644 tools/perf/pmu-events/arch/common/common/software.json
> > >  create mode 100755 tools/perf/python/ilist.py
> > >  create mode 100644 tools/perf/util/tp_pmu.c
> > >  create mode 100644 tools/perf/util/tp_pmu.h
> > >
> > > --
> > > 2.50.0.727.gbf7dc18ff4-goog
> > >

Powered by blists - more mailing lists