Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy] Restructure JSON profile structure to be parsable #86821

Conversation

ilovepi
Copy link
Contributor

@ilovepi ilovepi commented Mar 27, 2024

The existing JSON output from --enable-profile uses the check names as
keys in the object. This limits the machine readability of the output,
as each key is unique, forcing consumers to split and parse the
substrings of the key itself to organize and extract information.

This change adapts the existing code to output more structured JSON. The
profile is now an array of 'checks', which each have a name that can be
queried and individual times for each of 'wall', 'user', 'sys', 'mem',
and 'instr'. We also make 'mem' and 'sys' non optional, since this makes
parsing uniform. Future patches can triage any inconsistency in the
numeric representation for 'mem' and 'sys', e.g. by using NAN, 0 or
another well defined sentinel value for JSON numbers.

Created using spr 1.3.4
Copy link

github-actions bot commented Mar 27, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
@ilovepi
Copy link
Contributor Author

ilovepi commented Apr 2, 2024

So, when I thought this was only used for printing tidy JSON output, I assumed changing the implementation would only affect users of clang-tidy's profiling info. Since this seems to affect a plist output, I wonder if the old implementation should be kept around and renamed to something more fitting that isn't printJSON*?

@ilovepi ilovepi closed this May 23, 2024
@ilovepi ilovepi deleted the users/ilovepi/spr/clang-tidy-restructure-json-profile-structure-to-be-parsable branch May 23, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant