Commit d7717192 authored by Sean McGivern's avatar Sean McGivern

Speed up DeclarativePolicy::Runner#steps_by_score

There were a couple of things here:

1. If the state was already enabled, we don't need to check all the remaining
   steps - only those that can prevent the state. (An enable followed by an
   enable is a no-op.) This logic is in `#run`, but we still did the work of
   scoring and sorting the steps.
2. The sorting is known to be inefficient, but we can make it slightly more
   efficient by stopping once we have a step with zero score, as that means it's
   free.

Neither of these make this _fast_, especially when called lots of times - as we
do when there is lots of activity on an issue - but they do help some.
parent 4a4f8093
---
title: Speed up permission checks
merge_request:
author:
type: other
...@@ -107,7 +107,7 @@ module DeclarativePolicy ...@@ -107,7 +107,7 @@ module DeclarativePolicy
end end
# This is the core spot where all those `#score` methods matter. # This is the core spot where all those `#score` methods matter.
# It is critcal for performance to run steps in the correct order, # It is critical for performance to run steps in the correct order,
# so that we don't compute expensive conditions (potentially n times # so that we don't compute expensive conditions (potentially n times
# if we're called on, say, a large list of users). # if we're called on, say, a large list of users).
# #
...@@ -139,30 +139,39 @@ module DeclarativePolicy ...@@ -139,30 +139,39 @@ module DeclarativePolicy
return return
end end
steps = Set.new(@steps) remaining_steps = Set.new(@steps)
remaining_enablers = steps.count { |s| s.enable? } remaining_enablers, remaining_preventers = remaining_steps.partition(&:enable?).map { |s| Set.new(s) }
loop do loop do
return if steps.empty? if @state.enabled?
# Once we set this, we never need to unset it, because a single
# prevent will stop this from being enabled
remaining_steps = remaining_preventers
else
# if the permission hasn't yet been enabled and we only have # if the permission hasn't yet been enabled and we only have
# prevent steps left, we short-circuit the state here # prevent steps left, we short-circuit the state here
@state.prevent! if !@state.enabled? && remaining_enablers == 0 @state.prevent! if remaining_enablers.empty?
end
return if remaining_steps.empty?
lowest_score = Float::INFINITY lowest_score = Float::INFINITY
next_step = nil next_step = nil
steps.each do |step| remaining_steps.each do |step|
score = step.score score = step.score
if score < lowest_score if score < lowest_score
next_step = step next_step = step
lowest_score = score lowest_score = score
end end
end
steps.delete(next_step) break if lowest_score.zero?
end
remaining_enablers -= 1 if next_step.enable? [remaining_steps, remaining_enablers, remaining_preventers].each do |set|
set.delete(next_step)
end
yield next_step, lowest_score yield next_step, lowest_score
end end
......
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