This is an archive of the discontinued LLVM Phabricator instance.

[XRay][tools] Use Support/JSON.h in llvm-xray convert
ClosedPublic

Authored by dberris on Jul 31 2018, 9:47 PM.

Details

Summary

This change removes the ad-hoc implementation used by llvm-xray's
convert subcommand to generate JSON encoded catapult (AKA Chrome
Trace Viewer) trace output, to instead use the JSON encoder now in the
Support library.

Diff Detail

Repository
rL LLVM

Event Timeline

dberris created this revision.Jul 31 2018, 9:47 PM
kpw added a comment.Aug 2 2018, 11:22 PM

This is way easier to read now. Did you sanity check loading a trace into chrome trace viewer that it outputs?

llvm/tools/llvm-xray/xray-converter.cpp
282 ↗(On Diff #158462)

Strange that the code you're replacing changed the precision between 4 and 3 depending on the version. Four seems like a better choice and matches the behavior for version >= 3.

324 ↗(On Diff #158462)

Is it meaningful to have the trailing comma just before the end of the initializer list?

I am kind of surprised to see that this works.

337 ↗(On Diff #158462)

What does {0:2} mean in this context? Is there a Json particular format provider?

dberris updated this revision to Diff 158935.Aug 3 2018, 12:25 AM
dberris marked 3 inline comments as done.

Add comment on the format line for JSON.

In D50129#1186823, @kpw wrote:

This is way easier to read now. Did you sanity check loading a trace into chrome trace viewer that it outputs?

Yep!

llvm/tools/llvm-xray/xray-converter.cpp
324 ↗(On Diff #158462)

Not really, but that's valid C++, and is not creating an initializer list (but rather is initializing a KV).

337 ↗(On Diff #158462)

Yep, this pretty-prints with indents of two characters.

kpw accepted this revision.Aug 3 2018, 1:34 AM
This revision is now accepted and ready to land.Aug 3 2018, 1:34 AM
This revision was automatically updated to reflect the committed changes.