[PATCH] D43272: [clangd] Fix tracing now that spans lifetimes can overlap on a thread.
Ilya Biryukov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 14 05:28:53 PST 2018
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LG with a few NITs.
================
Comment at: clangd/Trace.cpp:133
+ std::atomic<double> EndTime; // Filled in by endSpan().
+ std::string Name;
+ const uint64_t TID;
----------------
Maybe make all fields except `EndTime` const?
================
Comment at: clangd/Trace.h:46
+ // The Context returned by beginSpan is active, but Args is not ready.
+ virtual void endSpan() {};
----------------
It feels awkward that `endSpan()` does not have any parameters explicitly connecting it to `beginSpan()`.
We could change `beginSpan()` to return a callback that would be called when `Span` ends instead, that would make the connection clear, albeit it's not very useful for `JSONTracer`.
Not sure we should change it, just thinking out loud :-)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43272
More information about the cfe-commits
mailing list