Skip to content

Commit

Permalink
fix a performance issue with dependent params
Browse files Browse the repository at this point in the history
Fixes #2100

The reason was in `ActiveSupport::HashWithIndifferentAccess`, it
is super expensive.

When users use a `Grape::Extensions::ActiveSupport::HashWithIndifferentAccess::ParamBuilder`
or `Grape::Extensions::Hashie::Mash::ParamBuilder` parameter builder
there is no change. However, users who use `Grape::Extensions::Hash::ParamBuilder`
must make sure a parameter to be dependent on must be a symbol.

    given :matrix do
      # block here
    end

Benchmark after this fix:

Warming up --------------------------------------
               Given     1.000  i/100ms
              Simple     1.000  i/100ms
Calculating -------------------------------------
               Given      0.804  (± 0.0%) i/s -     49.000  in  61.186831s
              Simple      0.855  (± 0.0%) i/s -     52.000  in  60.926097s

Comparison:
              Simple:        0.9 i/s
               Given:        0.8 i/s - 1.06x  slower
  • Loading branch information
dnesteryuk committed Oct 28, 2020
1 parent 1e97967 commit affd474
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#### Fixes

* Your contribution here.
* [#2127](https://github.com/ruby-grape/grape/pull/2127): Fix a performance issue with dependent params - [@dnesteryuk](https://github.com/dnesteryuk).
* [#2126](https://github.com/ruby-grape/grape/pull/2126): Fix warnings about redefined attribute accessors in `AttributeTranslator` - [@samsonjs](https://github.com/samsonjs).
* [#2121](https://github.com/ruby-grape/grape/pull/2121): Fix 2.7 deprecation warning in validator_factory - [@Legogris](https://github.com/Legogris).
* [#2115](https://github.com/ruby-grape/grape/pull/2115): Fix declared_params regression with multiple allowed types - [@stanhu](https://github.com/stanhu).
Expand Down
22 changes: 22 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,28 @@
Upgrading Grape
===============

### Upgrading to >= 1.5.1

#### Dependent params

If you use [dependent params](https://github.com/ruby-grape/grape#dependent-parameters) with
`Grape::Extensions::Hash::ParamBuilder`, make sure a parameter to be dependent on is set as a Symbol.
If a String is given, a parameter that other parameters depend on won't be found even if it is present.

_Correct_:
```ruby
given :matrix do
# dependent params
end
```

_Wrong_:
```ruby
given 'matrix' do
# dependent params
end
```

### Upgrading to >= 1.5.0

Prior to 1.3.3, the `declared` helper would always return the complete params structure if `include_missing=true` was set. In 1.3.3 a regression was introduced such that a missing Hash with or without nested parameters would always resolve to `{}`.
Expand Down
5 changes: 3 additions & 2 deletions lib/grape/validations/params_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ def meets_dependency?(params, request_params)
end

return params.any? { |param| meets_dependency?(param, request_params) } if params.is_a?(Array)
return false unless params.respond_to?(:with_indifferent_access)
params = params.with_indifferent_access

# params might be anything what looks like a hash, so it must implement a `key?` method
return false unless params.respond_to?(:key?)

@dependent_on.each do |dependency|
if dependency.is_a?(Hash)
Expand Down

0 comments on commit affd474

Please sign in to comment.