From 53599f011cff367ffba954f13105cfd1ed38f2bc Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Sun, 15 Sep 2019 10:49:01 +1000 Subject: [PATCH] Avoid expensive tracking objects for prepared statements Per #36949 we introduce a race condition fix for #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. --- .../connection_adapters/abstract_adapter.rb | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 32667cbc6201b..cacff6d104ae9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -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: @@ -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( @@ -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 @@ -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