Skip to content

Commit

Permalink
Avoid expensive tracking objects for prepared statements
Browse files Browse the repository at this point in the history
Per rails#36949 we introduce a race condition fix for rails#36763

This refines the fix to avoid using Concurrent::ThreadLocalVar

The implementation in the concurrent lib is rather expensive, culminating in
a finalizer per object that spins off a thread to do cleanup work.

None of this expense is needed as we can simply implement
the desired behavior using Ruby primitives. Additionally this moves to a Fiber bound implementation vs a thread bound implementation, something that is not desired for this particular usage.
  • Loading branch information
SamSaffron committed Sep 15, 2019
1 parent fb6d5e0 commit 53599f0
Showing 1 changed file with 14 additions and 5 deletions.
Expand Up @@ -10,7 +10,6 @@
require "arel/collectors/composite"
require "arel/collectors/sql_string"
require "arel/collectors/substitute_binds"
require "concurrent/atomic/thread_local_var"

module ActiveRecord
module ConnectionAdapters # :nodoc:
Expand Down Expand Up @@ -92,10 +91,10 @@ def initialize(connection, logger = nil, config = {}) # :nodoc:
@lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new

if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true })
@prepared_statement_status = Concurrent::ThreadLocalVar.new(true)
@prepared_statements_default = true
@visitor.extend(DetermineIfPreparableVisitor)
else
@prepared_statement_status = Concurrent::ThreadLocalVar.new(false)
@prepared_statements_default = false
end

@advisory_locks_enabled = self.class.type_cast_config_to_boolean(
Expand Down Expand Up @@ -139,7 +138,10 @@ def schema_migration # :nodoc:
end

def prepared_statements
@prepared_statement_status.value
if @prepared_statements_default
hash = Thread.current[:ar_prepared_statements]
!(hash && hash[self.object_id] == false)
end
end

class Version
Expand Down Expand Up @@ -226,7 +228,14 @@ def seconds_idle # :nodoc:
end

def unprepared_statement
@prepared_statement_status.bind(false) { yield }
if @prepared_statements_default
hash = Thread.current[:ar_prepared_statements] ||= {}
hash[self.object_id] = false
end

yield
ensure
hash.delete(self.object_id) if hash
end

# Returns the human-readable name of the adapter. Use mixed case - one
Expand Down

0 comments on commit 53599f0

Please sign in to comment.