[PATCH] D60609: Use native llvm JSON library for time profiler output

[Anton Afanasyev via Phabricator via llvm-commits]

anton-afanasyev marked an inline comment as done.
anton-afanasyev added inline comments.


================
Comment at: llvm/lib/Support/TimeProfiler.cpp:91
 
-    OS << "{ \"traceEvents\": [\n";
+    json::Array Events;
 
----------------
sammccall wrote:
> Sorry, I was vaguely aware of this patch but hadn't looked at the detail.
> As there's likely to be lots of events, I'd suggest still writing the `{"traceEvents":[` / `]}` container by hand and the "," separator between events if you care about performance.
> You can still use the JSON lib for composing and serializing each individual event.
> This is what we do in clangd.
> 
> This will reduce peak mem usage, but also the repeated allocation/deallocation of similar structures may make allocation cheaper (I know it does a lot with tcmalloc).
> 
> Not sure how much this will help, as always with performance, measuring is a good idea.
Hmm, good point, thanks. I'm to try and measure it. My preliminary thought is that json::Array reserving has already got rid of this time extra cost (not memory peak usage though).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60609/new/

https://reviews.llvm.org/D60609