Expose a way to get current timestamp in runtime events

[c-cube]

I have existing tracing libraries that rely on mtime to get timestamps, and runtime events don't really suffice to replace them. I'd still like to use runtime events to collect GC spans/GC events, but my problem is that the timestamps are totally opaque and it's not really possible to correlate them with mtime (since the instant I observe an event is not the timestamp at which the event was created). By exposing the timestamp, I can compute the offset (if any) with mtime and use that as a way to translate Runtime_events.Timestamp.t into Mtime.t.

[gasche]

No strong opinion here -- from a distance it looks reasonable, but then I wonder if there was an explicit decision not to expose this in the past. I wonder if @sadiqj has an opinion?

[sadiqj]

Thanks for the ping @gasche . This looks like a good idea to me. My only question is do we want get_current to return Timestamp.t?

[dra27]

This applied to osdeps.h should fix the Windows build:

diff --git a/runtime/caml/osdeps.h b/runtime/caml/osdeps.h
index 5dd42bbd8b..0f5636481b 100644
--- a/runtime/caml/osdeps.h
+++ b/runtime/caml/osdeps.h
@@ -147,7 +147,7 @@ CAMLextern value caml_win32_get_temp_path(void);
    millisecond). This makes it useful for benchmarking and timeouts, but not
    for telling the time. The units are always nanoseconds, but the achieved
    resolution may be less. The starting point is unspecified. */
-extern uint64_t caml_time_counter(void);
+CAMLextern uint64_t caml_time_counter(void);

 extern void caml_init_os_params(void);

[dra27]

Implementation reviewed (thanks for the changes!) - there's still @sadiqj's question+approval required

Implementation corrected, but approval needed for @sadiqj for the PR overall

[c-cube]

Thanks for the ping @gasche . This looks like a good idea to me. My only question is do we want get_current to return Timestamp.t?

I hadn't even registered that the code returned int64 😅 and not t. I don't suppose we can return t and still have the external be unboxed, can we? Personally I don't care much about keeping the abstraction around t being opaque, but I can change the return type to t anyway if that's what's preferred.

[tmcgilchrist]

@c-cube Could you say more about what properties you expect from this time source? If it's POSIX time then this comment is incorrect, since there will be instances where it pauses or jumps based on my understanding of this POSIX document
https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xbd_chap04.html#tag_21_04_15.

The Erlang docs have a good summary of the properties they include for time. For runtime events I expect you'd want monotonically increasing or strictly monotonically increasing, so you can give a happened-before ordering for events.

[c-cube]

Good point, it's a bit weird to have both the base unit be nanoseconds (as internal comments state), and at the same time a clock that can jump forward or stall.

Currently I use mtime, so I'd assume the guarantees are the same — a monotonic clock. If this gets merged I would default to it when it's present (no reconciliation needed) or else fallback on mtime.

[gasche]

Ha! There can also be differences between monotonic clock, for example you can choose to "count time" where the system is suspended (or not), and some systems offer clocks with varying resolutions.

In general I think that computing the difference between two clocks only once would be okay in practice, but that a robust way to proceed would be to recompute the difference between the two clock sources regularly.

[sadiqj]

I don't suppose we can return t and still have the external be unboxed, can we?

I'm not sure about that either.

Personally I don't care much about keeping the abstraction around t being opaque, but I can change the return type to t anyway if that's what's preferred.

I guess now's a good opportunity to decide on whether to keep t opaque. Are there realistic reasons we might want to change it later on? The original motivation was if something like Int63 made it in to the standard library eventually.

[dra27]

I don't suppose we can return t and still have the external be unboxed, can we?

I'm not sure about that either.

Oops, I missed that the first time round. The unboxed version is already superfluous; either:

• Change the signature in the .mli to unit -> t
• Use the bytecode primitive for both (i.e. delete the unboxed primitive and rename the _byte primitive back)
  or:
• Duplicate the external declaration from the .ml file in the .mli

At the moment, we have an unboxed primitive whose output is immediately boxed by the compiler!

Is there definitely value in being able to access the unboxed value, at least while the timestamps being passed to the callbacks are still boxed?

[c-cube]

Does the compiler always box, even if the int64 is used only locally?!

[lthls]

The compiler always boxes in all situations, but using an [@unboxed] primitive lets the backend see box/unbox pairs and remove them. Since the external declaration is not exported, meaningful unboxing can only occur after inlining, but even the non-flambda compiler should be able to handle this case.

I think the PR is fine and works as intended, but changing the .mli to unit -> t would be nice (I don't think it requires any other changes). Note that in the .ml (or any other context where the alias t = int64 is available) the declaration external foo : unit -> (t [@unboxed]) = "foo" "foo_unboxed" is valid and does what you would expect. You will get an error if it stops working too, and not just the annotation getting dropped silently.

[dra27]

meaningful unboxing can only occur after inlining, but even the non-flambda compiler should be able to handle this case.

Heh, heh - another one I can chalk up to "things I thought I understood in the middleend which @lthls corrected" - I didn't realise the inliner could "see" through an abstract definition to the primitive underneath!

[sadiqj]

Just checking (prior to the triage meeting) is there anything else we're waiting on here before merge?

[gasche]

Nope, let's merge. Thanks!

[sadiqj]

(We may want to amend the Changes)

[c-cube]

Thank you!! 😁