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

Create a metaclass for each supplemental data type #972

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

oremanj
Copy link
Contributor

@oremanj oremanj commented Mar 14, 2025

This allows many metaclass customizations without incurring the performance cost of allowing arbitrary metaclasses. As a side benefit, we get nontrivial construction and destruction of supplement types, and can check whether a particular nanobind type uses a particular supplement type. There are no additional per-instance performance or storage costs, no additional per-type storage costs, and negligible per-type performance costs.

The information about a particular supplement type is stored in a new nb::detail::supplement_data structure. An inline variable template is used to automatically create one of these in static data for each supplement type used in a particular extension module. Instances of nb_meta (i.e., metaclass types) gain a pointer to their supplement_data, which is null for the default metaclass nb_type_0. The non-default metaclass types nb_type_xyz are now stored in a nb_type_map_slow rather than a Python dictionary, and (except on free-threaded builds where everything is immortal) they are properly reference-counted so that they will be destroyed once all their instances are. This avoids leak warnings when assigning nanobind functions as their attributes.

Size cost:

  • On my laptop this adds 781 bytes to a release build of libnanobind-static.a.
  • One pointer per nanobind metatype, including the default one.
  • One nb::detail::supplement_data structure (five pointers) stored in static data per supplement type used in an extension module.

I originally added some caching in static data to limit lookups by supplement type to one per supplement type per extension module. I removed this because it was a bit complex and I don't think it's necessary (we already do two type_map_slow lookups per nb_type_new, avoiding a third doesn't save all that much) but it would be easy to re-add. See commit 34739e3. The additional cost from that caching would be 104 bytes of code and two more pointers in supplement_data.

@oremanj oremanj force-pushed the supplement-via-metaclass branch from 043b6ad to fe4c30e Compare March 14, 2025 17:54
@oremanj oremanj force-pushed the supplement-via-metaclass branch from fe4c30e to 0a92aef Compare March 14, 2025 17:56
@oremanj
Copy link
Contributor Author

oremanj commented Mar 14, 2025

The tests aren't running because the tests for a previous iteration hung. I don't have permission to stop the previous run https://github.com/wjakob/nanobind/actions/runs/13862493485. Maybe worth setting a timeout in github actions?

@oremanj oremanj marked this pull request as draft March 15, 2025 02:20
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