Performance degradation (~6x) when using given to model a dependent relation

[senhalil]

When change one of the parameters of out API as dependent to another one, the post duration increased from ~30 seconds to ~600 seconds. Is this normal?

Our model is deeply nested but both the dependee and depender are on the first level.

params do
  optional :matrix
  given matrix: ->(val) { val.nil? || val.empty? } do
    requires :vehicle, type: Hash do
      requires :router_mode, type: String
    end
  end
  given matrix: ->(val) { val.any? } do
    requires :vehicle, type: Hash do
      optional :matrix_id, type: String
      optional :router_mode, type: String
      exactly_one_of :matrix_id, :router_mode
    end
  end
end

[dblock]

I bet it's fixed by #2096. Try HEAD?

[senhalil]

Unfortunately, the performance degradation still exists. Actually, I work together with @braktar and we tested on his computer with the fix.

[dblock]

Put this into a benchmark and let's fix it.

[dnesteryuk]

@senhalil are you going to work on the benchmark?

[senhalil]

@dnesteryuk this issue fell in the cracks apparently, thanks for the ping.

I don't think I will be able to work on this issue since we gave up using given. The performance issue is still there and can be observed by making one of the parameters dependent in the existing large_model benchmark https://github.com/ruby-grape/grape/blob/master/benchmark/large_model.rb

[dnesteryuk]

I wrote this simple benchmark to compare params without dependencies and with them:

require 'grape'
require 'benchmark/ips'

class GivenAPI < Grape::API
  prefix :api
  version 'v1', using: :path

  params do
    optional :matrix
    given matrix: ->(val) { val.nil? || val.empty? } do
      requires :vehicle, type: Hash do
        requires :router_mode, type: String
      end
    end
    given matrix: ->(val) { val.any? } do
      requires :vehicle, type: Hash do
        optional :matrix_id, type: String
        optional :router_mode, type: String
        exactly_one_of :matrix_id, :router_mode
      end
    end
  end
  post '/' do
    'hello'
  end
end

class SimpleAPI < Grape::API
  prefix :api
  version 'v1', using: :path

  params do
    optional :matrix

    requires :vehicle, type: Hash do
      optional :matrix_id, type: String
      optional :router_mode, type: String
      exactly_one_of :matrix_id, :router_mode
    end
  end
  post '/' do
    'hello'
  end
end

options = {
  method: 'POST',
  params: {
    matrix: {
      something: 'test'
    },
    vehicle: {
      router_mode: 'hello'
    }
  }
}

env = Rack::MockRequest.env_for('/api/v1', options)

# warm up
GivenAPI.call

Benchmark.ips do |ips|
  ips.report('Given') do
    GivenAPI.call env
  end

  ips.report('Simple') do
    SimpleAPI.call env
  end

  ips.compare!
end

The result:

Warming up --------------------------------------
               Given   388.000  i/100ms
              Simple   583.000  i/100ms
Calculating -------------------------------------
               Given      3.938k (± 2.6%) i/s -     19.788k in   5.028092s
              Simple      5.920k (± 2.1%) i/s -     29.733k in   5.024627s

Comparison:
              Simple:     5920.1 i/s
               Given:     3938.2 i/s - 1.50x  slower

I would say it isn't that bad.

[senhalil]

I happened to have sometime this weekend, I thought I can chip-in.

I suspect this issue is related to data size and model parameter type, that's why it doesn't show up on a simple model with a small instance. I am still surprised that it takes 1.50x slower even with this model and instance.

I should have shared the definition of the model parameters properly in my original post. The matrix parameter actually has parameters of type Array[Array[Float]] inside (which are not taking part in the conditional statements). Even though neither the definition of these matrix elements depends on the conditional statement nor they appear in the condition itself, somehow they decrease the performance.

I have found an error in my original code that produced 20x difference -- one of the parameters were not defined in the SimpleAPI 🤦 -- sorry for the hype!

I created the following smallish model from scratch by cropping the irrelevant parts of ./benchmark/large_model.rb. The perf diff is around 6x with vrp_w_matrix_example.json (which is the relevant parts of a not so small but definitely not big VRP problem of 432 points).

I suspect the bigger the data gets, the worse the performance difference gets -- even if the part of the data have nothing to do with the conditional part of the model.

Warming up --------------------------------------
               Given     1.000  i/100ms
              Simple     1.000  i/100ms
Calculating -------------------------------------
               Given      0.109  (± 0.0%) i/s -      7.000  in  65.676987s
              Simple      0.720  (± 0.0%) i/s -     43.000  in  60.040736s

Comparison:
              Simple:        0.7 i/s
               Given:        0.1 i/s - 6.58x  (± 0.00) slower

require 'grape'
require 'benchmark/ips'

class SimpleAPI < Grape::API
  prefix :api
  version 'v1', using: :path
  content_type :json, 'application/json; charset=UTF-8'
  default_format :json

  def self.vrp_request_point(this)
    this.requires(:id, type: String, allow_blank: false)
    this.optional(:matrix_index, type: Integer, allow_blank: false, desc: 'Index within the matrices')
    this.optional(:location, type: Hash, allow_blank: false) do
      requires(:lat, type: Float, allow_blank: false)
      requires(:lon, type: Float, allow_blank: false)
    end
    this.exactly_one_of :matrix_index, :location
  end

  params do
    optional(:vrp, type: Hash, documentation: { param_type: 'body' }) do
      optional(:name, type: String)

      optional(:matrices, type: Array, documentation: { desc: 'Define the distances' }) do
        requires(:id, type: String, allow_blank: false)
        optional(:time, type: Array[Array[Float]], allow_blank: false, desc: 'Matrix of time')
        optional(:distance, type: Array[Array[Float]], allow_blank: false, desc: 'Matrix of distance')
      end

      optional(:points, type: Array) do
        SimpleAPI.vrp_request_point(self)
      end
    end
  end

  post '/' do
    'hello'
  end
end


class GivenAPI < Grape::API
  prefix :api
  version 'v1', using: :path
  content_type :json, 'application/json; charset=UTF-8'
  default_format :json

  def self.vrp_request_point(this)
    this.requires(:id, type: String, allow_blank: false)
    this.optional(:location, type: Hash, allow_blank: false) do
      requires(:lat, type: Float, allow_blank: false)
      requires(:lon, type: Float, allow_blank: false)
    end
  end

  params do
    optional(:vrp, type: Hash, documentation: { param_type: 'body' }) do
      optional(:name, type: String)

      optional(:matrices, type: Array, documentation: { desc: 'Define the distances' }) do
        requires(:id, type: String, allow_blank: false)
        optional(:time, type: Array[Array[Float]], allow_blank: false, desc: 'Matrix of time')
        optional(:distance, type: Array[Array[Float]], allow_blank: false, desc: 'Matrix of distance')
      end

      given matrices: ->(val) { val.nil? || val.empty? } do
        optional(:points, type: Array, documentation: { desc: 'Particular place on the map' }) do
          GivenAPI.vrp_request_point(self)
          exactly_one_of :location
        end
      end
      given matrices: ->(val) { val && !val.empty? } do
        optional(:points, type: Array, documentation: { desc: 'Particular place in the map' }) do
          GivenAPI.vrp_request_point(self)
          optional(:matrix_index, type: Integer, allow_blank: false, desc: 'Index within the matrices')
          exactly_one_of :matrix_index, :location
        end
      end
    end
  end
  post '/' do
    'hello'
  end
end

options = {
  method: 'POST',
  params: JSON.parse(File.read('vrp_w_matrix_example.json')) # https://gist.github.com/senhalil/263188ebdbe04ef545fc4278838cd8d8#file-vrp_w_matrix_example-json
}

env = Rack::MockRequest.env_for('/api/v1', options)

Benchmark.ips do |ips|
  ips.config(time: 60, warmup: 1)

  ips.report('Given') do
    GivenAPI.call env
  end

  ips.report('Simple') do
    SimpleAPI.call env
  end

  ips.compare!
end