Autoload Resolv instead of requiring it eagerly - #337
Open
tas50 wants to merge 1 commit into
Open
Conversation
net/http requires resolv at the top of the file, but the only thing it
uses from that library is two regexp constants, on one line:
case @address
when Resolv::IPv4::Regex, Resolv::IPv6::Regex
That line lives in the TLS branch of #connect, so resolv is not needed
until an HTTPS connection is opened, and not at all for plain HTTP or
for the many libraries that require net/http without connecting.
Switching to autoload matches what the very next line of the file
already does for OpenSSL, and it keeps Resolv resolvable for anything
downstream that relied on net/http defining it -- the constant still
works, the load just happens on first reference.
Measured on Ruby 4.0.6 (arm64-darwin), best of seven runs:
require 'net/http' files loaded
before 76.94 ms 33
after 70.54 ms 30
-6.40 ms -3 (8% faster)
Verified that requiring net/http no longer loads resolv, that
Resolv::IPv4::Regex still resolves afterwards and pulls the library in
on demand, and that a live HTTPS GET still succeeds.
Test suite: 201 tests, 0 failures, unchanged. The three added tests
cover all three properties; the first fails against the previous code.
Signed-off-by: Tim Smith <tsmith84@proton.me>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
net/httprequiresresolvat the top of the file, but the only thing it uses from that library is two regexp constants, on a single line:That line lives in the TLS branch of
#connect, soresolvisn't needed until an HTTPS connection is opened — and not at all for plain HTTP, or for the many libraries thatrequire 'net/http'to expose optional functionality and never connect.Fix
autoloadrather than a require inside#connect, for two reasons:Resolvstays resolvable, so anything downstream that relied onrequire 'net/http'defining it keeps working — the load just happens on first reference instead of eagerly. An inline require would have removed the constant from the post-require namespace.It also costs nothing per connection, which an inline
requireinconnectwould not.Measurements
Ruby 4.0.6 (arm64-darwin), best of seven runs:
require 'net/http'Verified directly:
Tests
bundle exec rake test: 201 tests, 0 failures, unchanged (204 with the additions).The three added tests cover that requiring net/http does not load resolv, that
Resolv::IPv4::Regexstill resolves afterwards, and that referencing it pulls the library in. The first fails against the previous code.The remaining net/http load cost is
socket(54 ms vianet/protocol) anduri(10 ms), both of which it genuinely needs — this was the only removable item.