-
-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a cache for building encoded URLs #1432
Conversation
We currently have a cache for parsing URLs, but we did not have one for building URLs because URL.build accepts a dict for the query which is not hashable. Since web app tend to see the same 80% of URLs over and over, its advantageous to cache building the URL object as aiohttp has to make them on every web request.
CodSpeed Performance ReportMerging #1432 will improve performances by 53.86%Comparing Summary
Benchmarks breakdown
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1432 +/- ##
==========================================
+ Coverage 96.10% 96.11% +0.01%
==========================================
Files 31 31
Lines 5855 5873 +18
Branches 348 349 +1
==========================================
+ Hits 5627 5645 +18
Misses 202 202
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. |
The hit rate for pre-encoded in production is great. Not so much for unencoded ... might not make sense to cache building unencoded |
We still end up losing all the cache if they call
Because edit: addressed in #1434 |
2024-11-29 09:05:09.163 CRITICAL (SyncWorker_9) [homeassistant.components.profiler] Cache stats for lru_cache <function build_pre_encoded_url at 0x7f827e1e1260> at /usr/local/lib/python3.13/site-packages/yarl/_url.py: CacheInfo(hits=187, misses=15, maxsize=128, currsize=15) |
Since
aiohttp
web_request
tends to see the sameURL
s over and over, its advantageous to cache building theURL
objects. We currently have a cache for parsing URLs, but we did not have one for building URLs.We didn't have a cache on
URL.build
becauseURL.build
accepts a dict for the query which is not hashable. This is solved by constructing the query string before the cache is used. Note, that it may be better to avoid caching URLs with a query string in future but it didn't seem make enough difference in testing that it was worth the additional complexity.Since
aiohttp
rarelyURL.build
withencoded=False
we do not cache unencoded builds as the hit rate was < 50% in production testing. The hit rate for https://github.com/aio-libs/aiohttp/blob/1fa237ffc9e7aa70cbabb68ab64d6fe03255cbc0/aiohttp/web_request.py#L429 is nearly perfectAdditionally, and similar to #1434 every time the new
URL
object was created it would have a newself._cache
, but caching the build,self._cache
will already be initialized.