programatik29 / tokio-rusqlite

Asynchronous handle for rusqlite library.
MIT License
84 stars 20 forks source link

Use `tokio::task::spawn_blocking` instead of `thread::spawn` #9

Closed czocher closed 1 year ago

czocher commented 1 year ago

Hello @programatik29, While creating the async implementation for rusqlite_migration we noticed that this project is using thread::spawn instead of the recommended tokio::task::spawn_blocking . Can I create a PR to change that or is there some underlying reason for this approach?

programatik29 commented 1 year ago

Hello,

This library uses thread::spawn intentionally. The reason for not using tokio::task::spawn_blocking is background thread being possibly long running. Using spawn_blocking would possibly take a thread from the pool for too long without returning and this can cause problems in theory.

For more information about this topic, you can take a look at this article.

That said, one case for spawn_blocking can be rapidly opening and closing rusqlite connections in that case it can offer performance benefits but I think that is extremely unusual and out of scope for this crate. For long running tasks, thread::spawn is the way to go.

czocher commented 1 year ago

Thanks @programatik29, that confirms my suspicions :D.

programatik29 commented 1 year ago

You are talking about tokio::spawn there, if you block those it is really bad. spawn_blocking has a different much larger thread pool. But it is still a thread pool and limited.

czocher commented 1 year ago

Yes I realized after @matze gave me their input, that's why I decided to ask you directly :). Thank you for the article, it made everything clear.

matze commented 1 year ago

Ah, read that article a while ago but surely forgot about that nice table at the end.