Reduce allocations in camelize inflector#41296
Merged
Merged
Conversation
a28f41a to
b68eaf2
Compare
kaspth
approved these changes
Feb 1, 2021
This method is the 4th biggest source of allocation during our application
boot (~300k allocations).
The goal of this PR was mostly to reduce allocations, but it also translate
to a 5-17% speedup.
The main change is leveraging `capitalize!` rather than `capitalize`.
The second important change is to merge the two `gsub!` together.
```ruby
require 'benchmark/ips'
require 'active_support/all'
module ActiveSupport
module Inflector
def camelize2(term, uppercase_first_letter = true)
string = term.to_s
if uppercase_first_letter
string = string.sub(/^[a-z\d]*/) { |match| inflections.acronyms[match] || match.capitalize! || match }
else
string = string.sub(inflections.acronyms_camelize_regex) { |match| match.downcase! || match }
end
string.gsub!(/(?:_|(\/))([a-z\d]*)/i) do
if $1
"::#{inflections.acronyms[$2] || $2.capitalize! || $2}"
else
inflections.acronyms[$2] || $2.capitalize! || $2
end
end
string
end
end
end
%w(foo foo_bar foo/bar_baz).each do |str|
puts "== Comparing with #{str.inspect} (#{RUBY_VERSION}) =="
Benchmark.ips do |x|
x.report('camelize') { ActiveSupport::Inflector.camelize(str) }
x.report('camelize2') { ActiveSupport::Inflector.camelize2(str) }
x.compare!
end
puts
end
```
Results
```
== Comparing with "foo" (2.7.2) ==
Warming up --------------------------------------
camelize 53.646k i/100ms
camelize2 56.797k i/100ms
Calculating -------------------------------------
camelize 532.012k (± 1.3%) i/s - 2.682M in 5.042620s
camelize2 567.317k (± 1.3%) i/s - 2.840M in 5.006633s
Comparison:
camelize2: 567317.4 i/s
camelize: 532012.3 i/s - 1.07x (± 0.00) slower
== Comparing with "foo_bar" (2.7.2) ==
Warming up --------------------------------------
camelize 24.010k i/100ms
camelize2 25.398k i/100ms
Calculating -------------------------------------
camelize 239.036k (± 1.1%) i/s - 1.200M in 5.022811s
camelize2 254.229k (± 1.3%) i/s - 1.295M in 5.095855s
Comparison:
camelize2: 254228.8 i/s
camelize: 239036.4 i/s - 1.06x (± 0.00) slower
== Comparing with "foo/bar_baz" (2.7.2) ==
Warming up --------------------------------------
camelize 17.116k i/100ms
camelize2 19.628k i/100ms
Calculating -------------------------------------
camelize 169.210k (± 1.0%) i/s - 855.800k in 5.058138s
camelize2 199.385k (± 2.1%) i/s - 1.001M in 5.022935s
Comparison:
camelize2: 199384.7 i/s
camelize: 169210.0 i/s - 1.18x (± 0.00) slower
```
b68eaf2 to
73bc476
Compare
Contributor
|
Perfect, looks like the test failure was unrelated. |
Contributor
Author
|
Thanks! |
This was referenced Feb 2, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This method is the 4th biggest source of allocation during our application
boot (~300k allocations).
The goal of this PR was mostly to reduce allocations, but it also translate
to a 5-17% speedup.
The main change is leveraging
capitalize!rather thancapitalize.The second important change is to merge the two
gsub!together.Results
cc @rafaelfranca @etiennebarrie
I think other inflectors could likely benefit from a similar optimization.