Significantly speed up filter_cookies #9204
Conversation
noticed while working on #9203 TODO: show profiles
bdraco
added
backport-3.10
backport-3.11
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #9204 +/- ##
=======================================
Coverage 98.42% 98.42%
=======================================
Files 107 107
Lines 34654 34708 +54
Branches 4104 4113 +9
=======================================
+ Hits 34107 34161 +54
Misses 380 380
Partials 167 167
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
This doesn't look like it should really make a difference.. I feel like the extra nesting might hurt readability too. |
|
Will do a more detailed profile later. If its not significant, I'll close this as it won't be churn if its only small. |
|
This benchmark should work |
|
before % python3 filter_large_cookie_jar.py |
|
after filter_domain: 6.75009624985978 |
|
Based on that I'd say its not worth the churn |
|
Still need to investigate if there is anything we can do to improve the filter domain case but that seems like it won't be a small change. I'm not sure if it's still a problem though |
bdraco
changed the title
|
Did some more analysis and found out that most of the time is copying the cookie and making another Morsel object so I added a cache since most of the time we set once, ready many. With adding a cache filter_domain: 1.3521433752030134 |
|
Still in flight for a few more hours so will need to do some real world analysis to make sure the cache is effective as expected. Right now we spend more time in cookie.py/cookiejar.py than http_writer.py in most cases so it would be a really nice speed up if we can make filter_cookies faster |
… to cookies" This reverts commit f496936.
…speed_up_filter_cookies
|
The hit rate on the cache is near 100% at least on Home Assistant so it definitely seems worth caching before after On another system |
psf-chronographer
bot
added
the
bot:chronographer:provided
Backport to 3.10: 💚 backport PR created✅ Backport PR branch: Backported as #9243 🤖 @patchback |
(cherry picked from commit 34a626d)
Backport to 3.11: 💚 backport PR created✅ Backport PR branch: Backported as #9244 🤖 @patchback |
(cherry picked from commit 34a626d)
…ies (#9243) Co-authored-by: J. Nick Koston <nick@koston.org>
…ies (#9244) Co-authored-by: J. Nick Koston <nick@koston.org>
What do these changes do?
Significantly speed up filter_cookies by adding a cache for morsel construction. Previously every request had to create new Morsel objects. Its a common pattern to set a cookie once and read it many times. When testing with Home Assistant, the cache had nearly a 100% hit rate.
Benchmark #7790 (comment)
before
filter_domain: 6.765125583391637
after
filter_domain: 1.3521433752030134
Are there changes in behavior for the user?
no
Is it a substantial burden for the maintainers to support this?
no