From 46cc85d5343844a812a570c2a0254f5ceda6b8b6 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 | 17 ++++++++++++----- 1 file changed, 12 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 19c7f3d1effd0..bdd746a96ce6d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "set" require "active_record/connection_adapters/determine_if_preparable_visitor" require "active_record/connection_adapters/schema_cache" require "active_record/connection_adapters/sql_type_metadata" @@ -11,7 +12,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: @@ -130,10 +130,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 = true @visitor.extend(DetermineIfPreparableVisitor) else - @prepared_statement_status = Concurrent::ThreadLocalVar.new(false) + @prepared_statements = false end @advisory_locks_enabled = self.class.type_cast_config_to_boolean( @@ -177,7 +177,11 @@ def schema_migration # :nodoc: end def prepared_statements - @prepared_statement_status.value + @prepared_statements && !prepared_statements_disabled_cache.include?(object_id) + end + + def prepared_statements_disabled_cache # :nodoc: + Thread.current[:ar_prepared_statements_disabled_cache] ||= Set.new end class Version @@ -264,7 +268,10 @@ def seconds_idle # :nodoc: end def unprepared_statement - @prepared_statement_status.bind(false) { yield } + cache = prepared_statements_disabled_cache.add(object_id) if @prepared_statements + yield + ensure + cache&.delete(object_id) end # Returns the human-readable name of the adapter. Use mixed case - one