[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps
Anton Afanasyev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 27 00:43:33 PDT 2019
anton-afanasyev marked 3 inline comments as done.
anton-afanasyev added inline comments.
================
Comment at: clang/lib/Parse/ParseAST.cpp:154
if (HaveLexer) {
+ llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
P.Initialize();
----------------
takuto.ikuta wrote:
> Remove StringRef?
I use `StringRef("")` to explicitly cast to one of overloaded `TimeScope(StringRef, StringRef)` and `TimeScope(StringRef, fuction_ref(std::string()))`.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:103
+
+ *OS << "{ \"traceEvents\": [\n";
+
----------------
takuto.ikuta wrote:
> Don't we want to use json utility for this?
> https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/JSON.h
>
This implementation looks compact and fast. I've read proposal for this library https://docs.google.com/document/d/1OEF9IauWwNuSigZzvvbjc1cVS1uGHRyGTXaoy3DjqM4 , it's recent, so I'm not sure it's stable and speed optimized. Do you actually can advise it? I can do it in the next refactor commit as well.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+ default:
+ if (isPrint(C)) {
+ OS += C;
----------------
takuto.ikuta wrote:
> include StringExtras.h for this?
Should one do it? It's already implicitly included.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58675/new/
https://reviews.llvm.org/D58675
More information about the llvm-commits
mailing list