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

Implement GVL-releasing, backing-off busy_timeout #475

Closed
wants to merge 1 commit into from

Conversation

fractaledmind
Copy link
Contributor

@fractaledmind fractaledmind commented Jan 11, 2024

After getting a tad more experienced with the C portion of this codebase, I decided to try and see if I could implement the logic of SQLite's busy_timeout (with its backoff logic) in the extension such that we could release the GVL while sleeping. You can find SQLite's implementation of sqlite3_busy_timeout here and the default callback functionality here.

I think this is a good start, but I am stumped because when I try to compile, I get this error:

error: implicit declaration of function 'rb_thread_call_without_gvl' is invalid in C99

I'm not certain why this method isn't available, but how can I sleep and release the GVL here in C-land? Any help on this would be greatly appreciated. (note: I see from CI that this method is only missing in darwin environments. Interesting)

cc @suwyn who originally challenged me to do this in C-land.

@fractaledmind
Copy link
Contributor Author

@byroot: I know that at this point you are spending a meaningful portion of your working day (and maybe outside of work day) helping me learn C in the context of this library. I don't want to be a burden or take advantage of your generosity, so feel free to ignore this until you are totally free. However, if at some point you could help me understand how to release the GVL while sleeping, I would be greatly appreciative.

Also, do you think we gain notable performance with such a C implementation of a timeout as opposed to the Ruby implementation recently merged?

@byroot
Copy link
Contributor

byroot commented Jan 12, 2024

when I try to compile, I get this error:

$ rg -F rb_thread_call_without_gvl ~/.rbenv/versions/3.3.0/include/ruby-3.3.0/ruby
/Users/byroot/.rbenv/versions/3.3.0/include/ruby-3.3.0/ruby/thread.h
89: *                 of a rb_thread_call_without_gvl()'s callback.
109: * Unlike rb_thread_call_without_gvl2()  this function  also reacts  to signals
130:void *rb_thread_call_without_gvl(void *(*func)(void *), void *data1,

rb_thread_call_without_gvl is defined in ruby/thread.h which isn't included by ruby.h, so you need # include "ruby.h".

@byroot
Copy link
Contributor

byroot commented Jan 12, 2024

Also, do you think we gain notable performance with such a C implementation of a timeout as opposed to the Ruby implementation recently merged?

I don't know for sure, but since we sleep for at least 1ms, I don't think it makes a huge difference.

What could make a difference would be if the GVL was released when the query is performed, then this PR would make sense as it would allow to handle the busy timeout without having to acquire the GVL back (which is problematic). But in the current state I don't think it's worth it.

@fractaledmind
Copy link
Contributor Author

Understood. Will close for now then.

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

Successfully merging this pull request may close these issues.

2 participants