Replace Regexp in for headers for perf#140
Conversation
|
Side topic: I've found a few additional areas where we might get some easy wins if you're up to spot check me on some more PRs later @byroot. |
|
Sure. But note that I'm not the maintainer, I can't merge your changes. https://github.com/ruby/ruby/blob/master/doc/maintainers.md#libnethttprb-libnethttpsrb |
|
|
||
| def capitalize(name) | ||
| name.to_s.split(/-/).map {|s| s.capitalize }.join('-') | ||
| name.to_s.split('-'.freeze).map {|s| s.capitalize }.join('-'.freeze) |
There was a problem hiding this comment.
Not what you are asking for, but another implementation work trying could be:
def capitalize(name)
name.to_s.gsub(/(\A|(?<=[\^\-]))([a-z])/) do |c|
c.upcase!
c
end
endThere was a problem hiding this comment.
I did some benchmarking, and using the regex seemed to be slower.
technicalpickles
left a comment
There was a problem hiding this comment.
I've been doing some benchmarking in this area too 😅
Using String with split is definitely faster than Regex. Freezing the string also can help, which I made a PR for over at #144
Some benchmarking I put together for this (including @byroot's gsub implementation): https://gist.github.com/technicalpickles/231940b1e64da1762df4a2e8fc53e1d8
|
|
||
| def capitalize(name) | ||
| name.to_s.split(/-/).map {|s| s.capitalize }.join('-') | ||
| name.to_s.split('-'.freeze).map {|s| s.capitalize }.join('-'.freeze) |
There was a problem hiding this comment.
I did some benchmarking, and using the regex seemed to be slower.
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
96e5305 to
826e008
Compare
|
Sorry to late response. This PR helps to work with https://bugs.ruby-lang.org/issues/20205 in the future. |
I had noticed that
Net::HTTPis splitting on aRegexpin the headers file, so wanted to put in a quick patch on that. Here's some of the performance data to back up this change:We could additionally hoist and freeze the header delimiter for an additional gain, but I wanted to keep this PR minimal and a constant being hoisted would introduce a potential public API surface for users to use.
My reasoning for doing this is that we have memory profiles from running Capybara tests that have this as a hot path in terms of memory and object allocations.