mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Reduce the number of times we look up the ScopeRegistry
I noticed in profiles of simple queries (like `Post.where(id: 1).first`) we're spending lots of time looking up the current thread. I couldn't find any way to speed this up in Ruby, so I thought maybe we could call `Thread.current` fewer times per query. This patch should eliminate 4 calls to `Thread.current` per query. For this benchmark: ```ruby StackProf.run(mode: :wall, out: 'out.dump') do 8000.times { Post.where(id: id).first } end ``` `Thread.current` goes from 7% to 4.7% of time: ``` ================================== Mode: wall(1000) Samples: 1633 (0.00% miss rate) GC: 51 (3.12%) ================================== TOTAL (pct) SAMPLES (pct) FRAME 140 (8.6%) 140 (8.6%) String#sub! 114 (7.0%) 114 (7.0%) Thread.current ``` ``` ================================== Mode: wall(1000) Samples: 1719 (0.00% miss rate) GC: 51 (2.97%) ================================== TOTAL (pct) SAMPLES (pct) FRAME 134 (7.8%) 134 (7.8%) String#sub! 99 (5.8%) 99 (5.8%) Module#=== 81 (4.7%) 81 (4.7%) Thread.current ``` This isn't huge, but I think we need to find more sources of Thread.current. It's surprising to me that we spend so much time looking up the current thread when doing a query that is so "easy"
This commit is contained in:
parent
73c18888ad
commit
8af85e161d
3 changed files with 23 additions and 14 deletions
|
@ -421,18 +421,20 @@ module ActiveRecord
|
|||
# Please check unscoped if you want to remove all previous scopes (including
|
||||
# the default_scope) during the execution of a block.
|
||||
def scoping(all_queries: nil)
|
||||
if global_scope? && all_queries == false
|
||||
registry = klass.scope_registry
|
||||
if global_scope?(registry) && all_queries == false
|
||||
raise ArgumentError, "Scoping is set to apply to all queries and cannot be unset in a nested block."
|
||||
elsif already_in_scope?
|
||||
elsif already_in_scope?(registry)
|
||||
yield
|
||||
else
|
||||
_scoping(self, all_queries) { yield }
|
||||
_scoping(self, registry, all_queries) { yield }
|
||||
end
|
||||
end
|
||||
|
||||
def _exec_scope(*args, &block) # :nodoc:
|
||||
@delegate_to_klass = true
|
||||
_scoping(nil) { instance_exec(*args, &block) || self }
|
||||
registry = klass.scope_registry
|
||||
_scoping(nil, registry) { instance_exec(*args, &block) || self }
|
||||
ensure
|
||||
@delegate_to_klass = false
|
||||
end
|
||||
|
@ -831,12 +833,12 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
private
|
||||
def already_in_scope?
|
||||
@delegate_to_klass && klass.current_scope(true)
|
||||
def already_in_scope?(registry)
|
||||
@delegate_to_klass && registry.current_scope(klass, true)
|
||||
end
|
||||
|
||||
def global_scope?
|
||||
klass.global_current_scope(true)
|
||||
def global_scope?(registry)
|
||||
registry.global_current_scope(klass, true)
|
||||
end
|
||||
|
||||
def current_scope_restoring_block(&block)
|
||||
|
@ -859,16 +861,19 @@ module ActiveRecord
|
|||
klass.create!(attributes, &block)
|
||||
end
|
||||
|
||||
def _scoping(scope, all_queries = false)
|
||||
previous, klass.current_scope = klass.current_scope(true), scope
|
||||
def _scoping(scope, registry, all_queries = false)
|
||||
previous = registry.current_scope(klass, true)
|
||||
registry.set_current_scope(klass, scope)
|
||||
|
||||
if all_queries
|
||||
previous_global, klass.global_current_scope = klass.global_current_scope(true), scope
|
||||
previous_global = registry.global_current_scope(klass, true)
|
||||
registry.set_global_current_scope(klass, scope)
|
||||
end
|
||||
yield
|
||||
ensure
|
||||
klass.current_scope = previous
|
||||
registry.set_current_scope(klass, previous)
|
||||
if all_queries
|
||||
klass.global_current_scope = previous_global
|
||||
registry.set_global_current_scope(klass, previous_global)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -8,7 +8,7 @@ module ActiveRecord
|
|||
module SpawnMethods
|
||||
# This is overridden by Associations::CollectionProxy
|
||||
def spawn #:nodoc:
|
||||
already_in_scope? ? klass.all : clone
|
||||
already_in_scope?(klass.scope_registry) ? klass.all : clone
|
||||
end
|
||||
|
||||
# Merges in the conditions from <tt>other</tt>, if <tt>other</tt> is an ActiveRecord::Relation.
|
||||
|
|
|
@ -324,6 +324,10 @@ class FakeKlass
|
|||
extend ActiveRecord::Delegation::DelegateCache
|
||||
|
||||
class << self
|
||||
def scope_registry
|
||||
ActiveRecord::Scoping::ScopeRegistry.instance
|
||||
end
|
||||
|
||||
def connection
|
||||
Post.connection
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue