Skip to content

Conversation

@bdraco
Copy link
Member

@bdraco bdraco commented Sep 2, 2024

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_encode which 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.

Screenshot 2024-09-02 at 1 16 25 PM

Are there changes in behavior for the user?

no

before
_encode_host

after
encode_host_after

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
Copy link

codecov bot commented Sep 2, 2024

Codecov Report

Attention: Patch coverage is 97.29730% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.06%. Comparing base (d88e9ff) to head (b13f6a2).
Report is 328 commits behind head on master.

Files with missing lines Patch % Lines
yarl/_url.py 96.42% 1 Missing ⚠️
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           
Flag Coverage Δ
CI-GHA 93.01% <97.29%> (+<0.01%) ⬆️
MyPy 26.97% <86.48%> (+0.57%) ⬆️
OS-Linux 99.33% <100.00%> (+<0.01%) ⬆️
OS-Windows 99.43% <100.00%> (+<0.01%) ⬆️
OS-macOS 99.01% <100.00%> (+<0.01%) ⬆️
Py-3.10.11 98.93% <100.00%> (+<0.01%) ⬆️
Py-3.10.14 99.19% <100.00%> (+<0.01%) ⬆️
Py-3.11.9 99.19% <100.00%> (+<0.01%) ⬆️
Py-3.12.5 99.19% <100.00%> (+<0.01%) ⬆️
Py-3.13.0-rc.1 99.19% <100.00%> (+<0.01%) ⬆️
Py-3.8.10 98.87% <100.00%> (+<0.01%) ⬆️
Py-3.8.18 99.13% <100.00%> (+<0.01%) ⬆️
Py-3.9.13 98.87% <100.00%> (+<0.01%) ⬆️
Py-3.9.19 99.13% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.11 99.19% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.16 99.22% <100.00%> (+<0.01%) ⬆️
VM-macos-latest 99.01% <100.00%> (+<0.01%) ⬆️
VM-ubuntu-latest 99.33% <100.00%> (+<0.01%) ⬆️
VM-windows-latest 99.43% <100.00%> (+<0.01%) ⬆️
pytest 99.33% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Sep 2, 2024
@bdraco bdraco mentioned this pull request Sep 2, 2024
@bdraco bdraco marked this pull request as ready for review September 3, 2024 00:16
@bdraco
Copy link
Member Author

bdraco commented Sep 3, 2024

Ran for a few hours, manual testing and LRU hit rate look good. Working as expected

@bdraco bdraco changed the title Cache IP address parsing Cache IP Address parsing Sep 3, 2024
@bdraco bdraco merged commit f55fa97 into master Sep 3, 2024
@bdraco bdraco deleted the ip_parse_caching branch September 3, 2024 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants