-
Notifications
You must be signed in to change notification settings - Fork 82
Description
Opening an issue rather than a PR given I already have a bunch of open PRs so I'd rather gauge appetite for the change before I spend time on it.
Context
I'd like to improve IO time reporting in Rails, and trilogy has a feature I think could help a lot, it is that its Result object has a query_time attribute.
The problem is that this time is recorded with the GVL acquired, so in case of GVL contention, it's widely innacurate:
require "bundler/inline"
gemfile do
gem "bigdecimal" # for trilogy
gem "trilogy"
end
trilogy = Trilogy.new
result = trilogy.query("SELECT 1")
puts "query time: #{result.query_time}"
def fibonacci( n )
return n if ( 0..1 ).include? n
( fibonacci( n - 1 ) + fibonacci( n - 2 ) )
end
threads = 10.times.map do
Thread.new do
loop do
fibonacci(25)
end
end
end
trilogy = Trilogy.new
result = trilogy.query("SELECT 1")
puts "query time: #{result.query_time}"query time: 0.000135
query time: 1.100369
Problem
Right now trilogy only release the GVL from the cb_wait callback. This is nice because it's simple, but It makes it very hard to properly time how long the IO actually took.
It also means we're not releasing the GVL for as long as we could, and that in some case we might need to release and re-acquire it multiple times for a single query.
Idea
How would you feel about a refactor that stop relying solely on the _cb_wait callback, but instead explicitly release the GVL around higher level function?
cc @jhawthorn @matthewd @composerinteralia @adrianna-chang-shopify