Repository navigation
Description
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
ofek commentedon Oct 26, 2025
Thanks a lot for both trying out the functionality and the detailed description of the issue! I can reproduce, and it appears like
StructMetadoesn'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
matterfield-sam commentedon Oct 26, 2025
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
StructMetaisn'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.provinzkraut commentedon Oct 26, 2025
It's been a while but iirc
Structnot calling__new__, as well as there not being an exposed metaclass forStructs 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 🤷♀️
ofek commentedon Oct 26, 2025
I didn't even consider that, thanks for explaining! I'm going to benchmark today but do you think the previous PR that exposed
StructMetaintroduced a performance regression?Yes, most definitely.
provinzkraut commentedon Oct 26, 2025
I didn't take a close look at it yet, but it's conceivable that the changes from
Py_TYPE(base) != &StructMetaTypeto!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?
apoorvkh commentedon Oct 26, 2025
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.pybenchmark. 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 exposingStructMetashouldn'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 usingabstract_issue.pyabove 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 explicitStructMetacase, rather than creating a regression.matterfield-sam commentedon Oct 26, 2025
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.pyandbench_encodings.pywith 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, soABCStruct(ABCMeta, StructMeta)andStructABC(StructMeta, ABCMeta)- the goal was to see if allowingABCMetato perform all themclssetup before callingStructMeta(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
bench_encodings.py: Encode/Decode Perf, n=1000
provinzkraut commentedon Oct 26, 2025
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.pyto run each test 100 times, with a few warmup runs):546f21d (exposed
StructMeta)vs 7acceff (commit before)
apoorvkh commentedon Oct 26, 2025
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
msgspeccontinues to be generally 6x faster than the second best library (mashumaro) and so forth. I think the difference of 5–10 μs perencodeordecodeinvocation would be imperceptible (even at scale). On the other hand, there seemed to be a good deal of demand for theStructMetafeature :)provinzkraut commentedon Oct 26, 2025
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:
Then it should be possible to judge if the compromise is a reasonable one.
ofek commentedon Oct 26, 2025
I think the fast-path check recommendation mentioned in one of the comments is a good idea and is what should happen.
ncoghlan commentedon Oct 28, 2025
@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
StructMetais 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 interoperableStructMetashould also be tested for compatibility with__init_subclass__and with instance descriptors that make use of__set_name__without beingStructfield 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.
ncoghlan commentedon Oct 30, 2025
One potential way to approach that would be to keep most of the changes to make
StructMetamore accessible, but turn the flag that allows subclassing back off for now.ofek commentedon Oct 30, 2025
I'll take a look at that this evening then, thanks! Would you mind linking the part you're referencing?
ncoghlan commentedon Oct 30, 2025
Removing
Py_TPFLAGS_BASETYPEfromtp_flagsagain in the declaration ofStructMeta: https://github.com/jcrist/msgspec/blob/24b7a9303be97ace1bb3e5863c6a45de2352d97e/src/msgspec/_core.c#L7311Some of the docs may also need updating to note that
StructMetadisallows subclassing.kramar11 commentedon Nov 3, 2025
Don't know if thats a good or bad idea, but what about keeping the
Structclass as it currently is (i.e. with its limitations, but great speed) and also have aCustomStructclass 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.ofek commentedon Nov 13, 2025
As far as I can tell, there was no performance regression. Although I was able to optimize slightly. #927
Working on the release.
provinzkraut commentedon Nov 13, 2025
Thanks for the thorough investigation @ofek, great work! ✨
ofek commentedon Nov 14, 2025
#928 - This was a nightmare to debug and fix, especially being unfamiliar with CPython internals 😅
apoorvkh commentedon Nov 14, 2025
Thank you so much for debugging this and finding a middle-ground solution @ofek!