[PATCH 2 of 6 v2] util: add an elapsed time wrapper

[Bryan O'Sullivan]

On Fri, Feb 3, 2017 at 2:26 PM, Simon Farnsworth <simonfar at fb.com> wrote:

I'd assumed that with Mercurial encouraging extension authors to wrap
> functions, the overhead wasn't significant.


Well, those functions are usually called just a tiny handful of times, so
that tends to be true in those cases.


> It looks to me like the conditional actually adds significant pain in its
> own right, almost as much as the complex class based wrapper does.
>

Yes.


> Assuming the community doesn't object strongly, I'll redo to be
> unconditionally measuring time (and only conditionally logging it), as the
> overhead of conditionality seems excessive for this purpose.


If you'd also use the same approach as I do of direct measurement in the
places that actually do the I/O, instead of indirection, that would be
great. In fact, I think you should take over my patches and adapt them to
your needs. The added detail in my patches of measuring when Mercurial
stops doing work, instead of when it exits, is going to be important to
your purposes, because just counting ui I/O time won't capture that.

I definitely want the reporting to be conditional - if you're not going to
> use the time, we shouldn't output it.


Clearly.

I also think there's no value to tracking time on stdin, stdout, and stderr
separately, which is why I didn't do that – but that there *is* value in
measuring invocations of subprocesses.


> From my point of view, the goals are:
>

>  1. Establish groups of "not Mercurial's fault" slowness - stdio, extdiff,
> editor invoked for histedit etc. This combines with FB infra to let us
> focus on cases where Mercurial code is the bottleneck.
>
>  2. Do so in such a way that users not on FB infra aren't affected unless
> they choose to be, even if they're using things like logtoprocess today.
>

Yep. If you do the measurement directly, it's cheap enough that you don't
need to worry about it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170203/bc766b96/attachment.html>