CookieJar improvements #7583
Comments
|
I profiled my program with yappi and found that As we can see, the preparation before filtering is very expensive. Lines 237 to 252 in 7ed2dd3 However, not all requests will have cookies in their jar, for example, the initial request, or, when the session is only used to request those URLs that never sent cookies (images, videos, files, etc). So I have another suggestion: test if there are any cookies in the jar before really doing anything. |
|
If it's an easy change, feel free to make a PR, it's easier for me to evaluate the code. |
I see |
|
It would be nice if we had a simple benchmark script to compare before and after changes for the cookie jar (probably the url dispatcher as well). The cookie jar and the url dispatcher tend to be the bottlenecks for large aiohttp installs so anything we can do to improve them will make things scale much better. |
|
This is a benchmarks repo, which I've not looked at yet, maybe if that is dusted off it can be used? |
|
It looks like those are mostly? end-to-end benchmarks. Since we already know where the bottlenecks are, I'd be more interested in something that adds 10000 cookies to the cookie jar and does timing on how long it takes to call For the url dispatcher add 5000 resources and see how much time it takes to dispatch to the to the last one in the list vs the first one in the list. |
See also aio-libs/aiohttp#7583 Signed-off-by: Rongrong <i@rong.moe>
…he jar is empty or all cookies have expired (#7822) **This is a backport of PR #7819 as merged into master (dfc3f89).** <!-- Thank you for your contribution! --> ## What do these changes do? The filtering itself and its preparation in `CookieJar.filter_cookies()` is expensive. Sometimes there are no cookies in the jar or all cookies have expired. Skip filtering and its preparation in this case. Because the empty check is much cheaper than `_do_expiration()`, I think it deserves to be duplicated before and after calling `_do_expiration()`. ```console $ python3.11 -m timeit -s 'from collections import defaultdict; d=defaultdict(foo="bar")' \ > 'if not d: pass' 50000000 loops, best of 5: 8.3 nsec per loop $ python3.11 -m timeit -s 'from collections import defaultdict; d=defaultdict()' \ > 'if not d: pass' 50000000 loops, best of 5: 8.74 nsec per loop $ python3.11 -m timeit -s 'from aiohttp import CookieJar; cj = CookieJar()' \ > 'cj._do_expiration()' 200000 loops, best of 5: 1.86 usec per loop ``` <!-- Please give a short brief about these changes. --> ## Are there changes in behavior for the user? No. <!-- Outline any notable behaviour for the end users. --> ## Related issue number #7583 (comment) <!-- Are there any issues opened that will be resolved by merging this change? --> ## Checklist - [x] I think the code is well written - [ ] Unit tests for the changes exist - [ ] Documentation reflects the changes - [x] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [x] Add a new news fragment into the `CHANGES` folder * name it `<issue_id>.<type>` for example (588.bugfix) * if you don't have an `issue_id` change it to the pr id after creating the pr * ensure type is one of the following: * `.feature`: Signifying a new feature. * `.bugfix`: Signifying a bug fix. * `.doc`: Signifying a documentation improvement. * `.removal`: Signifying a deprecation or removal of public API. * `.misc`: A ticket has been closed, but it is not of interest to users. * Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."
…` when the jar is empty or all cookies have expired (aio-libs#7822) **This is a backport of PR aio-libs#7819 as merged into master (dfc3f89).** <!-- Thank you for your contribution! --> The filtering itself and its preparation in `CookieJar.filter_cookies()` is expensive. Sometimes there are no cookies in the jar or all cookies have expired. Skip filtering and its preparation in this case. Because the empty check is much cheaper than `_do_expiration()`, I think it deserves to be duplicated before and after calling `_do_expiration()`. ```console $ python3.11 -m timeit -s 'from collections import defaultdict; d=defaultdict(foo="bar")' \ > 'if not d: pass' 50000000 loops, best of 5: 8.3 nsec per loop $ python3.11 -m timeit -s 'from collections import defaultdict; d=defaultdict()' \ > 'if not d: pass' 50000000 loops, best of 5: 8.74 nsec per loop $ python3.11 -m timeit -s 'from aiohttp import CookieJar; cj = CookieJar()' \ > 'cj._do_expiration()' 200000 loops, best of 5: 1.86 usec per loop ``` <!-- Please give a short brief about these changes. --> No. <!-- Outline any notable behaviour for the end users. --> aio-libs#7583 (comment) <!-- Are there any issues opened that will be resolved by merging this change? --> - [x] I think the code is well written - [ ] Unit tests for the changes exist - [ ] Documentation reflects the changes - [x] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [x] Add a new news fragment into the `CHANGES` folder * name it `<issue_id>.<type>` for example (588.bugfix) * if you don't have an `issue_id` change it to the pr id after creating the pr * ensure type is one of the following: * `.feature`: Signifying a new feature. * `.bugfix`: Signifying a bug fix. * `.doc`: Signifying a documentation improvement. * `.removal`: Signifying a deprecation or removal of public API. * `.misc`: A ticket has been closed, but it is not of interest to users. * Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."
See also aio-libs/aiohttp#7583 Signed-off-by: Rongrong <i@rong.moe>
While looking through #7577, we found a few details that could possibly be improved.
self._cookies[(domain, path)][name], therefore we could do domain-matching and path-matching on the keys, instead of testing every single cookie.aiohttp/aiohttp/cookiejar.py
Lines 141 to 142 in 4639b36
The text was updated successfully, but these errors were encountered: