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

pyclass: refactor tp_new / tp_dealloc / remove PyCellInner #1657

Merged
merged 2 commits into from
Jun 23, 2021

Conversation

davidhewitt
Copy link
Member

@davidhewitt davidhewitt commented Jun 5, 2021

This PR is a bunch of cleanups to PyCell and PyClassInitializer code for the slot implementations of tp_new, tp_alloc, tp_dealloc and tp_free:

  • tp_alloc and tp_free are only implemented for freelist classes, where they provide custom memory allocation / release
  • tp_new now does the same thing for all #[pyclass] (call super tp_new and initialize the PyCell contents)
  • tp_dealloc does the reverse of tp_new (drop the PyCell contents and then call super tp_dealloc)

This work seems necessary to me as a precursor to #1647, #1637, #1344 and #991.

While doing this I also found a nasty bug that we don't call tp_dealloc of inherited native types, which means that dict subclasses never decrease reference counts of all their contents on drop (for example).

I added a benchmark for class instantiation. Sadly these cleanups don't seem to have meaningfully changed the benchmark. I did notice that we're a lot faster than the numbers reported in #661, so there must have been progress over time.

@davidhewitt davidhewitt force-pushed the pycell-init branch 5 times, most recently from 0b3cf7a to 7a19c8b Compare June 5, 2021 16:38
@davidhewitt
Copy link
Member Author

Oooh nice, a segfault purely on windows 32-bit 👀

@davidhewitt
Copy link
Member Author

Turns out I forgot #[repr(C)] from PartiallyInitializedPyCell 😬

@davidhewitt davidhewitt mentioned this pull request Jun 6, 2021
7 tasks
@davidhewitt
Copy link
Member Author

If nobody reviews of raises any concerns by the weekend I plan to merge this.

@davidhewitt davidhewitt merged commit 2cba18f into PyO3:main Jun 23, 2021
@davidhewitt davidhewitt deleted the pycell-init branch June 23, 2021 07:22
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.

1 participant