wyfo / pyo3-async

PyO3 bindings to various Python asynchronous frameworks
MIT License
30 stars 1 forks source link

Release the GIL while polling futures/stream #2

Open wyfo opened 1 year ago

wyfo commented 1 year ago

The default behavior of version 0.1 is to release the GIL while polling futures/stream. However, I'm not sure that it is the best thing to do.

In fact, if you have a big future, you may want to spawn it in a task on Rust side, to have it polled more efficiently on Rust side; then you would just wrap a JoinHandle in a coroutine, and releasing the GIL to just check the task status may be counterproductive. In the same way, you may wanna wrap a channel send operation in a coroutine, and again, you should not need to release the GIL to do one atomic check.

That's why I've changed the default behavior in f12cad3b65bd131bf49144018a474d410986beb2, while providing a new type: GilUnbound, which releases the GIL in its PyFuture/PyStream implementation. An extension trait is also provided to just call my_future.unbind_gil().

I may be wrong in my design decision, that's why I let the issue open for now, if anybody wants to discuss about it.

ahirner commented 1 year ago

I think a convenient API would allow releasing the GIL on demand. Say you want a long running and computationally expensive stream, as in video processing. I'd like to py.allow_thread after being woken, perform the computation and check some python object whether and how to continue. Can this be built on top of https://github.com/wyfo/pyo3-async/commit/f12cad3b65bd131bf49144018a474d410986beb2 's API? Maybe instead of some python object, reacting to cancellation is a sufficient model for such generator.

wyfo commented 1 year ago

I think a convenient API would allow releasing the GIL on demand.

That's the goal of UnbindGIL extension trait. (By the way, I think the naming was very wrong from me, so I plan rename it AllowThreads in next version and deprecate the current name.)

Maybe instead of some python object, reacting to cancellation is a sufficient model for such generator.

You can also wrap something like a tokio::sync::oneshot::Sender in a #[pyclass], and await the associated receiver in your future/stream with a tokio::select. You can also put this oneshot in the coroutine throw callback (or use a mpsc for both).

Anyway, GIL handling is not trivial with the coroutine wrapper, because when you coroutine is polled, the GIL is hold, but you cannot reuse the pyo3::Python object in your async code, because it's neither Send nor 'static, but both are required for the boxed Future/Stream. You need to manually implement PyFuture. That's why, because I couldn't use it, I made allow_threads default at the beginning. However, the optimization necessary for async generator handling, where I didn't want to acquire the GIL back, have made me going back on the behavior (and I think it's better now for small future which will be, in my opinion the majority, but I may be wrong). But you can still release the GIL easily.

I'm currently trying to implement a #[pyasync] macro which would give access to the GIL in an async block (with some embedded unsafe behind, unfortunately), but it's not easy. but using Python::with_gil if the GIL is already hold is just thread_local boolean check, so it may be cheap enough to use it anyway.