lebedev.ri added a comment. Looks good ignoring the json bits.
Re license: > https://reviews.llvm.org/D58675 > This is the first part of time tracing system, I have splitted them cause > this part is mostly written by Aras Pranckevicius except of several minor > fixes concerning formatting. So i can't and won't claim any legal knowledge, but it maybe would be good for him to at least comment here, that he is ok with this? ================ Comment at: llvm/lib/Support/TimeProfiler.cpp:20 #include <string> #include <unordered_map> #include <vector> ---------------- Unused header? ================ Comment at: llvm/lib/Support/TimeProfiler.cpp:37-38 DurationType Duration; std::string Name; std::string Detail; }; ---------------- Ah yes, they are allocated when created. Hmm. ================ Comment at: llvm/lib/Support/TimeProfiler.cpp:47-48 void begin(std::string Name, llvm::function_ref<std::string()> Detail) { - Entry E = {steady_clock::now(), {}, Name, Detail()}; + Entry E = {steady_clock::now(), {}, std::move(Name), Detail()}; Stack.push_back(std::move(E)); } ---------------- Does ``` Stack.emplace_back(steady_clock::now(), {}, std::move(Name), Detail()); ``` not work? (also, the `std::string` returned from `Detail` function invocation is moved?) ================ Comment at: llvm/lib/Support/TimeProfiler.cpp:57 // Only include sections longer than 500us. if (duration_cast<microseconds>(E.Duration).count() > 500) Entries.emplace_back(E); ---------------- I feel like this should be ``` static cl::opt<unsigned> TimeProfileGranularity( "time-profile-granularity", cl::desc("<fixme>"), cl::init(500)); ``` ? ================ Comment at: llvm/lib/Support/TimeProfiler.cpp:65 // itself. if (std::find_if(++Stack.rbegin(), Stack.rend(), [&](const Entry &Val) { return Val.Name == E.Name; ---------------- Yes, good point. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60663/new/ https://reviews.llvm.org/D60663 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits