Skip to content

Commit

Permalink
micro optimizations in allocating hashes and arrays
Browse files Browse the repository at this point in the history
There were a few places where hashes could be mutated without side effects,
they already work with copies of original hashes. So, `merge!` replaced some
of `merge` calls.

Also, `Grape::Util::StackableValues`, `Grape::Util::ReverseStackableValues` and
`Grape::Util::InheritableValues` got a base class which keeps common methods
to avoid duplication. Besides, that a `[]` and `keys` methods were optimized
to allocate less arrays when it is possible.

There is an additional benchmark which measures more complex scenario. Unfortunately,
there isn't noticeable difference in performance between the master and this change.
However, I also profiled the same endpoint as the benchmark has.

Before this change:

     Measure Mode: memory
     Thread ID: 47216803207600
     Fiber ID: 47216805628400
     Total: 40536.000000
     Sort by: self_time

     %self      total      self      wait     child     calls  name
     13.85   6848.000  5616.000     0.000  1232.000       15  *Hash#each_pair
     9.73    3944.000  3944.000     0.000     0.000       17   Hash#merge
     8.80    5088.000  3568.000     0.000  1520.000        8  *Array#map
     8.57   11720.000  3472.000     0.000  8248.000       27  *Class#new
     4.54    1840.000  1840.000     0.000     0.000       46   Symbol#to_s
     4.44    1800.000  1800.000     0.000     0.000       20  *Grape::Util::StackableValues#[]
     3.36    1440.000  1360.000     0.000    80.000        5   Grape::Endpoint#run_filters
     2.88    3040.000  1168.000     0.000  1872.000        9  *Hash#each
     2.37    1200.000   960.000     0.000   240.000        6   <Class::Grape::Router>#normalize_path
     2.29    1088.000   928.000     0.000   160.000        4   ActiveSupport::HashWithIndifferentAccess#[]=
     2.21    1128.000   896.000     0.000   232.000        8  *Kernel#dup
     2.07     840.000   840.000     0.000     0.000        9   String#split
     1.64   12008.000   664.000     0.000 11344.000        1   Grape::Endpoint#run_validators

After this change:

     Measure Mode: memory
     Thread ID: 47390769740200
     Fiber ID: 47390772121400
     Total: 37296.000000
     Sort by: self_time

     %self      total      self      wait     child     calls  name
     15.06   6848.000  5616.000     0.000  1232.000       15  *Hash#each_pair
     9.57    5088.000  3568.000     0.000  1520.000        8  *Array#map
     9.31   11720.000  3472.000     0.000  8248.000       27  *Class#new
     4.93    1840.000  1840.000     0.000     0.000       46   Symbol#to_s
     4.35    1624.000  1624.000     0.000     0.000        7   Hash#merge
     3.65    1440.000  1360.000     0.000    80.000        5   Grape::Endpoint#run_filters
     3.13    3040.000  1168.000     0.000  1872.000        9  *Hash#each
     2.57    1200.000   960.000     0.000   240.000        6   <Class::Grape::Router>#normalize_path
     2.49    1088.000   928.000     0.000   160.000        4   ActiveSupport::HashWithIndifferentAccess#[]=
     2.40    1128.000   896.000     0.000   232.000        8  *Kernel#dup
     2.25     840.000   840.000     0.000     0.000        9   String#split
     2.15    1400.000   800.000     0.000   600.000       20  *Grape::Util::StackableValues#[]
     1.78   11768.000   664.000     0.000 11104.000        1   Grape::Endpoint#run_validators

 - The total memory usage went down from 40536 to 37296 bytes.
 - `Hash#merge` moved from second position to 5th.
 - `Grape::Util::StackableValues#[]` moved from 6th to 12th.
  • Loading branch information
dnesteryuk committed Oct 6, 2019
1 parent 3241eb7 commit 4905995
Show file tree
Hide file tree
Showing 12 changed files with 119 additions and 93 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#### Features

* Your contribution here.
* [#1915](https://github.com/ruby-grape/grape/pull/1915): Micro optimizations in allocating hashes and arrays - [@dnesteryuk](https://github.com/dnesteryuk).
* [#1904](https://github.com/ruby-grape/grape/pull/1904): Allows Grape to load files on startup rather than on the first call - [@myxoh](https://github.com/myxoh).
* [#1907](https://github.com/ruby-grape/grape/pull/1907): Adds outside configuration to Grape with `configure` - [@unleashy](https://github.com/unleashy).

Expand Down
41 changes: 41 additions & 0 deletions benchmark/nested_params.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
require 'grape'
require 'benchmark/ips'

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

params do
requires :address, type: Hash do
requires :street, type: String
requires :postal_code, type: Integer
optional :city, type: String
end
end
post '/' do
'hello'
end
end

options = {
method: 'POST',
params: {
address: {
street: 'Alexis Pl.',
postal_code: '90210',
city: 'Beverly Hills'
}
}
}

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

10.times do |i|
env["HTTP_HEADER#{i}"] = '123'
end

Benchmark.ips do |ips|
ips.report('POST with nested params') do
API.call env
end
end
2 changes: 1 addition & 1 deletion lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ def prepare_version
end

def merge_route_options(**default)
options[:route_options].clone.merge(**default)
options[:route_options].clone.merge!(**default)
end

def map_routes
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/error_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def builtin_formatters
end

def formatters(options)
builtin_formatters.merge(default_elements).merge(options[:error_formatters] || {})
builtin_formatters.merge(default_elements).merge!(options[:error_formatters] || {})
end

def formatter_for(api_format, **options)
Expand Down
6 changes: 4 additions & 2 deletions lib/grape/exceptions/validation_errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,16 @@ def to_json(**_opts)
end

def full_messages
map { |attributes, error| full_message(attributes, error) }.uniq
messages = map { |attributes, error| full_message(attributes, error) }
messages.uniq!
messages
end

private

def full_message(attributes, error)
I18n.t(
'grape.errors.format'.to_sym,
'grape.errors.format',
default: '%{attributes} %{message}',
attributes: attributes.count == 1 ? translate_attribute(attributes.first) : translate_attributes(attributes),
message: error.message
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def builtin_formmaters
end

def formatters(options)
builtin_formmaters.merge(default_elements).merge(options[:formatters] || {})
builtin_formmaters.merge(default_elements).merge!(options[:formatters] || {})
end

def formatter_for(api_format, **options)
Expand Down
2 changes: 1 addition & 1 deletion lib/grape/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def builtin_parsers
end

def parsers(options)
builtin_parsers.merge(default_elements).merge(options[:parsers] || {})
builtin_parsers.merge(default_elements).merge!(options[:parsers] || {})
end

def parser_for(api_format, **options)
Expand Down
34 changes: 34 additions & 0 deletions lib/grape/util/base_inheritable.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
module Grape
module Util
# Base for classes which need to operate with own values kept
# in the hash and inherited values kept in a Hash-like object.
class BaseInheritable
attr_accessor :inherited_values
attr_accessor :new_values

# @param inherited_values [Object] An object implementing an interface
# of the Hash class.
def initialize(inherited_values = {})
@inherited_values = inherited_values
@new_values = {}
end

def delete(key)
new_values.delete key
end

def initialize_copy(other)
super
self.inherited_values = other.inherited_values
self.new_values = other.new_values.dup
end

def keys
combined = inherited_values.keys
combined.concat(new_values.keys)
combined.uniq!
combined
end
end
end
end
30 changes: 5 additions & 25 deletions lib/grape/util/inheritable_values.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
require_relative 'base_inheritable'

module Grape
module Util
class InheritableValues
attr_accessor :inherited_values
attr_accessor :new_values

def initialize(inherited_values = {})
self.inherited_values = inherited_values
self.new_values = {}
end

class InheritableValues < BaseInheritable
def [](name)
values[name]
end
Expand All @@ -17,26 +11,12 @@ def []=(name, value)
new_values[name] = value
end

def delete(key)
new_values.delete key
end

def merge(new_hash)
values.merge(new_hash)
end

def keys
(new_values.keys + inherited_values.keys).sort.uniq
values.merge!(new_hash)
end

def to_hash
values.clone
end

def initialize_copy(other)
super
self.inherited_values = other.inherited_values
self.new_values = other.new_values.dup
values
end

protected
Expand Down
43 changes: 7 additions & 36 deletions lib/grape/util/reverse_stackable_values.rb
Original file line number Diff line number Diff line change
@@ -1,45 +1,16 @@
require_relative 'stackable_values'

module Grape
module Util
class ReverseStackableValues
attr_accessor :inherited_values
attr_accessor :new_values

def initialize(inherited_values = {})
@inherited_values = inherited_values
@new_values = {}
end
class ReverseStackableValues < StackableValues
protected

def [](name)
def concat_values(inherited_value, new_value)
[].tap do |value|
value.concat(@new_values[name] || [])
value.concat(@inherited_values[name] || [])
end
end

def []=(name, value)
@new_values[name] ||= []
@new_values[name].push value
end

def delete(key)
new_values.delete key
end

def keys
(@new_values.keys + @inherited_values.keys).sort.uniq
end

def to_hash
keys.each_with_object({}) do |key, result|
result[key] = self[key]
value.concat(new_value)
value.concat(inherited_value)
end
end

def initialize_copy(other)
super
self.inherited_values = other.inherited_values
self.new_values = other.new_values.dup
end
end
end
end
41 changes: 19 additions & 22 deletions lib/grape/util/stackable_values.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
require_relative 'base_inheritable'

module Grape
module Util
class StackableValues
attr_accessor :inherited_values
attr_accessor :new_values
class StackableValues < BaseInheritable
attr_reader :frozen_values

def initialize(inherited_values = {})
@inherited_values = inherited_values
@new_values = {}
def initialize(*_args)
super

@frozen_values = {}
end

def [](name)
return @frozen_values[name] if @frozen_values.key? name

value = []
value.concat(@inherited_values[name] || [])
value.concat(@new_values[name] || [])
value
inherited_value = @inherited_values[name]
new_value = @new_values[name] || []

return new_value unless inherited_value

concat_values(inherited_value, new_value)
end

def []=(name, value)
Expand All @@ -26,14 +28,6 @@ def []=(name, value)
@new_values[name].push value
end

def delete(key)
new_values.delete key
end

def keys
(@new_values.keys + @inherited_values.keys).sort.uniq
end

def to_hash
keys.each_with_object({}) do |key, result|
result[key] = self[key]
Expand All @@ -44,10 +38,13 @@ def freeze_value(key)
@frozen_values[key] = self[key].freeze
end

def initialize_copy(other)
super
self.inherited_values = other.inherited_values
self.new_values = other.new_values.dup
protected

def concat_values(inherited_value, new_value)
[].tap do |value|
value.concat(inherited_value)
value.concat(new_value)
end
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions lib/grape/validations/validators/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ def validate!(params)

def self.convert_to_short_name(klass)
ret = klass.name.gsub(/::/, '/')
.gsub(/([A-Z]+)([A-Z][a-z])/, '\1_\2')
.gsub(/([a-z\d])([A-Z])/, '\1_\2')
.tr('-', '_')
.downcase
ret.gsub!(/([A-Z]+)([A-Z][a-z])/, '\1_\2')
ret.gsub!(/([a-z\d])([A-Z])/, '\1_\2')
ret.tr!('-', '_')
ret.downcase!
File.basename(ret, '_validator')
end

Expand Down

0 comments on commit 4905995

Please sign in to comment.