lkml.org 
[lkml]   [2021]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] perf data: Add JSON export
From
Date
Hi Jiri,

Thanks again for the review. I've fixed the issues you mentioned. Some notes
below:


On 2021-04-01 8:15 a.m., Jiri Olsa wrote:
> I recall you did not add support for walltime clock,
> don't you need it to sync with other events?

Not necessarily. As long as the perf recording and the GPU trace are made from
the same clock, the events should line up; the real time doesn't matter. I've
added it anyway though since it's good to have. It now outputs "clockid",
"clock-time" and "real-time" in the headers if clock info was included in the
recording.


On 2021-04-01 9:18 a.m., Jiri Olsa wrote:
> also I will not push hard for test, becase we don't have any for CTF ;-)
> but if you could think of any, that'd be great

I'm not sure what kind of tests would be useful. We could include a small
perf.data file and its JSON output and verify that they match. This is probably
pointless because almost any change to the JSON code would be expected to
change the output so we'd just be resetting the test on each change.


> I was wondering how to make this \t mess more readable,
> how about you define function like output_json:
>
> output_json(FILE, level, field, format, ...);
>
> and use it:
>
> output_json(c->out, 3, "data-offset", "PRIu64", header->data_offset);
> output_json(c->out, 3, "data-size", "PRIu64", header->data_size);
> output_json(c->out, 3, "feat-offset", PRIu64, header->feat_offset);
>
> similar way as we do for pr_debug -> eprintf

I've cleaned up the output with some additional helper functions. This is
essentially turning into its own mini JSON encoder which I was hoping to avoid
but I suppose it's necessary to make the code maintainable.


I'll be sending a new patch shortly.

Nick

\
 
 \ /
  Last update: 2021-04-06 11:32    [W:0.051 / U:0.468 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site