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

Optimize String#downcase and String#upcase for single byte optimizabl… #12389

Merged
merged 1 commit into from
Aug 16, 2022

Conversation

asterite
Copy link
Member

…e case

This is a very simple change that leads to a pretty good performance boost.

Here's the benchmark:

require "benchmark"

string = "hEllO WoRlD!." * 10

Benchmark.ips do |x|
  x.report("downcase") do
    string.downcase
  end
  x.report("upcase") do
    string.upcase
  end
end

Before:

downcase   7.73M (129.38ns) (± 1.92%)  144B/op        fastest
  upcase   7.68M (130.15ns) (± 2.50%)  144B/op   1.01× slower

After:

downcase  42.20M ( 23.69ns) (± 3.39%)  144B/op        fastest
  upcase  41.84M ( 23.90ns) (± 3.83%)  144B/op   1.01× slower

@straight-shoota
Copy link
Member

Char#downcase/#upcase essentially does pretty much the same thing, just with a bit indirection through some method calls.

I suppose the performance gain comes from skipping the options.none? || options.ascii? test for every single byte, because the entire loop already is already guarded by that condition.

@straight-shoota straight-shoota added this to the 1.6.0 milestone Aug 15, 2022
@asterite
Copy link
Member Author

This interpreter spec is failing quite often:

    1) HTTP::Server handles Expect: 100-continue correctly when body is read
  
         server failed to start within 00:00:05 (Exception)

I guess the interpreter is slower and it takes more time than that? 🤔

@straight-shoota straight-shoota merged commit f160040 into master Aug 16, 2022
@straight-shoota straight-shoota deleted the opt/string-downcase-upcase branch August 16, 2022 19:13
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.

2 participants