Proof of Concept: Release GVL in Statement#step

[jhawthorn]

Refs #287, but I don't think this is ready to merge. As described below the performance impact is very significant. I think this is something the library should do, but it's a penalty that's hard to just eat.

This allows other threads to execute Ruby code (including performing more work sqlite operations if on another DB connection). This should significantly improve multithreaded performance, however as @tenderlove correctly predicted in #287 adds significant overhead to single-threaded performance.

This required wrapping all callbacks which can happen within statement execution with rb_thread_call_with_gvl.

I've written two benchmarks, one which shows why this could be a good idea and one which shows why it's a bad idea. It's unfortunately not clear cut.

The good

https://gist.github.com/jhawthorn/ae84ee2723ff4930344778cfe40205a2

I made a benchmark based on making sqlite actually do work (a poorly optimized query) before returning, and then having multiple threads perform the same operation. This shows a very significant speed up as we are actually able to run the sqlite operations in parallel.

main

$ bundle exec ruby test.rb
single threaded:
0.9432485040742904
multi threaded:
1.1921860249713063

This branch:

$ bundle exec ruby test.rb
single threaded:
0.927121008047834
multi threaded:
0.17125224182382226

The bad

https://gist.github.com/jhawthorn/492b6a9c5fb6bebe1ebfe985f52495de

When running a completely trivial query in a single thread, this is about 60% slower due to the need to release and re-acquire the GVL on every call to Statement#step. I don't think there's an easy way around this as even if our API was a more "bulk" operation (ie "return all rows" rather than "return one row") I believe we'll need the GVL every time we want to allocate a Ruby string and our memory buffer returned from SQLite is only good until our next call to sqlite3_step.

Comparison:
SELECT * LIMIT 1000 (gvl):     4336.3 i/s
SELECT * LIMIT 1000 (nogvl):     2705.2 i/s - 1.60x  slower

I do think this is capturing the absolute worst case as the query is completely trivial and doesn't even make allocations other than the one array per-row.

So that's where we're at. There might be some sort of compromise we could do like making the GVL releasing optional or based on a heuristic. I haven't yet investigated any way to make this faster, but it does seem unlikely we'll find a way to do this without a performance penalty.

TODO

• execute_batch2 doesn't yet release the GVL (which probably makes it crashy if any callbacks exist)
• prepare and some other operations should likely also release the GVL
• It might be good to provide a UBF to rb_thread_call_without_gvl

[flavorjones]

Do you think it might be useful to allow users to make this decision, e.g. via a Database constructor option like Database.new(release_gvl: true)?

Allowing folks who are knowledgeable about their production runtime to decide whether to release the GVL or not might still be a huge step forward, and allow higher-level abstractions (like the Rails database adapter) to evolve their own policies.

[jhawthorn]

A configuration option is probably the safest bet, at least for step. Though I'd love if the option was kept open to make release_gvl: true the default in the future if we are either able to reduce the overhead or find that in real-world scenarios it's the best option.

I'm not sure immediately why this is failing CI on the Windows --disable-system-libraries builds specifically. I'll look into that when I get the chance.

[byroot]

Something we mentioned in the past was to only release the GVL around the first call to step. From my understanding, that's the one that perform the query hence is significantly "slower". Supposedly the following calls are just fetching the data and don't allow for much concurrency anyway.

[jhawthorn]

Something we mentioned in the past was to only release the GVL around the first call to step. From my understanding, that's the one that perform the query hence is significantly "slower". Supposedly the following calls are just fetching the data and don't allow for much concurrency anyway.

In the general case this isn't true, unfortunately. Building on the benchmark in my gist:

db.prepare("SELECT * FROM t2 WHERE b>=2 AND b<500") do |stmt|
  10.times do
    puts Benchmark.realtime { stmt.step } * 1000
  end
end

0.05734805017709732
0.017974060028791428
0.012603937648236752
0.25284592993557453
0.04639802500605583
0.07501093205064535
0.008465955033898354
0.1521760132163763
0.01405598595738411
0.0023850006982684135

stmt.step just allows sqlite's VM to continue running, so it depends whether it has work to do between the first row returned and the second (ie. continue scanning the table).

It's possible that releasing once is a good heuristic though for queries which have an index?