is_empty check before retrieving elapsed time for activations

[ncblakely]

While profiling our application, I noticed that we were spending a lot of time in std::time::Instant::elapsed. Despite not having any timer-based activations, it looks like this function was being invoked quite a bit and checking the elapsed time unnecessarily on every call. Just a quick fix to make sure the queue has items first.

[frankmcsherry]

This seems totally reasonable! I think it's maybe a bit surprising if this is hot, though, as it suggests that you may be in a busy-loop doing not too much? (I think it's about 50ns each call, for me, and most operators should be much more than that). Mostly thinking out loud that if you are in a busy loop something will be on the stack, always.

Or, am I totally wrong and you are doing lots of work, and this is getting in the way? (( just trying to grok what's going on; we should totally merge the PR! ))

[ncblakely]

Hmm, could certainly be misuse on our end somewhere. :)

Here's the trace from VTune, if it helps:

I did see some improvement from fixing it locally, though less than expected considering it has the highest CPU time (ignoring the top two, which are just background threads waiting). That may indeed suggest we have some other problem that this was merely a symptom of (possibly stalled in a busy loop waiting for data as you mentioned).

I ran another trace after the fix, and std::time::Instant was completely gone from the top contributors.

Thanks for merging the PR!

[frankmcsherry]

Thanks for the explanation!

[frankmcsherry]

Btw, if you end up finding that it is busy waiting, and you didn't intend that, drop in and let us know. We've had at least one case at Materialize where the current scheduler's API means you can overload the scheduler with busy work, because it doesn't coalesce reactivations. There's a draft PR to fix it, but it's been lingering due to not having the repro / motivation (MZ worked around it a different way).