On Thu, 2024-10-03 at 00:37 -0700, Andi Kleen wrote:
> > Note that if the user requests SARIF output e.g. with
> >   -fdiagnostics-format=sarif-stderr
> > then any timevar data from -ftime-report is written in JSON form as
> > part of the SARIF, rather than in text form to stderr (see
> > 75d623946d4b6ea80a777b789b116d4b4a2298dc).
> > 
> > I see that the proposed patch leaves the user and sys stats as
> > zero,
> > and conditionalizes what's printed for text output as part of
> > timer::print.  Should it also do something similar in
> > make_json_for_timevar_time_def for the json output, and not add the
> > properties for "user" and "sys" if the data hasn't been gathered?
> 
> > Hope I'm reading the patch correctly.
> 
> Yes that's right.

Thanks.

> 
> I mainly adjusted the human output for cosmetic reasons.
> 
> For machine readable i guess it is better to have a stable schema 
> and not skip fields to avoid pain for parsers. So I left it alone.

The only consumer I know of for the JSON time report data is in the
integration tests I wrote for -fanalyzer, which assumes that all fields
are present when printing, and then goes on to use the "user" times for
summarizing; see this commit FWIW:
https://github.com/davidmalcolm/gcc-analyzer-integration-tests/commit/5420ce968e6eae886e61486555b54fd460e0d35f

I'm not planning to use -ftime-report-wall, but given that my
summarization code is using the "user" times I think I'd prefer it the
property wasn't present rather than contained a bogus value that I
might mistakenly use.  The existing docs do say: "The precise format of
this JSON data is subject to change":
https://gcc.gnu.org/onlinedocs/gcc/Developer-Options.html#index-ftime-report
and there isn't a formal schema for this written down anywhere.

Hope this is constructive
Dave

Reply via email to