Skip to content

Commit e356c63

Browse files
committed
Refactor query cache and connection lease registry for performance
Fix: rails#52617 Followup: rails#52622 Previous fixes solved the memory issues, but our fallback implementation of WeakKeyMap actually have terrible performance, and I can't find a way to do it in a performent way. So instead we replace it by a specialized weak map that only accept Thread or Fiber as keys, and simple purge dead threads on insertion. This gives us reasonable performance on Ruby 3.1 and 3.2.
1 parent f26faa8 commit e356c63

File tree

2 files changed

+33
-31
lines changed

2 files changed

+33
-31
lines changed

activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -119,41 +119,25 @@ def dirties_query_cache
119119
# are now explicitly documented
120120
class ConnectionPool
121121
if ObjectSpace.const_defined?(:WeakKeyMap) # RUBY_VERSION >= 3.3
122-
WeakKeyMap = ::ObjectSpace::WeakKeyMap # :nodoc:
122+
WeakThreadKeyMap = ::ObjectSpace::WeakKeyMap # :nodoc:
123123
else
124-
class WeakKeyMap # :nodoc:
124+
class WeakThreadKeyMap # :nodoc:
125125
def initialize
126-
@map = ObjectSpace::WeakMap.new
127-
@values = nil
128-
@size = 0
126+
@map = {}
129127
end
130128

131-
alias_method :clear, :initialize
129+
def clear
130+
@map.clear
131+
end
132132

133133
def [](key)
134-
prune if @map.size != @size
135134
@map[key]
136135
end
137136

138137
def []=(key, value)
138+
@map.select! { |c, _| c.alive? }
139139
@map[key] = value
140-
prune if @map.size != @size
141-
value
142-
end
143-
144-
def delete(key)
145-
if value = self[key]
146-
self[key] = nil
147-
prune
148-
end
149-
value
150140
end
151-
152-
private
153-
def prune(force = false)
154-
@values = @map.values
155-
@size = @map.size
156-
end
157141
end
158142
end
159143

@@ -186,7 +170,7 @@ def clear(connection)
186170
class LeaseRegistry # :nodoc:
187171
def initialize
188172
@mutex = Mutex.new
189-
@map = WeakKeyMap.new
173+
@map = WeakThreadKeyMap.new
190174
end
191175

192176
def [](context)
@@ -197,7 +181,7 @@ def [](context)
197181

198182
def clear
199183
@mutex.synchronize do
200-
@map = WeakKeyMap.new
184+
@map.clear
201185
end
202186
end
203187
end

activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ def [](key)
6666

6767
def compute_if_absent(key)
6868
check_version
69+
6970
return yield unless @enabled
7071

7172
if entry = @map.delete(key)
@@ -93,10 +94,30 @@ def check_version
9394
end
9495
end
9596

97+
class QueryCacheRegistry # :nodoc:
98+
def initialize
99+
@mutex = Mutex.new
100+
@map = ConnectionPool::WeakThreadKeyMap.new
101+
end
102+
103+
def compute_if_absent(context)
104+
@map[context] || @mutex.synchronize do
105+
@map[context] ||= yield
106+
end
107+
end
108+
109+
def clear
110+
@map.synchronize do
111+
@map.clear
112+
end
113+
end
114+
end
115+
96116
module ConnectionPoolConfiguration # :nodoc:
97117
def initialize(...)
98118
super
99119
@query_cache_version = Concurrent::AtomicFixnum.new
120+
@thread_query_caches = QueryCacheRegistry.new
100121
@query_cache_max_size = \
101122
case query_cache = db_config&.query_cache
102123
when 0, false
@@ -108,10 +129,6 @@ def initialize(...)
108129
end
109130
end
110131

111-
def query_cache_version
112-
synchronize { @query_cache_version }
113-
end
114-
115132
def checkout_and_verify(connection)
116133
super
117134
connection.query_cache ||= query_cache
@@ -166,8 +183,9 @@ def clear_query_cache
166183
end
167184

168185
def query_cache
169-
caches = ActiveSupport::IsolatedExecutionState[:active_record_query_caches] ||= ConnectionPool::WeakKeyMap.new
170-
caches[self] ||= Store.new(@query_cache_version, @query_cache_max_size)
186+
@thread_query_caches.compute_if_absent(ActiveSupport::IsolatedExecutionState.context) do
187+
Store.new(@query_cache_version, @query_cache_max_size)
188+
end
171189
end
172190
end
173191

0 commit comments

Comments
 (0)