-
Notifications
You must be signed in to change notification settings - Fork 200
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
exec_batch does not release GVL. #287
Comments
We would have to unlock the GVL before calling For the above reasons, (high risk, low reward) I'm not particularly motivated to fix this. So if someone wants to try tackling it, that would be great. Otherwise I'm going to wait for user demand. |
I think what you proposed is the correct approach as locking the VM is definitely the wrong approach. The overhead of the per-row lock is to be worn by the user of the interface and is a natural consequence of the GVL. Sqlite3 should not be used on multiple threads IMHO, it should be a separate connection/instance per thread. Does that work? Otherwise, a per-connection mutex would make sense to me. What happens if the row callback tries to execute another query? |
I don't understand? My proposal is to do nothing unless there is demand for the feature by people that use it. |
I'm also open to deprecating and deleting the method too. But AFAIK it's not hurting any users the way it's implemented today |
This implementation is the correct one. However, I agree it's inefficient. If sqlite3 is not thread safe, you should protect it with a mutex, which would also prevent using the same connection for a 2nd query while enumerating the results of a different query. |
Do any of the methods in this code base release the GVL? |
Greetings! I'm working on an project at https://github.com/fly-apps/railsite that would make Sqlite a real option for smaller production Rails deployments, which would ideally work with a threaded Puma server. Since it's been a while, here's what the results look like in
This confirms this is still an issue with the latest versions of Ruby and the Gem. My question is whether or not the flag at Lines 13 to 14 in beaa142
|
AFAICT this is not only an issue with Now that "SQLite in production" is trending, this is going to pose obvious performance issues for anyone using thread / fiber based servers / job processors like puma / falcon / sidekiq etc for their apps. The extralite gem is an alternative SQLite client which releases the GVL during blocking, see note on concurrency here: https://github.com/digital-fabric/extralite?tab=readme-ov-file#concurrency. It is both significantly faster than this gem in general and doesn't have concurrency issues. Maybe that could be a source of inspiration for how to solve this? (I'd love to help out myself, but alas I have no experience programming in C) I'm currently messing with a POC for allowing |
I took a quick look and unfortunately, no, that does not look like an implementation to look up to. It seems like by default they release the GVL for 1 in 1000 rows returned which is essentially just an overly complex way to call |
Gives
Expected:
Or something to that effect.
cc @ko1 @tenderlove @jeremyevans
The text was updated successfully, but these errors were encountered: