Swapping in clocks calibrated off thread about clocksource

[brayniac]

@jonathanstrong - apologies for not having a chance to respond sooner. Travel and moving have prevented me from having time to really sit and address this. Thanks for opening this issue and providing the data around the approach you've taken.

I would be curious to see if there is any substantial gain to be found on virtualized environments like AWS where the clock_gettime() calls tend to be particularly expensive. On baremetal, I would expect clocksource w/ rdtsc to be of less benefit overall.

I still feel that having the ability to re-calibrate in a non-blocking way would be of value - but I do have to wonder if there's any other approach we could take for this. Perhaps we could periodically make a call to both the reference and local clocks and use those to determine the new constants for conversion to reference timescale. This may require some revision to the current strategy of cloning the clocksource around in multi-threaded applications. We still wouldn't want different instances of clocksource within the app to provide differing results.

I feel this is going to require some more thought and experimentation. But do want to let you know that I appreciate what you've done in this experiment. If we find that your approach does have some strong benefit in the virtualized use-case it would be worth considering adding to the library. I think we can do better than what the library currently provides, but it may also be worth further consideration of different approaches to this. I'll try to carve some time to implement another approach to this and update with further thoughts when I have them.

Again, apologies for my slow response and thanks for raising this.

[jonathanstrong]

No worries about the delay! Totally understand. Thanks for your detailed thoughts on it - looking forward to your conclusions. Good call on virtualized environments, I'll try to test it out on some cloud servers I'm running in the next few days.

A couple additional thoughts:

• Any event loop like this is going to be waiting on a channel or queue of some kind for new data. If the calibrated clock came over that main channel, like as part of an enum of possible messages, the marginal cost might be reduced because it's one less check on a concurrent queue. Possibly this could slow down the main channel though, for instance by requiring it to be a larger sized object. I'm not sure that approach would allow an ergonomic handle like the one I prototyped, either.

• I realized that the structs holding the calibration thread need to store it as an Option<Arc<JoinHandle<()>>> rather than an Option<JoinHandle<()>>. The current design relies on the initial object lasting longer than any of its clones without any compile-time check this is the case. An example of the improved pattern would be (some pseudocode included):

struct A<T> {
    tx: Sender<Option<T>>,
    thread: Option<Arc<JoinHandle<()>>>
}

impl<T> A<T> {
    pub fn send(&self, t: T) -> Result<_, _> {
        self.tx.send(Some(t))
    }
}

impl<T> Drop for A<T> {
    fn drop(&mut self) {
        if let Some(arc) = self.thread.take() {
            if let Ok(thread) = Arc::try_unwrap(arc) {
                let _ = self.tx.send(None);
                let _ = thread.join();
            }
        }
    }
}