Skip to content

Set policy class explicitly for Vulnerability model

What does this MR do and why?

Inferring the policy class for declarative policy can be slow for some classes if the class itself is not the first element in the ancestors list. Luckily we can address this performance issue by explicitly setting the policy class.

Benchmark

Here is a simple benchmark to compare implicit vs explicit policy classes
GL_PATH = "path to gitlab"

ENV['ENABLE_BOOTSNAP'] = '0'
ENV['BUNDLE_GEMFILE'] = "#{GL_PATH}/Gemfile"
system "BUNDLE_GEMFILE=#{ENV['BUNDLE_GEMFILE']} bundle install"

puts "Loading the Application..."
require "#{GL_PATH}/config/environment.rb"

Rails.configuration.eager_load_namespaces.each(&:eager_load!)
puts "Application is loaded."

require 'benchmark/ips'

class VulnerabilityWithExplicitPolicyClass < Vulnerability
  def self.declarative_policy_class
    :VulnerabilityPolicy
  end
end

user = User.first
subject_1 = Vulnerability.first
subject_2 = VulnerabilityWithExplicitPolicyClass.first

Benchmark.ips do |b|
  b.report('implicit') do
    DeclarativePolicy.policy_for(user, subject_1)
  end

  b.report('explicit') do
    DeclarativePolicy.policy_for(user, subject_2)
  end

  b.compare!
end

And the result:

Warming up --------------------------------------
            implicit     1.537k i/100ms
            explicit    27.151k i/100ms
Calculating -------------------------------------
            implicit     19.303k (± 8.4%) i/s -     96.831k in   5.049997s
            explicit    270.085k (± 3.7%) i/s -      1.358M in   5.033705s

Comparison:
            explicit:   270085.5 i/s
            implicit:    19303.1 i/s - 13.99x  slower

WRK test results

This performance difference is more important for the vulnerabilities GraphQL field as we can be serializing 100 vulnerabilities based on user preference which makes the problem 100 times more visible. Therefore I have run a simple test on my local development environment to verify the performance improvement using wrk.

Before the patch

Screenshot_2023-05-08_at_10.23.29

After the patch

Screenshot_2023-05-08_at_10.23.52

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Mehmet Emin INAC

Merge request reports