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

String#gsub given a String can avoid Regexp #5905

Closed
headius opened this issue Oct 3, 2019 · 7 comments
Closed

String#gsub given a String can avoid Regexp #5905

headius opened this issue Oct 3, 2019 · 7 comments

Comments

@headius
Copy link
Member

@headius headius commented Oct 3, 2019

It appears in Ruby 2.2 a change by @nobu modified the logic to just call String#index when passing a String to String#gsub, rather than always using a Regexp object and paying the cost to compile it every time.

We still do the old behavior, making this form of gsub slower than it could be.

rvm truffleruby do ruby bench_gsub.rb 
Warming up --------------------------------------
         gsub-string    24.074k i/100ms
          gsub-regex    79.710k i/100ms
    gsub-regex-block    65.247k i/100ms
Calculating -------------------------------------
         gsub-string    354.958k (±34.9%) i/s -      1.420M in   5.036696s
          gsub-regex      2.097M (±34.5%) i/s -      8.290M in   5.010322s
    gsub-regex-block      2.007M (±27.7%) i/s -      8.417M in   5.004553s

$ rvm ruby-2.6.4 do ruby bench_gsub.rb 
Warming up --------------------------------------
         gsub-string    88.943k i/100ms
          gsub-regex    51.561k i/100ms
    gsub-regex-block    49.026k i/100ms
Calculating -------------------------------------
         gsub-string      1.115M (± 5.2%) i/s -      5.603M in   5.041119s
          gsub-regex    596.028k (± 6.3%) i/s -      2.991M in   5.041490s
    gsub-regex-block    546.740k (± 3.7%) i/s -      2.745M in   5.028991s

$ jruby -Xcompile.invokedynamic bench_gsub.rb 
Warming up --------------------------------------
         gsub-string    92.520k i/100ms
          gsub-regex   142.823k i/100ms
    gsub-regex-block   109.341k i/100ms
Calculating -------------------------------------
         gsub-string      2.188M (± 8.8%) i/s -     10.825M in   5.001965s
          gsub-regex      3.631M (± 9.5%) i/s -     17.996M in   5.024391s
    gsub-regex-block      2.315M (± 7.6%) i/s -     11.481M in   4.999877s

The change in question is here: ruby/ruby@5752b61

Probably not a difficult one to duplicate in JRuby, for anyone who wants to do some spelunking.

@rdubya
Copy link

@rdubya rdubya commented Oct 4, 2019

Can you you throw your bench_gsub.rb file in here or is it in the codebase somewhere?

@headius
Copy link
Member Author

@headius headius commented Oct 8, 2019

Oops, that's a good idea.

headius added a commit that referenced this issue Oct 8, 2019
@fidothe
Copy link
Contributor

@fidothe fidothe commented Oct 16, 2019

Looks like this also applies to String#sub. Should I generate a separate benchmark file or update the existing bench_gsub.rb?

@headius
Copy link
Member Author

@headius headius commented Oct 29, 2019

For anyone else following along... @fidothe has been working with us to implement this. If you'd like to help, coordinate with him and us on Matrix at https://matrix.to/#/#jruby:matrix.org

@fidothe
Copy link
Contributor

@fidothe fidothe commented Oct 30, 2019

@fidothe
Copy link
Contributor

@fidothe fidothe commented Dec 5, 2019

@headius presumably this issue can be closed now?

@headius
Copy link
Member Author

@headius headius commented Dec 5, 2019

@fidothe Oh yes, it can!

@headius headius closed this Dec 5, 2019
@headius headius added this to the JRuby 9.2.10.0 milestone Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants