aiopg
aiopg copied to clipboard
Rework aiopg.sa.result module with Cython.
I've noticed that classes ResultProxy
and RowProxy
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:
simple [1.3329865639971104, 1.3546994640055345, 1.349644674002775]
json [1.6616086649883073, 2.042356903999462, 2.0650724380102474]
Results after:
simple [0.5680085269996198, 0.5687800829909975, 0.5716462380078156]
json [0.8984643409930868, 0.8978340939938789, 0.8937820449937135]
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
- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [ ] Documentation reflects the changes
- [ ] Add a new news fragment into the
CHANGES
folder- name it
<issue_id>.<type>
(e.g.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.
- name it
This pull request fixes 1 alert when merging f8ab48abffc228c3b69979ff00064245f7504ae8 into c83e6a5a93ceeb71fc08aff459e7d67aaf1fefa3 - view on LGTM.com
fixed alerts:
- 1 for Comparison using is when operands support __eq__
Codecov Report
Merging #664 into master will increase coverage by
0.16%
. The diff coverage isn/a
.
@@ 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.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update c83e6a5...509c53c. Read the comment docs.
@asvetlov, couple of concrete questions:
- Is ability to index by SQLAlchemy columns really needed? Can it be dropped?
- Can lazy processing of row values with processors be replaced with eager processing?
This pull request fixes 1 alert when merging b6ac0257523ea6d9add136ba9c2a75ac736b8fa7 into c83e6a5a93ceeb71fc08aff459e7d67aaf1fefa3 - view on LGTM.com
fixed alerts:
- 1 for Comparison using is when operands support __eq__
This pull request fixes 1 alert when merging 420ee855134e3bf336940807237d841aa64356cb into c83e6a5a93ceeb71fc08aff459e7d67aaf1fefa3 - view on LGTM.com
fixed alerts:
- 1 for Comparison using is when operands support __eq__
This pull request fixes 1 alert when merging db954215a8b4e0de12785c198dc5aef41d707106 into c83e6a5a93ceeb71fc08aff459e7d67aaf1fefa3 - view on LGTM.com
fixed alerts:
- 1 for Comparison using is when operands support __eq__
This pull request fixes 1 alert when merging 509c53cbea8aeb3bc85bb4f0efca8570561c5119 into 3fb3256319de766384b4867c7cc6710397bd1a8c - view on LGTM.com
fixed alerts:
- 1 for Comparison using is when operands support __eq__
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.