Cache IP Address parsing #1086
Merged
Cache IP Address parsing #1086
Conversation
This file contains 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
In _encode_host, the most expensive thing is converting the string to an ip address to normalize it. Its much more expensive than _idna_encode which we already cache.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1086 +/- ##
=======================================
Coverage 93.05% 93.06%
=======================================
Files 31 31
Lines 4408 4424 +16
Branches 364 370 +6
=======================================
+ Hits 4102 4117 +15
- Misses 280 281 +1
Partials 26 26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
psf-chronographer
bot
added
the
bot:chronographer:provided
Merged
bdraco
commented
Sep 2, 2024
|
Ran for a few hours, manual testing and LRU hit rate look good. Working as expected |
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.
What do these changes do?
In
_encode_host, the most expensive thing is converting the string to an ip address to normalize it when the URL has an ip address instead of a hostname. Its much more expensive than_idna_encodewhich we already cache.I always wondered the aiohttp benchmarking we did for Home Assistant performed worse when using 127.0.0.1 vs a hostname as it was confusing that that the dns lookup it would be faster, but its cached and avoids the ip address object creation for every iteration in yarl. I didn't look deep enough in the stack to look at yarl when we did that.
In hindsight, it should have been a bit more obvious since IP addresses are more common than names for Home Assistant because there is so much local network traffic and ips are usually discovered via mdns or some other method.
Are there changes in behavior for the user?
no
before
after
