[PATCH] D39086: Performance tracing facility for clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/Trace.cpp:87
+
+static Tracer* T = nullptr;
+} // namespace
----------------
Maybe use `unique_ptr<Tracer>` (or even `llvm::Optional<Tracer>`)? Otherwise we leak memory and don't flush the stream if users of tracing API forget to call `stop()`. (I think I'm on the same page with @ioeric here  about forgetting a call to `stop()`).
 
Having a static global of non-pod type is a no-go, but we could use static local variables:

```
llvm::Optional<Tracer>& getTracerInstance() {
 static llvm::Optional<Tracer> Tracer;
 return Tracer;
}
```


================
Comment at: clangd/Trace.cpp:102
+  if (!T) return;
+  // TODO: remove str() once formatv_object is formatv-able.
+  T->event("i", formatv(R"("name":"{0}")", yaml::escape(Message.str())).str());
----------------
NIT: Use `FIXME` instead of `TODO`


https://reviews.llvm.org/D39086