-
-
Notifications
You must be signed in to change notification settings - Fork 252
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
Re-introduce thread pool for the sync functions (maybe) #22
Comments
Using https://docs.rs/tokio/1.7.1/tokio/task/fn.spawn_blocking.html may be the best way, it'll spawn inside tokios internal threadpool and allow you to await it! |
Also wondering if it's possible, as the routes are being added we store some meta data about them rust side, i.e. if they're async or not so we don't have to waste time during a request spent inside the GIL which I gather bottlenecks down to 1 thread. |
@JackThomson2 , I just did that. You can have a look here: #20 |
Thank you for this(https://docs.rs/tokio/1.7.1/tokio/task/fn.spawn_blocking.html) . 😀 |
I suggest we store that info in the rust side, I guess the less time we spend in the GIL the better from what I gather. So each of the hashtables stores an enum of the py function + info about the function. Then a rough idea of what we could do. Something like
|
@JackThomson2 , we have also have a gitter room in case you want to join (https://gitter.im/robyn_/community#) I have also added the non blocking sync feature in the latest release. You can have a look 😄 |
@sansyrox My concern was around the fact that for an async function now we acquire the GIL 3x and for Sync 2x, this is going to bottleneck the server. This can be reduced by one each by storing the async info rust side rather than in a py dict |
@JackThomson2 , how can you differentiate in rust between sync and async without calling those functions?
For this, we can do one thing by acquiring gil once and allowing threads inside the code block. But from what I understand, those all are essentially the same things. Do correct me if I am wrong. |
@sansyrox When the function is added can't we use the same method? So the router stores |
@JackThomson2 , that makes sense. We would save this gil acquision
on every function call |
@JackThomson2 , thank you for the feedback. 😄 I'll implement it tomorrow. But we are still locking it twice here
Do you have any suggestions for this as well? |
I think it looks like a limitation with the py03_asyncio, it may be worth asking the library owned if there's a way to return the result from the async function without having to enter the gil again |
@JackThomson2 , I did talk a lot with the maintainer(when I was creating robyn initially). I don't think it is possible atm. I'll try asking again.
Also, it is restricted natively at |
Closing this issue as this seems to be complete. @JackThomson2 feel free to reopen if you have anything to add. Thanks again! 😄 |
#20 (comment)
Read discussion above
And see if introducing a threadpool will impact asgi compliance(most probably not) but still check again.
The text was updated successfully, but these errors were encountered: