Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use #start_with? and #[] for speed #21100

Merged
merged 1 commit into from Aug 2, 2015
Merged

Conversation

bquorning
Copy link
Contributor

This is a follow-up to #21057 as per https://github.com/rails/rails/pull/21057/files#r36032324

While the readability may be slightly worse, the speed improvement is significant: Twice as fast when there's no leading / to remove, and over 4 times as fast when there is a leading /.

Benchmark:

require 'benchmark/ips'

def match(controller)
  if controller
    if m = controller.match(/\A\/(?<controller_without_leading_slash>.*)/)
      m[:controller_without_leading_slash]
    else
      controller
    end
  end
end

def start_with(controller)
  if controller
    if controller.start_with?('/'.freeze)
      controller[1..-1]
    else
      controller
    end
  end
end

Benchmark.ips do |x|
  x.report("match") { match("no_leading_slash") }
  x.report("start_with") { start_with("no_leading_slash") }

  x.compare!
end

Benchmark.ips do |x|
  x.report("match") { match("/a_leading_slash") }
  x.report("start_with") { start_with("/a_leading_slash") }

  x.compare!
end

Result (Ruby 2.2.2):

Calculating -------------------------------------
               match    70.324k i/100ms
          start_with   111.264k i/100ms
-------------------------------------------------
               match      1.468M (± 7.1%) i/s -      7.314M
          start_with      3.787M (± 3.5%) i/s -     18.915M

Comparison:
          start_with:  3787389.4 i/s
               match:  1467636.4 i/s - 2.58x slower

Calculating -------------------------------------
               match    36.694k i/100ms
          start_with    86.071k i/100ms
-------------------------------------------------
               match    532.795k (± 4.7%) i/s -      2.679M
          start_with      2.518M (± 5.8%) i/s -     12.566M

Comparison:
          start_with:  2518366.8 i/s
               match:   532794.5 i/s - 4.73x slower

cc @schneems

While the readability may be slightly worse, the speed improvement is
significant: Twice as fast when there's no leading "/" to remove, and
over 4 times as fast when there is a leading "/".

Benchmark:

    require 'benchmark/ips'

    def match(controller)
      if controller
        if m = controller.match(/\A\/(?<controller_without_leading_slash>.*)/)
          m[:controller_without_leading_slash]
        else
          controller
        end
      end
    end

    def start_with(controller)
      if controller
        if controller.start_with?('/'.freeze)
          controller[1..-1]
        else
          controller
        end
      end
    end

    Benchmark.ips do |x|
      x.report("match") { match("no_leading_slash") }
      x.report("start_with") { start_with("no_leading_slash") }

      x.compare!
    end

    Benchmark.ips do |x|
      x.report("match") { match("/a_leading_slash") }
      x.report("start_with") { start_with("/a_leading_slash") }

      x.compare!
    end

Result (Ruby 2.2.2):

    Calculating -------------------------------------
                   match    70.324k i/100ms
              start_with   111.264k i/100ms
    -------------------------------------------------
                   match      1.468M (± 7.1%) i/s -      7.314M
              start_with      3.787M (± 3.5%) i/s -     18.915M

    Comparison:
              start_with:  3787389.4 i/s
                   match:  1467636.4 i/s - 2.58x slower

    Calculating -------------------------------------
                   match    36.694k i/100ms
              start_with    86.071k i/100ms
    -------------------------------------------------
                   match    532.795k (± 4.7%) i/s -      2.679M
              start_with      2.518M (± 5.8%) i/s -     12.566M

    Comparison:
              start_with:  2518366.8 i/s
                   match:   532794.5 i/s - 4.73x slower
@schneems
Copy link
Member

schneems commented Aug 2, 2015

I ran this local and my test app does hit this optimization. Doing some math we save 4.3108037795403217e-07 seconds per iteration and I hit this 1776 times per request. So this PR would buy me 1776 * 4.3108037795403217e-07 # => 0.0007655987512463612 seconds per request or 0.7655987512463611 miliseconds per request. So over the course of 1000 requests we would save 765.5 miliseconds. Great work.

schneems added a commit that referenced this pull request Aug 2, 2015
Use #start_with? and #[] for speed
@schneems schneems merged commit 61ed2b3 into rails:master Aug 2, 2015
@bquorning bquorning deleted the route-set branch August 2, 2015 18:02
@bquorning
Copy link
Contributor Author

Thanks!

So over the course of 1000 requests we would save 765.5 miliseconds.

It all adds up.

@schneems
Copy link
Member

schneems commented Aug 2, 2015

Totally. Two of these patches would be 1.5 ms per request and 1.5 seconds over 1000 requests. 20 of them would save us 15 seconds over 1000 requests. 👏

@gazay
Copy link
Contributor

gazay commented Aug 8, 2015

#17316 👍

@philipgiuliani
Copy link

For me the readability is much better, not worse :D Nice find!

@bf4
Copy link
Contributor

bf4 commented Aug 10, 2015

Did anyone benchmark this on non-c rubies? JRuby, Rbx, etc? (Just curious)

@schneems
Copy link
Member

@bf4 You have the Power!

Also no, I didn't. I don't think anyone else did either, though I would be surprised since match() creates an object and starts_with? does not. I always encourage benchmarking, and posting of benchmarks, don't take OP's word for it, try it yourself 😉

@bquorning
Copy link
Contributor Author

@bf4 You made me curious as well, so I tried running the same benchmark file on Rubinius (I don’t have JRuby installed). The results are quite interesting:

Calculating -------------------------------------
               match   129.966k i/100ms
          start_with   125.731k i/100ms
-------------------------------------------------
               match      2.540M (± 7.9%) i/s -     12.607M
          start_with      1.688M (± 6.1%) i/s -      8.424M

Comparison:
               match:  2539686.6 i/s
          start_with:  1687994.7 i/s - 1.50x slower

Calculating -------------------------------------
               match    65.451k i/100ms
          start_with    90.956k i/100ms
-------------------------------------------------
               match    785.147k (± 4.8%) i/s -      3.927M
          start_with      1.070M (± 2.2%) i/s -      5.366M

Comparison:
          start_with:  1069531.1 i/s
               match:   785147.0 i/s - 1.36x slower

On Rubinius, the regex version is actually faster than start_with? when there’s no match. When there is a match, the new version is still faster.

Also I’m a bit confused how to read “some x slower” message. The “1.50x slower” above actually means “33% slower”, right?

@bf4
Copy link
Contributor

bf4 commented Aug 11, 2015

On my late 2013 mbp with SSD and 16mg ram under rvm

jruby-1.7.11

Calculating -------------------------------------
               match   163.812k i/100ms
          start_with   234.837k i/100ms
-------------------------------------------------
               match      4.961M (± 5.8%) i/s -     24.736M
          start_with      8.103M (± 6.3%) i/s -     40.392M

Comparison:
          start_with:  8102566.0 i/s
               match:  4960875.9 i/s - 1.63x slower

Calculating -------------------------------------
               match    97.730k i/100ms
          start_with   196.458k i/100ms
-------------------------------------------------
               match      1.597M (± 9.3%) i/s -      8.014M
          start_with      5.653M (± 9.5%) i/s -     28.093M

Comparison:
          start_with:  5,653,184.5 i/s
               match:  1,596,900.8 i/s - 3.54x slower

(Running with env JRUBY_OPTS='--server -Xcompile.invokedynamic=false -Xcompat.version=2.0' doesn't appreciably change the differential.)

Rbx-2.2.6

Calculating -------------------------------------
               match   124.906k i/100ms
          start_with   110.187k i/100ms
-------------------------------------------------
               match      2.186M (± 7.4%) i/s -     10.867M
          start_with      1.374M (± 5.3%) i/s -      6.942M

Comparison:
               match:  2186229.5 i/s
          start_with:  1374060.8 i/s - 1.59x slower

Calculating -------------------------------------
               match    61.650k i/100ms
          start_with    71.189k i/100ms
-------------------------------------------------
               match    701.384k (± 6.8%) i/s -      3.514M
          start_with    869.309k (± 5.9%) i/s -      4.343M

Comparison:
          start_with:   869,309.3 i/s
               match:   701,384.0 i/s - 1.24x slower

jruby-9.0.0.0.pre1

Calculating -------------------------------------
               match    25.359k i/100ms
          start_with    34.234k i/100ms
-------------------------------------------------
               match    648.181k (±10.5%) i/s -      3.221M
          start_with      1.219M (±12.6%) i/s -      5.957M

Comparison:
          start_with:  1218611.1 i/s
               match:   648181.3 i/s - 1.88x slower

Calculating -------------------------------------
               match    21.767k i/100ms
          start_with    36.031k i/100ms
-------------------------------------------------
               match    374.600k (± 8.3%) i/s -      1.872M
          start_with      1.010M (± 9.5%) i/s -      5.008M

Comparison:
          start_with:  1,010,252.9 i/s
               match:   374,600.3 i/s - 2.70x slower

@schneems By the power of OSS...

image

@schneems
Copy link
Member

@bf4 cool, i wonder if there's some optimizations to be made in rbx's `starts_with? https://github.com/rubinius/rubinius/blob/32b4d39faff249740c0235f41f4d40526a5085d6/kernel/common/string.rb#L2321 compared to https://github.com/ruby/ruby/blob/c7dc6a34b50e4d9d76c1fe9d2888ccb2aa6aeef3/string.c#L8202-L8215

@bquorning

Also I’m a bit confused how to read “some x slower” message. The “1.50x slower” above actually means “33% slower”, right?

I never use the x.compare! option when using benchmark/ips, I think it's super confusing.

@bquorning
Copy link
Contributor Author

I never use the x.compare! option when using benchmark/ips, I think it's super confusing.

@schneems Let’s fix it: evanphx/benchmark-ips#49

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants