Add support for scoped timings. #4533
Conversation
|
|
||
| private: | ||
| std::optional<Timings>* timings; | ||
| llvm::StringRef label; |
There was a problem hiding this comment.
WDYT about using a std::string here and moving it into the Add call (with the old API)?
Mostly it'd make it safe to use this with a string built with any formatting, and it shouldn't change how many strings we end up needing.
There was a problem hiding this comment.
That would allocate the string regardless of whether timings were on. Is that desirable?
There was a problem hiding this comment.
Don't get me wrong, there could be a similar: label(*timings ? std::string(label) : std::string())
It'd just been my leaning to keep the size smaller for the common case where timings weren't collected.
There was a problem hiding this comment.
Note, might want a slightly different structure if this is used with formatting, since we'd probably want to avoid creation of the formatted string when not storing timings.
I think there are a few related ways to do this. I considered llvm::make_scope_exit, but the return type is difficult to work with. I particularly was thinking I could encapsulate the duration logic this way.