add the lwt_direct package, for direct-style control flow
#1060
Conversation
Co-authored-by: Raphaël Proust <[email protected]>
|
I added some tests! |
c-cube
force-pushed
the
simon/lwt-direct
branch
2 times, most recently
from
0693b72 to
8586717
Compare
😢 not sure how to make CI happy on OCaml 4 .xx at this rate… edit: worked by doing the test with a manual rule |
There was a problem hiding this comment.
Very nice work.
I'll want to do a light editing pass with some renaming and such to make the code more homogeneous (e.g., in run "resolve" and "promise" should have consistent name). But I'll do that after we discuss the points that need discussing and we resolve some questions.
src/direct/lwt_direct.mli
Outdated
| {[ | ||
| Lwt_direct.run (fun () -> | ||
| let continue = ref true in | ||
| while !continue do | ||
| match Lwt_io.read_line in_channel |> Lwt_direct.await with | ||
| | exception End_of_file -> continue := false | ||
| | line -> | ||
| let uppercase_line = String.uppercase_ascii line in | ||
| Lwt_io.write_line out_channel uppercase_line |> Lwt_direct.await | ||
| done) | ||
| ]} |
There was a problem hiding this comment.
Idea from reading this example: make an openable module in direct and encourage bracketed use:
module Syntax : sig
val run : (unit -> 'a) -> 'a Lwt.t
val await : 'a Lwt.t -> 'a
end
and then
Lwt_direct.Syntax.(run @@ fun () ->
wahatever code here the await is available
)
I don't think this is needed for alpha release. And also it shouldn't be documented here in an introductory explanation of the concepts of lwt-direct. But just puting this out there for the future.
|
|
||
| Each task has its own storage, independent from other tasks or promises. *) | ||
| module Storage : sig | ||
| type 'a key = 'a Lwt.key |
There was a problem hiding this comment.
- Do we want to export the type equality? It makes it possible to use
Lwtfunctions to handle keys. Which at the very least is confusing but also would require to have tests in place. - Do we actually need to expose all of this considering it's all compatible with vanilla lwt storage? We could just say in the doc that all the
Lwt.key-related functions are ok to use inrunblocks., - Should we actually advertise storage? it's deprecated so idk that we should encourage it much
There was a problem hiding this comment.
- I think it's good to be able to use existing keys
- it's deprecated but why? Local storage is important!! For logging and tracing at least. Eio has it too :)
There was a problem hiding this comment.
I think it's deprecated because the way that the storage flows is non-obvious and not extensible.
Non-obvious:
Like even experienced Lwt users might have a hard time answering questions like:
In Lwt.on_success (fun v -> f v)) is f executed with the storage "of v" or the local ambient one?
What even is the storage "of v"? It's kinda ok to say if you have let v = <long series of bind>, but what if you have let v, r = Lwt.task ()? Does it get the storage from declaration site or does it gets it from resolution site?
What if you pass r around to several "threads" with different storages so that they race to resolve v?
It's the fundamental original issue of Lwt: there are no threads but the API sometimes say there is.
Non-extensible:
When you define something à la Lwt_list but for whatever data-structure you have in-house. Say Seqes or say lache. You'd want to be able to get storage and reset it so that the abstractions you build on top of your data-structure matches the behaviour of Lwt.
(E.g., in lache you would want to decide what happens if someone binds on a cached promise: do they get the storage from the "thread" which added the promise in the first place? do they get a clean-slate storage? you can't make that happen.
There was a problem hiding this comment.
I guess actually you adding these storage function is because of the the lack of extensibility. So maybe the way out of it, the way to undeprecate is to
- document clearly the built-in flow
- provide the functions needed for extensibility
- provide a few example and some documentation of how to properly do extensibility
There was a problem hiding this comment.
I need to write more about this, but in a way this storage makes more sense for Lwt_direct, where the scope should be clear (one call to run = one instance of storage). Even then there are questions about whether run inside run should inherit the storage or not? hmm
There was a problem hiding this comment.
yes I think the stoarge makes more sense for direct-style, because of the run wrapper gives it a more explicit scope. let's keep it for now and we'll see later
|
merged the branch into i want to try and keep |
This is a WIP, it needs testing, but the fundamentals are here.