Optimize URI#hostname and URI#hostname= #12
Conversation
lib/uri/generic.rb
Outdated
| @@ -659,7 +659,7 @@ def hostname | |||
| # it is wrapped with brackets. | |||
| # | |||
| def hostname=(v) | |||
| v = "[#{v}]" if /\A\[.*\]\z/ !~ v && /:/ =~ v | |||
| v = "[#{v}]" if v&.index(':') && !v.start_with?('[') && !v.end_with?(']') | |||
There was a problem hiding this comment.
/\A\[.*\]\z/ !~ v and !v.start_with?('[') && !v.end_with?(']') differ.
It should be !(v.start_with?('[') && v.end_with?(']')).
There was a problem hiding this comment.
What an overlook, shame on me, this is 101. I will amend the fix. Also, I think I should start with start_with in the statement because index call is more costly and it is more likely to not match in the future as the world progresses towards IPv6.
lzap
force-pushed
the
faster-hostname-17219
branch
from
b3a035f to
36c3fd7
Compare
|
Amended the change with force-push. |
|
Could you please check if there are tests for those methods? |
Yes, I can actually amend one more change - an extra edge case and a case when square brackets are already present. Lemme do that. |
lzap
force-pushed
the
faster-hostname-17219
branch
from
36c3fd7 to
599d2b9
Compare
|
@lzap It wasn't me who meant |
|
Oh I am sorry, fixed! |
|
Sorry for the delay, this looks good, will merge shortly. |
Hello,
URI#hostname extends URI#host with IPv6 support. In URI, IPv6 address must have square brackets (e.g. http://[2001:db8::1]), URI#hostname strips these characters out while URI#hostname= adds them if missing. There are three regular expressions to perform these tasks which can dramatically slow down performance. I am attaching a two-line patch and here is a benchmark: https://gist.github.com/lzap/24cbecb47daf29111350e41a24250922
In short, URI#hostname is 86-89% and hostname= is 55-154% faster according to my measurements.
For the record, I created a Ruby MRI RM ticket https://bugs.ruby-lang.org/issues/17219 as I had no idea URI is a separate gem hosted on github.