Skip to content
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

Merged
merged 9 commits into from
Apr 13, 2020

Conversation

syrusakbary
Copy link
Member

@syrusakbary syrusakbary commented Mar 14, 2020

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:

py.test --benchmark-only graphene/types/tests/test_objecttype.py

Before this patch (385,925 operations / second)

-------------------------------------------------------- benchmark: 1 tests -------------------------------------------------------
Name (time in us)                          Min      Max    Mean  StdDev  Median     IQR  Outliers  OPS (Kops/s)  Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------------------
test_objecttype_container_benchmark     1.7730  61.1060  2.5912  1.3161  2.4700  0.2210  911;2832      385.9257   46230           1
-----------------------------------------------------------------------------------------------------------------------------------

After this patch (1,270,600 operations / second) - More than 3x speedup

---------------------------------------------------------------- benchmark: 1 tests ----------------------------------------------------------------
Name (time in ns)                            Min           Max      Mean      StdDev    Median       IQR  Outliers  OPS (Mops/s)  Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------
test_objecttype_container_benchmark     594.9987  103,029.9973  787.0462  1,168.2685  708.9984  144.9953  752;3399        1.2706  119389           1
----------------------------------------------------------------------------------------------------------------------------------------------------

Breaking change

This PR introduces a breaking change, private attributes (aka: _* fields) would not be automatically modified on instantiation.

class MyObject(ObjectType):
    field = String()

MyObject(_private_field=1)  # <--- This will not work, unless you create a custom __init__

Note: DO NOT MERGE YET

Copy link
Member

@jkimbo jkimbo left a 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
Copy link
Member

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?

Copy link
Member Author

@syrusakbary syrusakbary Mar 16, 2020

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

Copy link
Member

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/

Copy link
Member Author

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 :)

graphene/types/objecttype.py Show resolved Hide resolved
@jkimbo
Copy link
Member

jkimbo commented Mar 15, 2020

Also the breaking change could be quite a big one. Can you give an example of a custom __init__ method that would let you define private attributes?

@syrusakbary
Copy link
Member Author

Also the breaking change could be quite a big one. Can you give an example of a custom init method that would let you define private attributes?

You can overwrite your __init__ allowing the extra private fields. There is a hacky solution we can get to skip the breaking change, but I think a better long-term solution is just removing it's support and focusing on documentation.

@syrusakbary
Copy link
Member Author

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!

@syrusakbary syrusakbary merged commit 49fcf9f into master Apr 13, 2020
@radekwlsk radekwlsk mentioned this pull request Jul 29, 2020
8 tasks
@jkimbo jkimbo deleted the features/fast-object-creation branch August 26, 2020 15:08
@wodCZ
Copy link

wodCZ commented Jan 6, 2021

Hi,
just a thought - could this change be backported to v2? From a quick glance it looks like the changed code is almost identical to what's in v2.
I'm not sure whether there are some other changes elsewhere on which this depends on.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants