Skip to content

Lwt.awaken: replace wakeup and wakeup_later #1055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

raphael-proust
Copy link
Collaborator

TODO:

  • documentation
  • tests

TODO:
- documentation
- tests
@MisterDA
Copy link
Contributor

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!

@raphael-proust
Copy link
Collaborator Author

@MisterDA thanks!

@raphael-proust
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants