lkml.org 
[lkml]   [2022]   [Aug]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v4 07/17] perf jevents: Sort json files entries
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:
>

\
 
 \ /
  Last update: 2022-08-10 16:25    [W:0.081 / U:1.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site