eio
eio copied to clipboard
Add monotonic clock support
This PR adds support for monotonic clock to eio. The original clock method in Eio.Stdevn.t is now renamed to real_clock since it is in fact an encapsulation of real-time OS clock. The monotonic clock is mono_clock.
The real_clock is implemented using Ptime.t and mono_clock is implemented using Mtime.t. There is an open PR to use CLOCK_BOOTTIME as monotonic clock in Mtime (https://github.com/dbuenzli/mtime/pull/44).
We now use a common Sleep_until effect in Eio_unix.Private for both luv and uring backend.
make bench now uses mono_clock to mark start/end time. I believe this is "more" correct than using real_clock for benchmarking purposes.
In addition to Eio.Time.clock being polymorhphic, it now also features two functions to make working with time in seconds a bit convenient, to_seconds and add_seconds.
Seconds is chosen as default time unit since it seems to be the unit used by OCaml stdlib. Secondly and subjectively it seems to be a convenient unit to be thinking in terms of time when using time related functions in eio.
Fixes https://github.com/ocaml-multicore/eio/issues/229
/cc @haesbaert
NOTE: This is a non backward compatible change. Specifically, env#clock is no longer there. Additionally, the Eio.Time.clock now also requires 'a generic parameter. Any advice on how to go about maintaining backwards compatibility would be greatly appreciated.
@talex5
NOTE: This is a non backward compatible change. Specifically, env#clock is no longer there. Additionally, the Eio.Time.clock now also requires 'a generic parameter. Any advice on how to go about maintaining backwards compatibility would be greatly appreciated.
You can keep Stdenv.clock, but mark it deprecated (but it doesn't look like it's possible to get a compile-time warning about env#clock).
As we're not at Eio 1.0 yet, it's OK to break the API if it's difficult to avoid it, like here.
However, if you did want to avoid that it would be possible to add a new Clock module with the new API e.g.
module Clock : sig
type 'a t = ...
...
end
module Time : sig
type clock = float Clock.t
...
(* deprecated functions here *)
end
I have rebased on master now.
As we're not at Eio 1.0 yet, it's OK to break the API if it's difficult to avoid it, like here.
Thanks. Yes, I would like to keep the current API set since every other option introduces un-necessary cruft.
Could you please approve/merge if there are no further objections.
@talex5 the PR has been rebased to the latest master.
Now #338 is merged, the remaining parts of this PR (e.g. the use of Ptime) need rebasing on top of that.
@talex5 It seems this PR has bit rotted quite a bit. Do we still want to use the actual clock to implement timeout functionality, i.e. if mono then we use mono clock, if sys then we use real/sys clock and so forth. If so, I will open another PR rather than continue with this one. Also I believe https://github.com/ocaml-multicore/ocaml-uring/pull/79 in ocaml-uring is required to implement such functionality at least in uring backend.
Updating the benchmarks to use monotonic times sounds good.
Getting eio_linux to use the realtime clock directly would be good too, fixing this comment:
https://github.com/ocaml-multicore/eio/blob/32c26ab3133b5e51a981533cb4bb201d0eb35148/lib_eio_linux/eio_linux.ml#L1300-L1304
I don't see why that needs https://github.com/ocaml-multicore/ocaml-uring/pull/79. We can just use the new Uring.timeout with Realtime, I think.
Closing this PR and opening new ones is fine with me.
I don't see why that needs https://github.com/ocaml-multicore/ocaml-uring/pull/79. We can just use the new Uring.timeout with Realtime, I think.
My memory is a bit hazy at the moment regarding this. However, the last time I tried this PR, I believe I realized that eventually, timeout value drills down to this bit of code as well. https://github.com/ocaml-multicore/eio/blob/670e656560b1464cdef6e48481b0fd6aa240fbfd/lib_eio_linux/eio_linux.ml#L581
IIRC the clock defaults to CLOCK_MONOTONIC, in order to change/improve on that I think I opened that uring PR.
You don't need to (and can't) use Uring.wait's timeout for real-time timeouts. You can just submit them as their own separate operations instead. I don't see what completion counts have to do with this either.
- Submit a Realtime timeout to the ring, just like any other operation.
- When it's done,
Uring.waitwill return and it will be processed like any other operation. - If you need to cancel, send a cancel message, like cancelling other uring operations.
Closing this since it has bit rotted quite a bit now.