[Lldb-commits] [PATCH] D117632: Instrument SBAPI with scoped timers

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D117632#3253445 <https://reviews.llvm.org/D117632#3253445>, @jingham wrote:

> I was about to say this shouldn't matter, since most people don't run with timers enabled and the timers should be no-ops when timing is off.  But LLDB_SCOPED_TIMER seems to do a bunch of work even when the timers are disabled.  The work includes "look up the Category in the map of categories, making a new one if not found", get the current time of day and stash that away, and do whatever the Signpost chooses to do on entry & exit.  So this would be adding non-trivial work for everybody for every SB API call.
>
> Is this really necessary?  Seems like you could have LLDB_SCOPED_TIMER do nothing if the Timer::g_display_depth == 0.  That would mean that you wouldn't get timings for the remaining bit of any timers that would have been active when you enabled the timers, but that doesn't seem particularly wrong to me...

The way that timers currently work is that they are always active and measuring and can always be retrieved via `log timers dump`. `g_display_depth` (and `log timers enable`) only control whether their information gets dumped to stdout.

Apart from that, I don't have any particular opinion on this. I can't say I have found the lldb timers particularly useful. I think we could make then sufficiently cheap so that they could be added to every SB method (and hidden in the LLDB_RECORD_METHOD macro, since we already have it), but if I wanted to do some performance analysis, I'd probably use a profiler.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117632/new/

https://reviews.llvm.org/D117632