-
Notifications
You must be signed in to change notification settings - Fork 827
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
Allow fast ObjectType creation based on dataclasses #1157
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very cool! Just a couple of small questions.
@@ -0,0 +1,1260 @@ | |||
# This is a polyfill for dataclasses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not add a dependency on the dataclasses
lib?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dataclasses are not available until Python 3.7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I mean the dataclasses library on PyPi: https://pypi.org/project/dataclasses/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to customize it a bit, so I think is better to ship it with the lib for now and remove it later :)
Also the breaking change could be quite a big one. Can you give an example of a custom |
You can overwrite your |
After thinking on it a bit more, I think we can test the water by merging the PR and seeing if anyone has significant issues over the breaking change. So we should be good to merge! YAY! |
Hi, Thanks! EDIT: just tested applying the change against v2 and it works fine. 307 kops/s -> 1454 kops/s. The change doesn't have a significant effect on the response time in my case though. |
This PR optimizes the ObjectType initialization by an order of magnitude by leveraging the same strategy that dataclasses introduced (dynamic creation of optimal
__init__
functions based on eval).Benchmarks
This are the benchmarking numbers after running:
Before this patch (385,925 operations / second)
After this patch (1,270,600 operations / second) - More than 3x speedup
Breaking change
This PR introduces a breaking change, private attributes (aka:
_*
fields) would not be automatically modified on instantiation.Note: DO NOT MERGE YET