Does it make sense to pin `Upkeep` thread?

[gootorov]

My knowledge regarding timestamp counters is limited. However, the discussions in #17 and #61 left me wondering - does it make sense to pin the Upkeep thread and prevent it from moving to other cores/sockets?

Since Upkeep can move freely, and it calls Clock::now() periodically, is it possible to get a wildly different raw value after rescheduling, thus resulting in less than ideal measurement even after scaling? Would pinning improve accuracy (at least theoretically), or is it not a concern due to some other reasons?

[tobz]

I think, in practice, though, there's not likely to be a meaningful difference. The majority of error when using Clock::recent is going to come from the interval of the upkeep thread itself: if your interval is 1ms, then the error bounds on accuracy are something like -15ns to -1ms, where you'll always be at least a little bit behind of the true time (the latency of the rdtsc call and scaling, plus the atomic operation to set the new global recent time) but you could be up to the full interval behind, where the upkeep thread is just about to refresh the global recent time but hasn't yet.

So for small core-to-core TSC offset errors, they will likely be drowned out by the inherent error bounds of the recent time feature itself. If you have cores with just wildly wrong TSC offsets (which is definitely real, and happened in #61 as you've seen), then you'll end up being backstopped by the monotonic logic that prevents negative durations when comparing two Instant values.

I'm not against pinning the upkeep thread in principle, although it feels like it might introduce a lot of extra code to do so since I don't believe there's a way in stdlib to pin threads in a cross-platform way. If this is something you're interested in trying to work up a PR for, I'll say that I'm willing to review it but I can't promise that I would accept it.

[gootorov]

Thank you for a detailed response! I was afraid that perhaps there's protection from getting negative values, but no protection from getting "too positive" values. That is, getting a Duration much larger than what actually passed.

I've decided to do some experimentation and got really surprising results. The proof of concept implementation is here: #93

I tried the following to determine whether pinning makes any difference:

use quanta::Upkeep;
use std::time::Duration;

fn main() {
    // Takes about 7 seconds for sampling to run on my hardware.
    const SAMPLES: usize = 1_000_000_000;

    let _handle =
        match Upkeep::new(Duration::from_millis(1)).start_pinned(core_affinity::CoreId { id: 0 }) {
            Ok(handle) => handle,
            Err(e) => panic!("{}", e),
        };
    /*
    let _handle = match Upkeep::new(Duration::from_millis(1)).start() {
        Ok(handle) => handle,
        Err(e) => panic!("{}", e),
    };
    */
    let clock = quanta::Clock::new();
    let mut then = clock.recent();

    let mut samples = Vec::with_capacity(SAMPLES);
    for _ in 0..SAMPLES {
        let now = clock.recent();
        let delta = now.duration_since(then);
        then = now;
        samples.push(delta);
    }

    let min = samples
        .iter()
        .filter(|sample| !sample.is_zero())
        .min()
        .unwrap()
        .as_nanos();
    let max = samples.iter().max().unwrap().as_nanos();

    let samples = samples
        .iter()
        .map(Duration::as_nanos)
        .filter(|&sample| sample != 0)
        .map(|sample| sample as f64)
        .collect::<Vec<_>>();
    let samples = ndarray::Array::from_vec(samples);

    println!("var: {}", samples.var(0.));
    println!("std: {}", samples.std(0.));
    println!("min: {}", min);
    println!("max: {}", max);
    println!("samples: {}", samples.len());
}

[package]
name = "quanta-sampling"
version = "0.1.0"
edition = "2021"

[dependencies]
quanta = { git = "https://github.com/gootorov/quanta", rev = "6dcf637933e2cb2f4972c0e53b0411c9fc813606" }
core_affinity = "0.8"
ndarray = "0.15"

And these are the results when running a pinned Upkeep:

var: 26124369446.428097
std: 161630.3481603257
min: 1008208
max: 8951081
samples: 5703

And when Upkeep isn't pinned:

var: 14028570377332153000000000
std: 3745473318197.8687
min: 1008613
max: 284113352378522
samples: 5753

I'm startled by that max spike, that can't possibly be right... I'm consistently getting huge max values in non-pinned version across multiple runs. And similarly, max is never larger than min by a single factor in the pinned version. That could be my faulty hardware, or wrong testing methodology -- I don't think I've seen anything traveling 3 days into the future at work (I'll double check when I can). Also, just as a sanity check, I've tried to run before my changes -- i.e. 0.12 version, and the huge max values are there.

My hardware in question is an Intel i7-7700HQ. Inspecting /proc/cpuinfo shows tsc, rdtscp, constant_tsc, nonstop_tsc, tsc_deadline_timer, tsc_adjust, and tsc_offset flags present.

Also, I don't mind if you don't accept the PR due to additional complexity/dependencies. I've just realized that this functionality can be implemented by using the quanta::set_recent API. At the very least, the PR can always be reopened if this feature gets requested more.

[gootorov]

Ok, just as I've written all of this, I've realized where max comes from. I haven't given enough time for the Upkeep thread to initialize :)
With the following change:

diff --git a/src/main.rs b/src/main.rs
index f8aa6c2..6d4ee12 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -16,6 +16,7 @@ fn main() {
         Ok(handle) => handle,
         Err(e) => panic!("{}", e),
     };
+    std::thread::sleep(Duration::from_secs(3));
 
     let clock = quanta::Clock::new();
     let mut then = clock.recent();

There's hardly any difference anymore between pinned:

var: 327603692.8911815
std: 18099.82576963606
min: 1007167
max: 1137609
samples: 5659

and non-pinned version:

var: 374931814.1682927
std: 19363.15610039574
min: 1008523
max: 1263400
samples: 5609

[tobz]

@gootorov Nice! Thanks for testing all of that out.

I agree with your conclusion: for example, the standard deviation decreasing by ~1us with a 1ms interval configured is indeed a very small change in the overall dispersion.

However, your testing revealed an interesting little ergonomic bug: we should always be taking an initial time measurement from the clock and setting the recent time with it before we start the upkeep thread, which as you've discovered, has some variable timing in terms of how long it takes to start. I opened #94 to track this.