Skip to content

Reduce allocations in camelize inflector#41296

Merged
kaspth merged 1 commit into
rails:mainfrom
Shopify:optimize-camelize
Feb 1, 2021
Merged

Reduce allocations in camelize inflector#41296
kaspth merged 1 commit into
rails:mainfrom
Shopify:optimize-camelize

Conversation

@casperisfine

Copy link
Copy Markdown
Contributor

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.

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

cc @rafaelfranca @etiennebarrie

I think other inflectors could likely benefit from a similar optimization.

Comment thread activesupport/lib/active_support/inflector/methods.rb Outdated
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
```
@kaspth kaspth merged commit 25028ad into rails:main Feb 1, 2021
@kaspth

kaspth commented Feb 1, 2021

Copy link
Copy Markdown
Contributor

Perfect, looks like the test failure was unrelated.

@casperisfine

Copy link
Copy Markdown
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants