Commit 786879e3 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'perf/policy-class-cache' into 'master'

Perf: Improve performance of basic ruby ops in DeclarativePolicy

See merge request !12922
parents 9d11a373 fcd3e5d4
...@@ -8,7 +8,12 @@ require_dependency 'declarative_policy/step' ...@@ -8,7 +8,12 @@ require_dependency 'declarative_policy/step'
require_dependency 'declarative_policy/base' require_dependency 'declarative_policy/base'
require 'thread'
module DeclarativePolicy module DeclarativePolicy
CLASS_CACHE_MUTEX = Mutex.new
CLASS_CACHE_IVAR = :@__DeclarativePolicy_CLASS_CACHE
class << self class << self
def policy_for(user, subject, opts = {}) def policy_for(user, subject, opts = {})
cache = opts[:cache] || {} cache = opts[:cache] || {}
...@@ -23,7 +28,36 @@ module DeclarativePolicy ...@@ -23,7 +28,36 @@ module DeclarativePolicy
subject = find_delegate(subject) subject = find_delegate(subject)
subject.class.ancestors.each do |klass| class_for_class(subject.class)
end
private
# This method is heavily cached because there are a lot of anonymous
# modules in play in a typical rails app, and #name performs quite
# slowly for anonymous classes and modules.
#
# See https://bugs.ruby-lang.org/issues/11119
#
# if the above bug is resolved, this caching could likely be removed.
def class_for_class(subject_class)
unless subject_class.instance_variable_defined?(CLASS_CACHE_IVAR)
CLASS_CACHE_MUTEX.synchronize do
# re-check in case of a race
break if subject_class.instance_variable_defined?(CLASS_CACHE_IVAR)
policy_class = compute_class_for_class(subject_class)
subject_class.instance_variable_set(CLASS_CACHE_IVAR, policy_class)
end
end
policy_class = subject_class.instance_variable_get(CLASS_CACHE_IVAR)
raise "no policy for #{subject.class.name}" if policy_class.nil?
policy_class
end
def compute_class_for_class(subject_class)
subject_class.ancestors.each do |klass|
next unless klass.name next unless klass.name
begin begin
...@@ -37,12 +71,8 @@ module DeclarativePolicy ...@@ -37,12 +71,8 @@ module DeclarativePolicy
nil nil
end end
end end
raise "no policy for #{subject.class.name}"
end end
private
def find_delegate(subject) def find_delegate(subject)
seen = Set.new seen = Set.new
......
...@@ -21,11 +21,14 @@ module DeclarativePolicy ...@@ -21,11 +21,14 @@ module DeclarativePolicy
private private
def id_for(obj) def id_for(obj)
if obj.respond_to?(:id) && obj.id id =
obj.id.to_s begin
else obj.id
"##{obj.object_id}" rescue NoMethodError
end nil
end
id || "##{obj.object_id}"
end end
end end
end end
......
...@@ -82,13 +82,14 @@ module DeclarativePolicy ...@@ -82,13 +82,14 @@ module DeclarativePolicy
# depending on the scope, we may cache only by the user or only by # depending on the scope, we may cache only by the user or only by
# the subject, resulting in sharing across different policy objects. # the subject, resulting in sharing across different policy objects.
def cache_key def cache_key
case @condition.scope @cache_key ||=
when :normal then "/dp/condition/#{@condition.key}/#{user_key},#{subject_key}" case @condition.scope
when :user then "/dp/condition/#{@condition.key}/#{user_key}" when :normal then "/dp/condition/#{@condition.key}/#{user_key},#{subject_key}"
when :subject then "/dp/condition/#{@condition.key}/#{subject_key}" when :user then "/dp/condition/#{@condition.key}/#{user_key}"
when :global then "/dp/condition/#{@condition.key}" when :subject then "/dp/condition/#{@condition.key}/#{subject_key}"
else raise 'invalid scope' when :global then "/dp/condition/#{@condition.key}"
end else raise 'invalid scope'
end
end end
def user_key def user_key
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment