lwt icon indicating copy to clipboard operation
lwt copied to clipboard

Lwt.awaken: replace wakeup and wakeup_later

Open raphael-proust opened this issue 8 months ago • 6 comments

TODO:

  • documentation
  • tests

raphael-proust avatar Apr 24 '25 14:04 raphael-proust

A bit early for a review I suppose, but the right spelling would be immediately, and you might want to use __FUNCTION__ to retrieve the function's name!

MisterDA avatar Apr 24 '25 14:04 MisterDA

@MisterDA thanks!

raphael-proust avatar Apr 24 '25 14:04 raphael-proust

proposal:

replace the awaken ~ordering function with two distinct functions: awaken_deferred (which resolves the given promise later) and awaken_nested (which resolves the promise now, and comes back to current code afterwards) (or maybe queued/stacked or fifo/lifo but I think these names are too abstract)

make the type of the two functions distinct: awaken_nested returns Lwt.t to indicate context switch / interleaving / etc. (even though there's no pausing/yielding per se, there is some concurrency shenanigans going on)

I'm unsure about the type thing. Any comments suggestions ideas?

raphael-proust avatar Apr 29 '25 09:04 raphael-proust

proposal:

make two distinct functions instead of one with parameter

awaken_deferred: awakens the promise the next time the scheduler kicks in (equivalent to doing Lwt.on_success (Lwt.pause ()) (Lwt.wakeup …) rn) yield_to_awaken: pauses current promise (as per pause) but resolves given promise before jumping to scheduler (equivalent to doing Lwt.wakeup …; Lwt.pause () rn)

The idea being: you can either prioritise the currently executing code and delay the resolving to when the scheduler runs or you can prioritise the resolving promise and you have to actuaslly pause the current promise

This is a more radical change that makes the semantics clearer (one promise executes and then the scheduler kicks in, not both promise) but also backwards incompatible (you can't express wakeup anymore. For this reason I don't think we sould have only those two functions but (a) interesting discussion maybe and (b) possibly as an addition rather than a replacement.

raphael-proust avatar Apr 29 '25 09:04 raphael-proust

Currently unhappy with the interface (the named parameter + constructor is too long). Alternatives:

  • separate functions? (also makes it easier to have Lwt.t in the return type of the yielding/nested/immeditate awaken)
  • choose one default (delayed/deferred) and use ?nested:unit to give a different behaviour
  • have a default function (delayed/deferred) and then have a function with a constructor.

I'm currently leaning towards this last possibility:

val awaken: 'a u -> 'a -> unit (* causes the promise associated to the [u] to be awoken the next time the scheduler kicks in *)

type _ interleaving =
  | Self_scheduler_target : unit interleaving (* same as awaken *)
  | Target_shceduler_self : unit Lwt.t interleaving (* = wakeup + pause *)
  | Target_self_scheduler : unit Lwt.t interleaving (* = wakeup *)
  (* other?? *)
  | Dont_care : unit Lwt.t
val awaken_control : interleaving: 'r interleaving -> 'a u -> 'a -> 'r

This makes the default case very simple and the complicated cases explicit.

raphael-proust avatar May 02 '25 13:05 raphael-proust

An issue with delaying the resolution of the promise is that the double-wakening exception has no place to be raised at. It happens asyncly meaning it should be handled by the async exception handler which I'm not keen to place more responsibility on. Still might be necessary.

raphael-proust avatar May 06 '25 12:05 raphael-proust