[PATCH] D60609: Use native llvm JSON library for time profiler output
    Anton Afanasyev via Phabricator via llvm-commits 
    llvm-commits at lists.llvm.org
       
    Mon Apr 15 05:48:24 PDT 2019
    
    
  
anton-afanasyev marked 4 inline comments as done.
anton-afanasyev added inline comments.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:83-86
+    json::Array Events;
 
     // Emit all events for the main flame graph.
     for (const auto &E : Entries) {
----------------
lebedev.ri wrote:
> There is no `reserve()` function in `json::Array`, thus you end up doing the memory dance *every* entry. :/
> (alloc larger, move, delete previous)
> I suspect this accounts for **majority** of the extra cost, although not all of it.
Good point to change in json lib.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:90
+
+      Events.push_back(json::Object{
+          {"pid", 1},
----------------
lebedev.ri wrote:
> `emplace_back()` doesn't work?
It works (for explicitly given cons), I've changed it in a separate review: https://reviews.llvm.org/D60663
Repository:
  rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60609/new/
https://reviews.llvm.org/D60609
    
    
More information about the llvm-commits
mailing list