[PATCH] D50129: [XRay][tools] Use Support/JSON.h in llvm-xray convert

[Keith via Phabricator via llvm-commits]

kpw added a comment.

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



================
Comment at: llvm/tools/llvm-xray/xray-converter.cpp:282
+          {"pid", llvm::to_string(Version >= 3 ? R.PId : 1)},
+          {"ts", llvm::formatv("{0:f4}", EventTimestampUs)},
+          {"sf", llvm::to_string(StackCursor->ExtraData.id)},
----------------
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.


================
Comment at: llvm/tools/llvm-xray/xray-converter.cpp:324
+             Symbolize ? FuncIdHelper.SymbolOrNumber(StackFunctionNode->FuncId)
+                       : llvm::to_string(StackFunctionNode->FuncId)}},
+    });
----------------
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.


================
Comment at: llvm/tools/llvm-xray/xray-converter.cpp:337
+  };
+  OS << formatv("{0:2}", json::Value(std::move(TraceJSON)));
 }
----------------
What does {0:2} mean in this context? Is there a Json particular format provider?


https://reviews.llvm.org/D50129