Skip to content

Optionally use __slots__ for payload members - #259

Open
misakwa wants to merge 12 commits into
Thriftpy:developfrom
misakwa:fix/optimize-payload
Open

Optionally use __slots__ for payload members#259
misakwa wants to merge 12 commits into
Thriftpy:developfrom
misakwa:fix/optimize-payload

Conversation

@misakwa

@misakwa misakwa commented Nov 18, 2016

Copy link
Copy Markdown

Fixes #198

  • Add TSPayload for slot based structs
  • Add new optional use_slots parameter to allow generating code with slots
  • Cache slot based objects slightly differently to avoid cache poisoning

misakwa added 5 commits November 18, 2016 01:35
- Expose using slot based generated code to load entry points
- Add first tests
- TODO:
    - Fix possible cache poisoning when loading slot vs no slot structs
    - Add tests for above
    - Add pickling tests for slot based structs
@misakwa

misakwa commented Nov 18, 2016

Copy link
Copy Markdown
Author

There are still a few questions and some cleanup to be done, but I think this should get the conversation started.
/cc @dan-blanchard

- Replace class with a newer one with slot fields
- Implement subclass and instance checks
@hit9

hit9 commented Nov 21, 2016

Copy link
Copy Markdown
Contributor

Feature accepted. Update this issue later.

During the first creation of the loaded class with slots, a new class is
inserted into the inheritance chain to ensure that the slot fields are defined.

This is done because of the need to create an empty struct during the
parsing phase and fill it in later - slots require the fields to be known before hand.

All checks on the new replacement class will have it looking like the
original except an equality comparison between the replaced class and
its replacement.

>>> import thriftpy
>>> ab = thriftpy.load('addressbook.thrift', use_slots=True)
>>> ab_inst = ab.AddressBook()
>>> ab_inst.__class__ == ab.AddressBook # will return False
>>> # all other checks should work as expected
>>> isinstance(ab_inst, ab.AddressBook) # will return True
>>> issubclass(ab_inst.__class__, ab.AddressBook) # will return True

In order to get pickling to work as expected, a new extension type is
registered with copyreg (copy_reg for py2x) to avoid pickling errors.
@misakwa

misakwa commented Nov 22, 2016

Copy link
Copy Markdown
Author

I have updated the implementation to support all types of objects as well as the recursive case. The service args and results are now created with slots as well.

@misakwa
misakwa force-pushed the fix/optimize-payload branch from ba74089 to afd9acd Compare November 22, 2016 12:58
@misakwa
misakwa force-pushed the fix/optimize-payload branch from 7f3f6bd to 682e191 Compare November 22, 2016 13:37
@misakwa

misakwa commented Nov 22, 2016

Copy link
Copy Markdown
Author

This is now ready for review.

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.

2 participants