takuto.ikuta added inline comments.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:28
+
+static std::string escapeString(const char *Src) {
+ std::string OS;
----------------
you can pass StringRef here and remove .data() or .c_str() from caller.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:33
+ switch (C) {
+ case '"':
+ case '\\':
----------------
Include escape for '/' too.
https://tools.ietf.org/html/rfc8259#section-7
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+ default:
+ if (C >= 32 && C < 126) {
+ OS += C;
----------------
I prefer to use isPrint here.
https://github.com/llvm/llvm-project/blob/2946cd701067404b99c39fb29dc9c74bd7193eb3/llvm/include/llvm/ADT/StringExtras.h#L105
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:72
+ Entry E = {steady_clock::now(), {}, Name, Detail};
+ Stack.emplace_back(E);
+ }
----------------
I prefer to write
```
Stack.push_back(std::move(E));
```
or
```
Stack.emplace_back(steady_clock::now(), {}, Name, Detail);
```
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:103
+
+ *OS << "{ \"traceEvents\": [\n";
+
----------------
Don't we want to use json utility for this?
https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/JSON.h
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:147
+ std::unordered_map<std::string, DurationType> TotalPerName;
+ std::unordered_map<std::string, size_t> CountPerName;
+ time_point<steady_clock> StartTime;
----------------
better to have one hash map so that we don't need to call 2 lookup in L92 and
L93.
Also StringMap may be faster than unordered_map.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58675/new/
https://reviews.llvm.org/D58675
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
