Skip to content

StructMeta / ABC Question - Docs #894

Closed

Description

@matterfield-sam

Question

Hi all,

I have been working on trying to use the newly-exposed functionality for building metaclasses using StructMeta. Thanks to everyone involved for making this possible!

Very briefly, I want to create an abstract Struct subclass which requires that a few methods be implemented, as in the attached minimal example. I suspect that my issue here is operator error, and more specifically a lack of understanding of the metaclass -> class -> instance hierarchy.

I created a small script to demonstrate my issue:
abstract_issue.py

I based my implementation on the suggestion here.

In brief, the inheritance hierarchy seems to work fine, right up until I ask if an instance of ConcreteIntStruct is a subclass of IntegerStructBase, or if the type ConcreteIntStruct is a subclass of IntegerStructBase. I suspect my issue lies in not understanding the correct way to define abstract methods on a Struct using StructMeta and ABCMeta. The captured stdout of my minimal script is at the bottom.

If this is the simple operator error it feels like, I would be happy to clean up my example into a running script and write a paragraph to expand this section in the docs to make myself useful. If this is something more substantial, I am happy to help however possible.

The attached script produces the following output:

Msgspec ABC Issue Repro
Python version info:
3.13.7 (main, Aug 14 2025, 11:12:11) [Clang 17.0.0 (clang-1700.0.13.3)]
----------
Msgspec version info:
Name: msgspec
Version: 0.19.0+12.g651231e
----------
ConcreteIntStruct(val=1)
4
ConcreteIntStruct(val=4)
b'\x81\xa3val\x01'
ConcreteIntStruct(val=1)
Traceback (most recent call last):
  File "/~path~", line 82, in <module>
    main()
    ~~~~^^
  File "~path~", line 77, in main
    print(issubclass(ConcreteIntStruct, IntegerStructBase))  # Expect: True
          ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen abc>", line 123, in __subclasscheck__
AttributeError: type object 'IntegerStructBase' has no attribute '_abc_impl'

Activity

  1. ofek commented on Oct 26, 2025

    @ofek
    Member

    Thanks a lot for both trying out the functionality and the detailed description of the issue! I can reproduce, and it appears like StructMeta doesn't call the next __new__ in the MRO. I consider this to be a release blocker.

    I'm unfamiliar with CPython internals and am not really sure the proper way to go about this but I will do some digging this weekend. For now, I think you can flip the order of inheritance and it might work.

    cc @apoorvkh

  2. matterfield-sam commented on Oct 26, 2025

    @matterfield-sam
    Author

    Thanks for the heads up on flipping the inheritance order. In my minimal example, I can confirm that works as expected. I will try it in my larger implementation tomorrow to see if the same holds up (especially when the inherited struct type becomes a field in other structs).

    I also think that StructMeta isn't calling __new__ - I tried to force it in my metaclass, but I'm entirely unfamiliar with the CPython machinery going on in the background, and so just ended up breaking things. I'll at least try get marginally conversant in how MRO heirarchy is working.

  3. provinzkraut commented on Oct 26, 2025

    @provinzkraut
    Member

    Thanks a lot for both trying out the functionality and the detailed description of the issue! I can reproduce, and it appears like StructMeta doesn't call the next __new__ in the MRO. I consider this to be a release blocker.

    It's been a while but iirc Struct not calling __new__, as well as there not being an exposed metaclass for Structs were a performance optimisation, and part of the reason why Structs are faster to instantiate than regular Python classes.

    Not exposing the metaclass was a relatively simple way to ensure all the instantiation dance was fully under msgspec's control, so issues like these could be prevented.

    The same goes for dataclass / (c)attrs support by the way, where msgspec also takes a shortcut in creating instances of them, that need special accommodations to make use of some of their native features (see for example #494 or #874).

    IMO this raises a broader question regarding msgspec's design; Should convention be sacrificed in favour of performance?

    A naive implementation would not have these issues, but it would also be less performant 🤷‍♀️

  4. ofek commented on Oct 26, 2025

    @ofek
    Member

    I didn't even consider that, thanks for explaining! I'm going to benchmark today but do you think the previous PR that exposed StructMeta introduced a performance regression?

    Should convention be sacrificed in favour of performance?

    Yes, most definitely.

  5. provinzkraut commented on Oct 26, 2025

    @provinzkraut
    Member

    I didn't even consider that, thanks for explaining! I'm going to benchmark today but do you think the previous PR that exposed StructMeta introduced a performance regression?

    I didn't take a close look at it yet, but it's conceivable that the changes from Py_TYPE(base) != &StructMetaType to !PyType_IsSubtype(Py_TYPE(base), &StructMetaType) have introduced somewhat of a regression, if these changes are in a hot path somewhere.

    Would probably good to verify that. I think in addition to #885, it might also be good to add some tests for general performance regressions?

  6. apoorvkh commented on Oct 26, 2025

    @apoorvkh
    Contributor

    Thanks for opening this issue. I am also unfamiliar with Python, so likely can't help with adding the call to __new__.

    However, I did run the python benchmarks/bench_struct.py benchmark. I can't find any significant difference in performance between the StructMeta commit (546f21d) and the prior commit (7acceff). I think this implies that simply exposing StructMeta shouldn't induce a "regression" in performance.

    Please feel free to benchmark further, as I think actually using a mixed metaclass (e.g. metaclass=IntegerStructMeta) may have different performance, as would a call to __new__. (I tried to quickly benchmark the former using abstract_issue.py above but am having difficulty measuring the difference with statistical significance. This needs a more careful test.) But anyway, maybe we can add a condition to only call __new__ when the metaclass is "mixed". Thus, any performance impact would continue to only affect this new explicit StructMeta case, rather than creating a regression.

  7. matterfield-sam commented on Oct 26, 2025

    @matterfield-sam
    Author

    Update: While I was writing this, @apoorvkh posted his comment above. The below just echoes what he saw.

    I took a very quick glance through the core code (I can't pretend to be anything approaching an expert here) but it looked like PyTypeIsSubType(**, &StructMetaType) is in several things that I'd expect to be hot paths - mainly struct allocation & encoding, but I will certainly defer to folks in the thread who are more qualified.

    To that end, I modified bench_structs.py and bench_encodings.py with the goal of trying to see if there was a noticeable performance regressing associated with allowing a more complex MRO in allocation and encoding. I tried the "ABC-first" and "StructMeta-first" orders, so ABCStruct(ABCMeta, StructMeta) and StructABC(StructMeta, ABCMeta) - the goal was to see if allowing ABCMeta to perform all the mcls setup before calling StructMeta (and the reverse). I just dropped the structure from the minimal example in my first comment into the benchmarks and flipped the order of the inheritance to see.

    I am not seeing a real difference - although I'm not doing a real statistical analysis to prove it. I suspect my basic methodology is at play and a more sophisticated test is required in order to see the performance regression impacts we are looking for.

    Here's what I see:

    bench_structs.py: Allocation & Equality
    Define 500 classes, create 1,000 new types, 10,000 instances, 10,000 compares

    +----------------------+-------------+-------------+---------------+------------+
    |                      | import (μs) | create (μs) | equality (μs) | order (μs) |
    +======================+=============+=============+===============+============+
    | **msgspec**          | 5.93        | 0.04        | 0.01          | 0.02       |
    +----------------------+-------------+-------------+---------------+------------+
    | **abc_structmeta**   | 25.04       | 0.05        | 0.01          | 0.02       |
    +----------------------+-------------+-------------+---------------+------------+
    | **structmeta_abc**   | 22.07       | 0.05        | 0.01          | 0.02       |
    +----------------------+-------------+-------------+---------------+------------+
    | **standard classes** | 5.78        | 0.19        | 0.04          | 0.09       |
    +----------------------+-------------+-------------+---------------+------------+
    | **dataclasses**      | 208.25      | 0.20        | 0.04          | 0.08       |
    +----------------------+-------------+-------------+---------------+------------+
    

    bench_encodings.py: Encode/Decode Perf, n=1000

    +-------------------------+-------------+------+-------------+------+------------+------+
    |                         | encode (μs) | vs.  | decode (μs) | vs.  | total (μs) | vs.  |
    +=========================+=============+======+=============+======+============+======+
    | StructMeta first struct | 67.7        | 1.00 | 185.3       | 1.00 | 253.0      | 1.00 |
    +-------------------------+-------------+------+-------------+------+------------+------+
    | msgspec structs         | 69.1        | 1.02 | 186.4       | 1.01 | 255.5      | 1.01 |
    +-------------------------+-------------+------+-------------+------+------------+------+
    | ABC first struct        | 70.6        | 1.04 | 185.2       | 1.00 | 255.8      | 1.01 |
    +-------------------------+-------------+------+-------------+------+------------+------+
    | orjson                  | 94.2        | 1.39 | 244.5       | 1.32 | 338.7      | 1.34 |
    +-------------------------+-------------+------+-------------+------+------------+------+
    | msgspec                 | 82.2        | 1.21 | 256.6       | 1.38 | 338.8      | 1.34 |
    +-------------------------+-------------+------+-------------+------+------------+------+
    | ujson                   | 342.5       | 5.06 | 414.2       | 2.24 | 756.7      | 2.99 |
    +-------------------------+-------------+------+-------------+------+------------+------+
    | json                    | 460.0       | 6.80 | 467.2       | 2.52 | 927.2      | 3.67 |
    +-------------------------+-------------+------+-------------+------+------------+------+
    
  8. provinzkraut commented on Oct 26, 2025

    @provinzkraut
    Member

    But anyway, maybe we can add a condition to only call __new__ when the metaclass is "mixed". Thus, any performance impact would continue to only affect this new explicit StructMeta case, rather than creating a regression.

    That sounds like a good solution in any case! The information about whether the class is "mixed" can also easily be stored on the struct metadata, so performance overhead of that check should be negligible.


    As for the performance regressions of the aforementioned change, I am seeing some degradation during struct encodes (modified bench_validations.py to run each test 100 times, with a few warmup runs):

    546f21d (exposed StructMeta)

    +---------+-------------+-----+-------------+-----+------------+-----+--------------+-----+
    |         | encode (μs) | vs. | decode (μs) | vs. | total (μs) | vs. | memory (MiB) | vs. |
    +=========+=============+=====+=============+=====+============+=====+==============+=====+
    | msgspec | 23223.5     | 1.0 | 61033.8     | 1.0 | 84257.3    | 1.0 | 34.2         | 1.0 |
    +---------+-------------+-----+-------------+-----+------------+-----+--------------+-----+
    

    vs 7acceff (commit before)

    +---------+-------------+-----+-------------+-----+------------+-----+--------------+-----+
    |         | encode (μs) | vs. | decode (μs) | vs. | total (μs) | vs. | memory (MiB) | vs. |
    +=========+=============+=====+=============+=====+============+=====+==============+=====+
    | msgspec | 22731.7     | 1.0 | 60220.0     | 1.0 | 82951.7    | 1.0 | 34.0         | 1.0 |
    +---------+-------------+-----+-------------+-----+------------+-----+--------------+-----+
    
  9. apoorvkh commented on Oct 26, 2025

    @apoorvkh
    Contributor

    This is great, thanks everyone for helping benchmark!

    Just helping put the above numbers into perspective: I guess this is an alleged 1–2% performance degradation to encode and decode, supposing this result is statistically significant. Either way, I personally believe this degree of change is acceptable. Users will care most that msgspec continues to be generally 6x faster than the second best library (mashumaro) and so forth. I think the difference of 5–10 μs per encode or decode invocation would be imperceptible (even at scale). On the other hand, there seemed to be a good deal of demand for the StructMeta feature :)

  10. provinzkraut commented on Oct 26, 2025

    @provinzkraut
    Member

    Just helping put the above numbers into perspective: I guess this is an alleged 1–2% performance degradation to encode and decode, supposing this result is statistically significant. Either way, I personally believe this degree of change is acceptable. Users will care most that msgspec continues to be generally 6x faster than the second best library (mashumaro) and so forth. I think the difference of 5–10 μs per encode or decode invocation would be imperceptible (even at scale)

    As an individual change, I'd agree that this is fine. In general though, small changes like this tend to accumulate over time, and then it might start to not be fine.

    IMO to make a good decision here, it must be known:

    1. Whether the degradation can be confirmed with some more confidence
    2. How much overhead the additional changes required to make this feature work properly add

    Then it should be possible to judge if the compromise is a reasonable one.

  11. ofek commented on Oct 26, 2025

    @ofek
    Member

    I think the fast-path check recommendation mentioned in one of the comments is a good idea and is what should happen.

  12. ncoghlan commented on Oct 28, 2025

    @ncoghlan
    Contributor

    @ofek Would it be feasible to postpone this change until the release after the one adding Python 3.14 support?

    Being able to inherit from StructMeta is a nice enhancement, but it's something the library hasn't supported in the past, and fully supporting cooperative inheritance in C implemented metaclasses isn't straightforward (as you've found). Even beyond the test cases already added in #890, a more interoperable StructMeta should also be tested for compatibility with __init_subclass__ and with instance descriptors that make use of __set_name__ without being Struct field definitions.

    Postponing the feature would mean we'd have the time to consider the best path forward for this enhancement without any concern about delaying the publication of Python 3.14 support to PyPI.

  13. ncoghlan commented on Oct 30, 2025

    @ncoghlan
    Contributor

    One potential way to approach that would be to keep most of the changes to make StructMeta more accessible, but turn the flag that allows subclassing back off for now.

  14. ofek commented on Oct 30, 2025

    @ofek
    Member

    I'll take a look at that this evening then, thanks! Would you mind linking the part you're referencing?

  15. ncoghlan commented on Oct 30, 2025

    @ncoghlan
    Contributor

    Removing Py_TPFLAGS_BASETYPE from tp_flags again in the declaration of StructMeta: https://github.com/jcrist/msgspec/blob/24b7a9303be97ace1bb3e5863c6a45de2352d97e/src/msgspec/_core.c#L7311

    Some of the docs may also need updating to note that StructMeta disallows subclassing.

  16. kramar11 commented on Nov 3, 2025

    @kramar11

    Don't know if thats a good or bad idea, but what about keeping the Struct class as it currently is (i.e. with its limitations, but great speed) and also have a CustomStruct class that exposes StructMeta, allows multiple inheritance etc. and generally is more customizable but probably has the drawback of not having the same performance of the "plain" Struct.

  17. ofek commented on Nov 13, 2025

    @ofek
    Member

    As far as I can tell, there was no performance regression. Although I was able to optimize slightly. #927

    Working on the release.

  18. provinzkraut commented on Nov 13, 2025

    @provinzkraut
    Member

    Thanks for the thorough investigation @ofek, great work! ✨

  19. ofek commented on Nov 14, 2025

    @ofek
    Member

    #928 - This was a nightmare to debug and fix, especially being unfamiliar with CPython internals 😅

  20. apoorvkh commented on Nov 14, 2025

    @apoorvkh
    Contributor

    Thank you so much for debugging this and finding a middle-ground solution @ofek!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions