fix: avoid some allocations of strings #387
Conversation
|
🍏 @dentarg |
| @normalized_host.force_encoding(Encoding::UTF_8) if @normalized_host | ||
| unless @normalized_host.empty? | ||
| @normalized_host.force_encoding(Encoding::UTF_8) | ||
| end |
There was a problem hiding this comment.
Is it guaranteed that @normalized_host will always respond to #empty?. The previous code guarded against nil (Maybe it wasn't necessary or the test coverage we have isn't sufficient, I don't know)
There was a problem hiding this comment.
Ah...ok, but @normalized_host can be empty string, so it is proper way?
if @normalized_host && !@normalized_host.empty?
@normalized_host.force_encoding(Encoding::UTF_8)
end
There was a problem hiding this comment.
What if @normalized_host is an empty string but not UTF-8 encoded..?
There was a problem hiding this comment.
I think that it will not be have matter because url without host is wrong url
🤔
it is may be have sense to add condition on emptiness at this line:
addressable/lib/addressable/uri.rb
Line 1246 in 0f468ea
|
🆙 |
|
🤔 |
|
Merged #383 before this one as requested. |
|
@dentarg Any updates on this? |
|
pinging @dentarg @sporkmonger |
Hello!
In this commit I suggest some optimizations for avoid unnecessary strings allocations
Before (our loop of mail delivery):
After:
no any conversions from nil to empty string