-
Notifications
You must be signed in to change notification settings - Fork 158
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
Rework aiopg.sa.result module with Cython. #664
base: master
Are you sure you want to change the base?
Conversation
36bfe06
to
f8ab48a
Compare
This pull request fixes 1 alert when merging f8ab48a into c83e6a5 - view on LGTM.com fixed alerts:
|
Codecov Report
@@ Coverage Diff @@
## master #664 +/- ##
==========================================
+ Coverage 92.18% 92.35% +0.16%
==========================================
Files 10 9 -1
Lines 1037 837 -200
Branches 121 88 -33
==========================================
- Hits 956 773 -183
+ Misses 65 56 -9
+ Partials 16 8 -8 Continue to review full report at Codecov.
|
@asvetlov, couple of concrete questions:
|
f8ab48a
to
b6ac025
Compare
This pull request fixes 1 alert when merging b6ac025 into c83e6a5 - view on LGTM.com fixed alerts:
|
b6ac025
to
420ee85
Compare
This pull request fixes 1 alert when merging 420ee85 into c83e6a5 - view on LGTM.com fixed alerts:
|
420ee85
to
c46a973
Compare
This pull request fixes 1 alert when merging db95421 into c83e6a5 - view on LGTM.com fixed alerts:
|
db95421
to
509c53c
Compare
This pull request fixes 1 alert when merging 509c53c into 3fb3256 - view on LGTM.com fixed alerts:
|
|
I've noticed that classes
ResultProxy
andRowProxy
are bottlenecks in one of my projects and that other libraries like SQLAlchemy or Asyncpg implement similar classes in C for speed [1], [2].I decided to start experimenting with doing the same thing in Aiopg - I've done some simple optimizations using Cython and already got some positive results using this benchmark setup: https://gist.github.com/AndreiPashkin/dd6aa232f91deaa987b5ee0285ad9b7d
Results before:
Results after:
What do maintainers think about this? I it worth to continue? Maybe it's even worth to simply re-use fast
ResultProxy
implementation from SQLAlchemy?Are there changes in behavior for the user?
Theoretically everything should be backwards-compatible
Related issue number
N/A
Checklist
CHANGES
folder<issue_id>.<type>
(e.g.588.bugfix
)issue_id
change it to the pr id after creating the PR.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.Fix issue with non-ascii contents in doctest text files.