Skip to content
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

Deprecate the Lwt_unix.async_method machinery and eventually remove it #572

Closed
aantron opened this issue Apr 10, 2018 · 9 comments
Closed
Milestone

Comments

@aantron
Copy link
Collaborator

aantron commented Apr 10, 2018

Here is a description of it:

(** For system calls that cannot be made asynchronously, Lwt uses one
of the following method: *)
type async_method =
| Async_none
(** System calls are made synchronously, and may block the
entire program. *)
| Async_detach
(** System calls are made in another system thread, thus without
blocking other Lwt threads. The drawback is that it may
degrade performance in some cases.
This is the default. *)
| Async_switch
(** System calls are made in the main thread, and if one blocks
the execution continue in another system thread. This method
is the most efficient, also you will get better performance
if you force all threads to run on the same cpu. On linux
this can be done by using the command [taskset].
Note that this method is still experimental. *)

Of the three methods, Async_switch, AFAICT, was only ever working on Linux, but might be broken now (#184). Since it is non-portable, marked as experimental, probably broken, and unsupported, I think we should deprecate it and eventually remove it.

Of the remaining two methods, Async_none and Async_detach, Async_detach is the reason why someone would want to use Lwt at all, over Unix. In case we bind a system call that is not available in blocking form in Unix, we can expose both synchronous and asynchronous bindings from Lwt. So, we should also remove Async_none, and thus the whole async_method type.

IIRC, setting the async method explicitly is basically not used in opam. I would have to look again to see what the exact impact is.

@chambart
Copy link
Contributor

chambart commented Aug 30, 2019

I'm all for removing Async_switch. It was a neat trick, but is a too brittle to seriously consider keeping. But Async_none has its uses. There are not that many system call that are completely blocking for an unbounded time. Most of the things that doesn't have a usable non-blocking equivalent are disk IO, which are often fast enough not to care. In particular, if the disk cache has a good hit-rate, Async_none is faster than Async_detach. It matters when you are CPU bound rather than IO bound.
But the main reason for me to use it is that it is often far easier to debug with GDB with Async_none.

By the way the only way I use it is through the environment variable.

@aantron
Copy link
Collaborator Author

aantron commented Aug 30, 2019

Thanks. I'll see about keeping Async_none somehow, maybe through a simpler API, or just the environment variable.

@chambart
Copy link
Contributor

chambart commented Sep 2, 2019

Thanks, I would be happy with only the environment variable

@aantron
Copy link
Collaborator Author

aantron commented Sep 8, 2019

I looked in more detail, and I am now leaning towards making Async_switch be effectively a synonym for Async_detach in 5.0.0, and leaving Async_none working as now. I'll experiment with Async_none some more, and perhaps un-deprecate it. Once the code implementing Async_switch is gone, it should also be much clearer what to do next (probably nothing :)).

@adrianbrink
Copy link

What is the current status of Async_none?

@aantron
Copy link
Collaborator Author

aantron commented Oct 20, 2019

It's unchanged from before this issue was opened.

@adrianbrink
Copy link

Does that mean that I can still use Lwt_unix.async_method Async_none even in 5.0.0?

@aantron
Copy link
Collaborator Author

aantron commented Oct 21, 2019

That's what I'm leaning towards, as per the earlier comment. Can you say how or for what purpose you're using Async_none? Having this information will help if anyone ever considers removing it again later.

@aantron
Copy link
Collaborator Author

aantron commented Dec 15, 2019

The attached commit makes Async_switch a synonym for Async_detach, and leaves Async_none working as before. It will be released in Lwt 5.0.0 in the coming days.

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

No branches or pull requests

3 participants