Skip to content
This repository has been archived by the owner on Dec 10, 2018. It is now read-only.

Optionally use __slots__ for payload members #259

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

misakwa
Copy link

@misakwa misakwa commented Nov 18, 2016

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
Copy link
Author

misakwa commented Nov 18, 2016

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
Copy link
Contributor

hit9 commented Nov 21, 2016

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
Copy link
Author

misakwa commented Nov 22, 2016

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
Copy link
Author

misakwa commented Nov 22, 2016

This is now ready for review.

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

Successfully merging this pull request may close these issues.

2 participants