New architectures bring new complexity, such as Intel's hybrid
models. jevents provides an alternative to specifying events in the
kernel and exposing them through sysfs, however, it is difficult to
work with. For example, an error in the json input would yield an
error message but no json file or location. It is also a challenge to
update jsmn.c given its forked nature.

The changes here switch from jevents.c to a rewrite in python called
jevents.py. This means there is a build time dependency on python, but
such a dependency already exists for asciidoc (used to generate perf's
man pages).

A challenge with this code is in avoiding regressions. For this reason
the jevents.py produces identical output to jevents.c, validated with a
test script and build target.

A difference in the python to the C approach is that the python loads
an entire json file in to memory, while the C code works from token to
token. In some cases the C approach was sensitive to the order of
dictionary items in the json file. To ensure matching output there are
two changes made to jevents.c to cause it to read all values before
creating output.

The changes also found a bug in Ivytown's UNC_M_ACT_COUNT.RD event
encoding, as well as unnecessary whitespace introduced in Alderlake's
metrics. In these cases the json input is fixed.

Ian Rogers (7):
  perf jevents: Append PMU description later
  perf vendor events: Fix Alderlake metric groups
  perf vendor events: Fix Ivytown UNC_M_ACT_COUNT.RD umask
  perf jevents: Modify match field
  perf jevents: Add python converter script
  perf jevents: Switch build to use jevents.py
  perf jevents: Remove jevents.c

 tools/perf/Makefile.perf                      |   13 +-
 tools/perf/pmu-events/Build                   |    9 +-
 .../arch/x86/alderlake/adl-metrics.json       |   32 -
 .../arch/x86/ivytown/uncore-memory.json       |    3 +-
 tools/perf/pmu-events/jevents.c               | 1322 -----------------
 tools/perf/pmu-events/jevents.py              |  392 +++++
 tools/perf/pmu-events/jsmn.h                  |   68 -
 tools/perf/pmu-events/json.c                  |  162 --
 tools/perf/pmu-events/json.h                  |   39 -
 tools/perf/tests/pmu-events.c                 |   30 +-
 10 files changed, 412 insertions(+), 1658 deletions(-)
 delete mode 100644 tools/perf/pmu-events/jevents.c
 create mode 100755 tools/perf/pmu-events/jevents.py
 delete mode 100644 tools/perf/pmu-events/jsmn.h
 delete mode 100644 tools/perf/pmu-events/json.c
 delete mode 100644 tools/perf/pmu-events/json.h

-- 
2.36.0.512.ge40c2bad7a-goog

On 11/05/2022 08:01, Ian Rogers wrote:
> New architectures bring new complexity, such as Intel's hybrid
> models. jevents provides an alternative to specifying events in the
> kernel and exposing them through sysfs, however, it is difficult to
> work with. For example, an error in the json input would yield an
> error message but no json file or location. It is also a challenge to
> update jsmn.c given its forked nature.
> 
> The changes here switch from jevents.c to a rewrite in python called
> jevents.py. This means there is a build time dependency on python, but
> such a dependency already exists for asciidoc (used to generate perf's
> man pages).
> 

Hi Ian,

This does not build for me:

Auto-detecting system features:
...                         dwarf: [ on  ]
...            dwarf_getlocations: [ on  ]
...                         glibc: [ on  ]
...                        libbfd: [ OFF ]
...                libbfd-buildid: [ OFF ]
...                        libcap: [ on  ]
...                        libelf: [ on  ]
...                       libnuma: [ on  ]
...        numa_num_possible_cpus: [ on  ]
...                       libperl: [ on  ]
...                     libpython: [ on  ]
...                     libcrypto: [ on  ]
...                     libunwind: [ on  ]
...            libdw-dwarf-unwind: [ on  ]
...                          zlib: [ on  ]
...                          lzma: [ on  ]
...                     get_cpuid: [ on  ]
...                           bpf: [ on  ]
...                        libaio: [ on  ]
...                       libzstd: [ on  ]
...        disassembler-four-args: [ on  ]


make[3]: Nothing to be done for 'install_headers'.
  GEN     pmu-events/pmu-events.c
  CC      /home/john/acme/tools/perf/libbpf/staticobjs/libbpf.o
  LINK    dlfilters/dlfilter-test-api-v0.so
  CC      dlfilters/dlfilter-show-cycles.o
  CC      builtin-bench.o
Traceback (most recent call last):
  File "pmu-events/jevents.py", line 23, in <module>
    def file_name_to_table_name(parents: list[str], dirname: str) -> str:
TypeError: 'type' object is not subscriptable
make[3]: *** [pmu-events/Build:15: pmu-events/pmu-events.c] Error 1
make[2]: *** [Makefile.perf:662: pmu-events/pmu-events-in.o] Error 2
make[2]: *** Waiting for unfinished jobs....

What baseline do you use? It's always good to mention it I think.

> A challenge with this code is in avoiding regressions. For this reason
> the jevents.py produces identical output to jevents.c, validated with a
> test script and build target.

As you know (and have done), verifying no regression should be 
straightforward by diff'ing. For avoidance of doubt, which archs did you 
test? We also need to ensure those which don't use pmu-events (like 
arm32) work as before.

> 
> A difference in the python to the C approach is that the python loads
> an entire json file in to memory, while the C code works from token to
> token. In some cases the C approach was sensitive to the order of
> dictionary items in the json file. To ensure matching output there are
> two changes made to jevents.c to cause it to read all values before
> creating output.
> 
> The changes also found a bug in Ivytown's UNC_M_ACT_COUNT.RD event
> encoding, as well as unnecessary whitespace introduced in Alderlake's
> metrics. In these cases the json input is fixed.

Thanks,
John

On Wed, May 11, 2022 at 4:13 AM John Garry <john.garry@huawei.com> wrote:
>
> On 11/05/2022 08:01, Ian Rogers wrote:
> > New architectures bring new complexity, such as Intel's hybrid
> > models. jevents provides an alternative to specifying events in the
> > kernel and exposing them through sysfs, however, it is difficult to
> > work with. For example, an error in the json input would yield an
> > error message but no json file or location. It is also a challenge to
> > update jsmn.c given its forked nature.
> >
> > The changes here switch from jevents.c to a rewrite in python called
> > jevents.py. This means there is a build time dependency on python, but
> > such a dependency already exists for asciidoc (used to generate perf's
> > man pages).
> >
>
> Hi Ian,
>
> This does not build for me:
>
> Auto-detecting system features:
> ...                         dwarf: [ on  ]
> ...            dwarf_getlocations: [ on  ]
> ...                         glibc: [ on  ]
> ...                        libbfd: [ OFF ]
> ...                libbfd-buildid: [ OFF ]
> ...                        libcap: [ on  ]
> ...                        libelf: [ on  ]
> ...                       libnuma: [ on  ]
> ...        numa_num_possible_cpus: [ on  ]
> ...                       libperl: [ on  ]
> ...                     libpython: [ on  ]
> ...                     libcrypto: [ on  ]
> ...                     libunwind: [ on  ]
> ...            libdw-dwarf-unwind: [ on  ]
> ...                          zlib: [ on  ]
> ...                          lzma: [ on  ]
> ...                     get_cpuid: [ on  ]
> ...                           bpf: [ on  ]
> ...                        libaio: [ on  ]
> ...                       libzstd: [ on  ]
> ...        disassembler-four-args: [ on  ]
>
>
> make[3]: Nothing to be done for 'install_headers'.
>   GEN     pmu-events/pmu-events.c
>   CC      /home/john/acme/tools/perf/libbpf/staticobjs/libbpf.o
>   LINK    dlfilters/dlfilter-test-api-v0.so
>   CC      dlfilters/dlfilter-show-cycles.o
>   CC      builtin-bench.o
> Traceback (most recent call last):
>   File "pmu-events/jevents.py", line 23, in <module>
>     def file_name_to_table_name(parents: list[str], dirname: str) -> str:
> TypeError: 'type' object is not subscriptable
> make[3]: *** [pmu-events/Build:15: pmu-events/pmu-events.c] Error 1
> make[2]: *** [Makefile.perf:662: pmu-events/pmu-events-in.o] Error 2
> make[2]: *** Waiting for unfinished jobs....
>
> What baseline do you use? It's always good to mention it I think.

You're right, sorry for that. I was testing with:
$ python3 --version
Python 3.9.10

From your output it looks like the newer type hints feature in python
is missing. I think the type hints help the code, but they aren't a
requirement for this change. I'll look into compatibility with older
pythons.

> > A challenge with this code is in avoiding regressions. For this reason
> > the jevents.py produces identical output to jevents.c, validated with a
> > test script and build target.
>
> As you know (and have done), verifying no regression should be
> straightforward by diff'ing. For avoidance of doubt, which archs did you
> test? We also need to ensure those which don't use pmu-events (like
> arm32) work as before.

The test in:
https://lore.kernel.org/linux-perf-users/20220511070133.710721-6-irogers@google.com/
tests all architectures that exist in tools/perf/pmu-eeventss/arch:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/pmu-events/arch?h=perf/core

You're right that I've missed arm32 and other architectures that don't
have a directory there. Something to fix for v2.

Thanks,
Ian

> >
> > A difference in the python to the C approach is that the python loads
> > an entire json file in to memory, while the C code works from token to
> > token. In some cases the C approach was sensitive to the order of
> > dictionary items in the json file. To ensure matching output there are
> > two changes made to jevents.c to cause it to read all values before
> > creating output.
> >
> > The changes also found a bug in Ivytown's UNC_M_ACT_COUNT.RD event
> > encoding, as well as unnecessary whitespace introduced in Alderlake's
> > metrics. In these cases the json input is fixed.
>
> Thanks,
> John

On Wed, May 11, 2022 at 12:01:26AM -0700, Ian Rogers wrote:

> The changes here switch from jevents.c to a rewrite in python called
> jevents.py. This means there is a build time dependency on python, but
> such a dependency already exists for asciidoc (used to generate perf's
> man pages).

You mean just building perf (not the docs) will now require snake stuff?

That's very tedious :/ I don't typically have snakes on my machines.

On Wed, May 11, 2022 at 12:35 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, May 11, 2022 at 12:01:26AM -0700, Ian Rogers wrote:
>
> > The changes here switch from jevents.c to a rewrite in python called
> > jevents.py. This means there is a build time dependency on python, but
> > such a dependency already exists for asciidoc (used to generate perf's
> > man pages).
>
> You mean just building perf (not the docs) will now require snake stuff?
>
> That's very tedious :/ I don't typically have snakes on my machines.

Hi Peter,

You're right that after these changes python is a build requirement
for jevents. We could keep the C code around for the case that python
isn't there, but I want to do things like remove the string
relocations, sort the events by name so we don't linearly search, etc.
which would be a massive chore to keep alive on the C side. An
alternative would be to have an empty pmu-events.c file that is used
for this case. If you wanted to keep things in C and have jevents like
event names, you could use the empty version and link in libpfm4.

Thanks,
Ian

On Wed, May 11, 2022 at 06:50:59AM -0700, Ian Rogers wrote:
> On Wed, May 11, 2022 at 12:35 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, May 11, 2022 at 12:01:26AM -0700, Ian Rogers wrote:
> >
> > > The changes here switch from jevents.c to a rewrite in python called
> > > jevents.py. This means there is a build time dependency on python, but
> > > such a dependency already exists for asciidoc (used to generate perf's
> > > man pages).
> >
> > You mean just building perf (not the docs) will now require snake stuff?
> >
> > That's very tedious :/ I don't typically have snakes on my machines.
> 
> Hi Peter,
> 
> You're right that after these changes python is a build requirement
> for jevents. We could keep the C code around for the case that python
> isn't there, but I want to do things like remove the string
> relocations, sort the events by name so we don't linearly search, etc.
> which would be a massive chore to keep alive on the C side. An
> alternative would be to have an empty pmu-events.c file that is used
> for this case. If you wanted to keep things in C and have jevents like
> event names, you could use the empty version and link in libpfm4.

I'm not normally linking to libpfm4. All I really care about is that I
can still build a bare cli perf (not even tui). If all the snake stuff
is purely optional and it just disables some features, but I do get a
perf out at the end, then I'm all good.

On Wed, May 11, 2022 at 6:56 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, May 11, 2022 at 06:50:59AM -0700, Ian Rogers wrote:
> > On Wed, May 11, 2022 at 12:35 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > On Wed, May 11, 2022 at 12:01:26AM -0700, Ian Rogers wrote:
> > >
> > > > The changes here switch from jevents.c to a rewrite in python called
> > > > jevents.py. This means there is a build time dependency on python, but
> > > > such a dependency already exists for asciidoc (used to generate perf's
> > > > man pages).
> > >
> > > You mean just building perf (not the docs) will now require snake stuff?
> > >
> > > That's very tedious :/ I don't typically have snakes on my machines.
> >
> > Hi Peter,
> >
> > You're right that after these changes python is a build requirement
> > for jevents. We could keep the C code around for the case that python
> > isn't there, but I want to do things like remove the string
> > relocations, sort the events by name so we don't linearly search, etc.
> > which would be a massive chore to keep alive on the C side. An
> > alternative would be to have an empty pmu-events.c file that is used
> > for this case. If you wanted to keep things in C and have jevents like
> > event names, you could use the empty version and link in libpfm4.
>
> I'm not normally linking to libpfm4. All I really care about is that I
> can still build a bare cli perf (not even tui). If all the snake stuff
> is purely optional and it just disables some features, but I do get a
> perf out at the end, then I'm all good.
>

Great! I'll add a conditional "empty" file version for this case.

Thanks,
Ian