From: Ian Rogers <irogers@google.com>
To: John Garry <john.garry@huawei.com>
Cc: Will Deacon <will@kernel.org>, James Clark <james.clark@arm.com>,
Mike Leach <mike.leach@linaro.org>, Leo Yan <leo.yan@linaro.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
Andi Kleen <ak@linux.intel.com>,
Zhengjun Xing <zhengjun.xing@linux.intel.com>,
Ravi Bangoria <ravi.bangoria@amd.com>,
Kan Liang <kan.liang@linux.intel.com>,
Adrian Hunter <adrian.hunter@intel.com>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-perf-users@vger.kernel.org,
Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH v4 07/17] perf jevents: Sort json files entries
Date: Wed, 10 Aug 2022 07:23:53 -0700 [thread overview]
Message-ID: <CAP-5=fVWx9ROmHiOYJdBRvQNCdoUxf83dY8nBOsxMe5Xb-ofKQ@mail.gmail.com> (raw)
In-Reply-To: <ad2ad328-89f9-7dea-a6fd-b473da2f1ad0@huawei.com>
On Fri, Aug 5, 2022 at 3:49 AM John Garry <john.garry@huawei.com> wrote:
>
> On 04/08/2022 23:18, Ian Rogers wrote:
> > Sort the json files entries on conversion to C. The sort order tries to
> > replicated cmp_sevent from pmu.c so that the input there is already
>
> replicate
Ack.
> > sorted except for sysfs events.
> >
> > Add the topic to JsonEvent on reading to simplify. Remove an unnecessary
> > lambda in the json reading.
>
> We sort the attributes of the events alphabetically by attribute name,
> right? Is there any advantage in this? Do we need it for later?
The sort order is given by the tuple:
(not j.desc is None, fix_none(j.topic), fix_none(j.name),
fix_none(j.pmu), fix_none(j.metric_name))
which is putting events with descriptions and topics before those
without, then sorting by name, then pmu and finally metric_name. The
advantage is that when we qsort alias events:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/pmu.c?h=perf/core#n1759
the events are already in the sorted format, which should make the
code faster - it still has to qsort the sysfs events.
Longer term I'd like to make the searching for an event, or metric, by
name a binary rather than a linear search.
Thanks,
Ian
> thanks,
> John
>
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/pmu-events/jevents.py | 48 +++++++++++++++++++++++---------
> > 1 file changed, 35 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> > index 12d2daf3570c..30e0e792221a 100755
> > --- a/tools/perf/pmu-events/jevents.py
> > +++ b/tools/perf/pmu-events/jevents.py
> > @@ -18,6 +18,8 @@ _sys_event_tables = []
> > _arch_std_events = {}
> > # Track whether an events table is currently being defined and needs closing.
> > _close_table = False
> > +# Events to write out when the table is closed
> > +_pending_events = []
> >
> >
> > def removesuffix(s: str, suffix: str) -> str:
> > @@ -127,6 +129,7 @@ class JsonEvent:
> > eventcode |= int(jd['ExtSel']) << 8
> > configcode = int(jd['ConfigCode'], 0) if 'ConfigCode' in jd else None
> > self.name = jd['EventName'].lower() if 'EventName' in jd else None
> > + self.topic = ''
> > self.compat = jd.get('Compat')
> > self.desc = fixdesc(jd.get('BriefDescription'))
> > self.long_desc = fixdesc(jd.get('PublicDescription'))
> > @@ -199,7 +202,7 @@ class JsonEvent:
> > s += f'\t{attr} = {value},\n'
> > return s + '}'
> >
> > - def to_c_string(self, topic_local: str) -> str:
> > + def to_c_string(self) -> str:
> > """Representation of the event as a C struct initializer."""
> >
> > def attr_string(attr: str, value: str) -> str:
> > @@ -211,25 +214,27 @@ class JsonEvent:
> > return attr_string(attr, getattr(self, attr))
> >
> > s = '{\n'
> > - s += f'\t.topic = "{topic_local}",\n'
> > for attr in [
> > 'aggr_mode', 'compat', 'deprecated', 'desc', 'event', 'long_desc',
> > 'metric_constraint', 'metric_expr', 'metric_group', 'metric_name',
> > - 'name', 'perpkg', 'pmu', 'unit'
> > + 'name', 'perpkg', 'pmu', 'topic', 'unit'
> > ]:
> > s += str_if_present(self, attr)
> > s += '},\n'
> > return s
> >
> >
> > -def read_json_events(path: str) -> Sequence[JsonEvent]:
> > +def read_json_events(path: str, topic: str) -> Sequence[JsonEvent]:
> > """Read json events from the specified file."""
> >
> > try:
> > - return json.load(open(path), object_hook=lambda d: JsonEvent(d))
> > + result = json.load(open(path), object_hook=JsonEvent)
> > except BaseException as err:
> > print(f"Exception processing {path}")
> > raise
> > + for event in result:
> > + event.topic = topic
> > + return result
> >
> >
> > def preprocess_arch_std_files(archpath: str) -> None:
> > @@ -237,7 +242,7 @@ def preprocess_arch_std_files(archpath: str) -> None:
> > global _arch_std_events
> > for item in os.scandir(archpath):
> > if item.is_file() and item.name.endswith('.json'):
> > - for event in read_json_events(item.path):
> > + for event in read_json_events(item.path, topic=''):
> > if event.name:
> > _arch_std_events[event.name.lower()] = event
> >
> > @@ -251,19 +256,36 @@ def print_events_table_prefix(tblname: str) -> None:
> > _close_table = True
> >
> >
> > -def print_events_table_entries(item: os.DirEntry, topic: str) -> None:
> > - """Create contents of an events table."""
> > +def add_events_table_entries(item: os.DirEntry, topic: str) -> None:
> > + """Add contents of file to _pending_events table."""
> > if not _close_table:
> > raise IOError('Table entries missing prefix')
> > - for event in read_json_events(item.path):
> > - _args.output_file.write(event.to_c_string(topic))
> > + for e in read_json_events(item.path, topic):
> > + _pending_events.append(e)
> >
> >
> > def print_events_table_suffix() -> None:
> > """Optionally close events table."""
> > +
> > + def event_cmp_key(j: JsonEvent):
> > + def fix_none(s: str):
> > + if s is None:
> > + return ''
> > + return s
> > +
> > + return (not j.desc is None, fix_none(j.topic), fix_none(j.name), fix_none(j.pmu),
> > + fix_none(j.metric_name))
> > +
> > global _close_table
> > - if _close_table:
> > - _args.output_file.write("""{
> > + if not _close_table:
> > + return
> > +
> > + global _pending_events
> > + for event in sorted(_pending_events, key=event_cmp_key):
> > + _args.output_file.write(event.to_c_string())
> > + _pending_events = []
> > +
> > + _args.output_file.write("""{
> > \t.name = 0,
> > \t.event = 0,
> > \t.desc = 0,
> > @@ -306,7 +328,7 @@ def process_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
> > if not item.is_file() or not item.name.endswith('.json'):
> > return
> >
> > - print_events_table_entries(item, get_topic(item.name))
> > + add_events_table_entries(item, get_topic(item.name))
> >
> >
> > def print_mapping_table(archs: Sequence[str]) -> None:
>
next prev parent reply other threads:[~2022-08-10 14:24 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-04 22:17 [PATCH v4 00/17] Compress the pmu_event tables Ian Rogers
2022-08-04 22:18 ` [PATCH v4 01/17] perf jevents: Clean up pytype warnings Ian Rogers
2022-08-09 19:36 ` Arnaldo Carvalho de Melo
2022-08-04 22:18 ` [PATCH v4 02/17] perf jevents: Simplify generation of C-string Ian Rogers
2022-08-09 19:37 ` Arnaldo Carvalho de Melo
2022-08-04 22:18 ` [PATCH v4 03/17] perf jevents: Add JEVENTS_ARCH make option Ian Rogers
2022-08-05 9:55 ` John Garry
2022-08-09 19:38 ` Arnaldo Carvalho de Melo
2022-08-04 22:18 ` [PATCH v4 04/17] perf jevent: Add an 'all' architecture argument Ian Rogers
2022-08-05 10:34 ` John Garry
2022-08-04 22:18 ` [PATCH v4 05/17] perf jevents: Remove the type/version variables Ian Rogers
2022-08-05 10:35 ` John Garry
2022-08-04 22:18 ` [PATCH v4 06/17] perf jevents: Provide path to json file on error Ian Rogers
2022-08-04 22:18 ` [PATCH v4 07/17] perf jevents: Sort json files entries Ian Rogers
2022-08-05 10:49 ` John Garry
2022-08-10 14:23 ` Ian Rogers [this message]
2022-08-10 15:53 ` John Garry
2022-08-04 22:18 ` [PATCH v4 08/17] perf pmu-events: Hide pmu_sys_event_tables Ian Rogers
2022-08-05 11:15 ` John Garry
2022-08-10 14:25 ` Ian Rogers
2022-08-04 22:18 ` [PATCH v4 09/17] perf pmu-events: Avoid passing pmu_events_map Ian Rogers
2022-08-05 11:28 ` John Garry
2022-08-10 15:23 ` Ian Rogers
2022-08-04 22:18 ` [PATCH v4 10/17] perf pmu-events: Hide pmu_events_map Ian Rogers
2022-08-05 12:31 ` John Garry
2022-08-10 14:29 ` Ian Rogers
2022-08-11 14:47 ` John Garry
2022-08-04 22:18 ` [PATCH v4 11/17] perf test: Use full metric resolution Ian Rogers
2022-08-04 22:18 ` [PATCH v4 12/17] perf pmu-events: Move test events/metrics to json Ian Rogers
2022-08-05 12:34 ` John Garry
2022-08-04 22:18 ` [PATCH v4 13/17] perf pmu-events: Don't assume pmu_event is an array Ian Rogers
2022-08-05 12:56 ` John Garry
2022-08-04 22:18 ` [PATCH v4 14/17] perf pmu-events: Hide the pmu_events Ian Rogers
2022-08-04 22:18 ` [PATCH v4 15/17] perf metrics: Copy entire pmu_event in find metric Ian Rogers
2022-08-04 22:18 ` [PATCH v4 16/17] perf jevents: Compress the pmu_events_table Ian Rogers
2022-08-05 14:02 ` John Garry
2022-08-10 14:35 ` Ian Rogers
2022-08-04 22:18 ` [PATCH v4 17/17] perf jevents: Fold strings optimization Ian Rogers
2022-08-09 19:40 ` [PATCH v4 00/17] Compress the pmu_event tables Arnaldo Carvalho de Melo
2022-08-09 19:41 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAP-5=fVWx9ROmHiOYJdBRvQNCdoUxf83dY8nBOsxMe5Xb-ofKQ@mail.gmail.com' \
--to=irogers@google.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=eranian@google.com \
--cc=james.clark@arm.com \
--cc=john.garry@huawei.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=leo.yan@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mike.leach@linaro.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=ravi.bangoria@amd.com \
--cc=will@kernel.org \
--cc=zhengjun.xing@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).