feat(hydroflow)!:Change current_tick_start to wall clock time

[rohitkulshreshtha]

Addresses #1187.

Wall-clock time isn't monotonic, but that's expected to be understood.

[cloudflare-workers-and-pages]

Deploying hydroflow with    Cloudflare Pages

Latest commit:	1a635a7
Status:	✅  Deploy successful!
Preview URL:	https://3c88fce3.hydroflow.pages.dev
Branch Preview URL:	https://rohit-hf-1187.hydroflow.pages.dev

View logs

[rohitkulshreshtha]

The WASM SystemTime implementation through the instant crate does the following for duration_since:

 pub fn duration_since(&self, earlier: SystemTime) -> Result<Duration, ()> {
        let dur_ms = self.0 - earlier.0;
        if dur_ms < 0.0 {
            return Err(());
        }
        Ok(Duration::from_millis(dur_ms as u64))
    }

1. The implementation in instant doesn't seem to be a drop-in replacement for std::time - std::time returns a Duration in Err(Duration) to indicate how far behind the time went.
2. The failing test fails every time, which means the time goes backward each time.
3. The failing test, as written, is flaky and can benefit from a pluggable, deterministic clock.

[MingweiSamuel]

Let's use https://crates.io/crates/web-time instead of instant (which is out of date)

#[cfg(not(target_family = "wasm"))]
pub use std::time::SystemTime;
#[cfg(target_family = "wasm")]
pub use web_time::SystemTime;

[rohitkulshreshtha]

This isn't a choice-of-library issue. My main point is that evaluating that some wall-clock time has elapsed after defer_tick is always going to be flaky. It worked in the past because performance counters were being used.

[MingweiSamuel]

Oh! it seems the code in instant is actually just completely broken: sebcrozet/instant#49

https://github.com/sebcrozet/instant/blob/master/src/wasm.rs#L201-L203

self.duration_since(SystemTime::now())

Is backwards and would always be negative if time goes forward. Should be

Self::now().duration_since(*self)

Should use web-time regardless

[rohitkulshreshtha]

Oh. I didn't know it had the check completely backward. I suspected it's a millisecond-grained timestamp that makes equal timestamp fails (so >= or <= in a check somewhere instead of < or >).

I agree with replacing it. The test will start passing most of the time with a correct implementation. I can then look at pluggable clocks independently.

[MingweiSamuel]

Yeah I knew to prefer web-time but I didn't realize how busted instant was until just now looking thru the issues 😭

[MingweiSamuel]

Nice!

[MingweiSamuel]

Oh actually may want to target flag this, I think it's always different than std, even on regular builds

[rohitkulshreshtha]

I think the library target flags this internally.

[MingweiSamuel]

I guess maybe your original bug cause may also be true?

[rohitkulshreshtha]

Yup. It passes on my laptop. It may succeed if the test is re-run. Disabling auto-merge.