-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 PXD only Cython extension for the WebSocket reader to improve performance #9543
Conversation
There are not functional changes here. The goal is to be able to able to make it easier to build a Cython implementation for the WebsocketReader
There are not functional changes here. The goal is to be able to able to make it easier to build a Cython implementation for the WebsocketReader
There are not functional changes here. The goal is to be able to able to make it easier to build a Cython implementation for the WebsocketReader
There are not functional changes here. The goal is to be able to able to make it easier to build a Cython implementation for the WebsocketReader
CodSpeed Performance ReportMerging #9543 will improve performances by ×2.1Comparing Summary
Benchmarks breakdown
|
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 #9543 +/- ##
=======================================
Coverage 98.57% 98.58%
=======================================
Files 112 113 +1
Lines 35222 35251 +29
Branches 4184 4186 +2
=======================================
+ Hits 34720 34751 +31
+ Misses 339 338 -1
+ Partials 163 162 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This reverts commit b74b874.
We could probably get another 30% out of it if we stopped shipping around the data in a tuple since it has to be converted back to pyint. But thats probably not worth it |
Backport to 3.11: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 6ae2f05 on top of patchback/backports/3.11/6ae2f053e9989a759b140e42937e2138f80e502d/pr-9543 Backporting merged PR #9543 into master
🤖 @patchback |
… the WebSocket reader to improve performance (#9548)
The idea is to be able to use a PXD file to augment the python code with types so cython can build an optimized version +~100% of it as an extension without having to maintain a separate PYX file.
The reader has been moved to
reader_py.py
to ensureAIOHTTP_NO_EXTENSIONS
does not loadreader_c
. There was some missing coverage inreader.py
. Tests have been added to close the gap.I considered an alternate approach of adding a PYX version which would be a tiny bit faster, but the downside of having to maintain the same code twice and requiring significant Cython knowledge to work on the WebSocket reader lead me to a PXD only approach since only Cython types have to be maintained which has a much lower learning curve.
The cythonized version can be examined with the following:
make cythonize
cythonize -X language_level=3 -a -i aiohttp/_websocket/reader_c.py
aiohttp/_websocket/reader_c.html
Yellow lines in the html hint at python interaction. See https://cython.readthedocs.io/en/latest/src/tutorial/cython_tutorial.html https://pythonprogramming.net/introduction-and-basics-cython-tutorial/
Note that if you make changes to the
.pxd
file,cythonize
may not see them until youtouch aiohttp/_websocket/reader_c.py
and run the above again.