Skip to content

Skip instance __dataclass_fields__ lookup for non-dataclass types - #1196

Merged
sobolevn merged 4 commits into
msgspec:mainfrom
Kumzy:perf/dataclass-fields-type-lookup
Sep 29, 2026
Merged

Skip instance __dataclass_fields__ lookup for non-dataclass types#1196
sobolevn merged 4 commits into
msgspec:mainfrom
Kumzy:perf/dataclass-fields-type-lookup

Conversation

@Kumzy

@Kumzy Kumzy commented Sep 21, 2026

Copy link
Copy Markdown

Skip the instance __dataclass_fields__ lookup when the type doesn't define it, so objects with a Python level __getattr__ (like pydantic models) reach enc_hook without running Python code.

@provinzkraut

provinzkraut commented Sep 21, 2026

Copy link
Copy Markdown
Member

I'm not sure where we stand on this. While dataclasses do not use __getattr__ in such a way, I'm not sure there aren't any libraries doing some funky metaprogramming to make a class appear as a dataclass and support its semantics (e.g. SQLAlchemy's MappedAsDataclass).

IMO if a type is recognised by dataclasses.is_dataclass, and behaves like a dataclass, we should support it. And dataclasses.is_dataclass simply uses a hasattr check: https://github.com/python/cpython/blob/484e025049d43c400d27097d969d3971076eff12/Lib/dataclasses.py#L1481.

@sobolevn any opinions on this?

sobolevn
sobolevn reviewed Sep 21, 2026
View reviewed changes

@sobolevn sobolevn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pros:

  • additional perf

Cons:

  • potential breaking change (which has very low probability in the real world)

Can we get the perf numbers please? So, we can fully decide on this.

@Kumzy

Kumzy commented Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

As for the results on my toaster computer on Python 3.13.15

case main PR change
json, no-op enc_hook, plain object 0.207 0.057 -72%
json, no-op enc_hook, pydantic model 0.865 0.060 -93%
msgpack, no-op enc_hook, plain object 0.201 0.054 -73%
to_builtins, no-op enc_hook, plain object 0.372 0.211 -43%
json, list of 100 pydantic models, Raw(to_json) hook 127.2 38.7 -70%
json, list of 100 pydantic models, model_dump hook 164.7 70.9 -57%
msgpack, list of 100 pydantic models, model_dump hook 154.7 66.8 -57%
json, list of 1000 attrs instances 210.5 78.7 -63%
json, unsupported type, no enc_hook (error path) 0.306 0.172 -44%
json, list of 1000 dataclass instances 41.0 45.9 +12%
msgpack, list of 1000 dataclass instances 43.7 44.8 +2%

Every object that reaches enc_hook gets faster, not only pydantic models, since a miss no longer creates and clears an AttributeError. Dataclass instances are the only path that is slower du to the extra lookup per instance (a few ns).

The script used below

uv run bench_msgspec_1196.py

bench_msgspec_1196.py

@sobolevn

sobolevn commented Sep 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

I'm all for the change, given its impressive perf results :)

@provinzkraut

provinzkraut commented Sep 21, 2026

Copy link
Copy Markdown
Member

12% slower encode of dataclasses seems a bit not so great. If we could mitigate that, I'd be for this. Otherwise

potential breaking change (which has very low probability in the real world)

As long as we do have an escape hatch, and document both it and the edge case, that would make it less severe

@Kumzy

Kumzy commented Sep 21, 2026

Copy link
Copy Markdown
Author

12% slower encode of dataclasses seems a bit not so great. If we could mitigate that, I'd be for this. Otherwise

Its on a list of dataclass, its 2% on a single dataclass

@Kumzy
Kumzy requested a review from sobolevn September 22, 2026 08:36
@Kumzy

Kumzy commented Sep 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Revamped it, so now for plain dataclasses it's using the fields dict we already found on the class instead of looking it up again on the instance, it removes the regression and makes ot a bit faster than main; unusual classes (custom __getattr__, descriptors) still take the old path.

The same bench file attached before was used (no modification). @provinzkraut @sobolevn
Python 3.13 used there.

case main PR PR vs main PR+fix PR+fix vs main
json, no-op enc_hook, plain object 0.207 0.057 -72% 0.055 -73%
json, no-op enc_hook, object with __getattr__ 0.208 0.057 -73% 0.057 -73%
msgpack, no-op enc_hook, plain object 0.202 0.054 -73% 0.053 -74%
to_builtins, no-op enc_hook, plain object 0.369 0.211 -43% 0.212 -43%
json, no-op enc_hook, pydantic model 0.871 0.062 -93% 0.058 -93%
json, list of 100 pydantic models, Raw(to_json) hook 125.5 38.8 -69% 38.5 -69%
json, list of 100 pydantic models, model_dump hook 164.0 72.0 -56% 70.6 -57%
msgpack, list of 100 pydantic models, model_dump hook 157.1 66.8 -57% 66.9 -57%
json, list of 1000 attrs instances 208.7 76.3 -63% 72.7 -65%
json, unsupported type, no enc_hook (error path) 0.306 0.173 -43% 0.168 -45%
json, list of 1000 dataclass instances 41.1 45.9 +12% 39.7 -3%
msgpack, list of 1000 dataclass instances 43.6 44.1 +1% 41.4 -5%

If this is accepted, I can add documentation about it + changelog

@Kumzy
Kumzy marked this pull request as ready for review September 22, 2026 09:19
@provinzkraut

provinzkraut commented Sep 22, 2026

Copy link
Copy Markdown
Member

This approach looks promising. Not having the performance hit on dataclasses is nice :)

sobolevn
sobolevn approved these changes Sep 29, 2026
View reviewed changes

@sobolevn sobolevn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good optimization idea and implementation! 👍

@sobolevn sobolevn mentioned this pull request Sep 29, 2026
Closed
@sobolevn
sobolevn added this pull request to the merge queue Sep 29, 2026
Merged via the queue into msgspec:main with commit 8a25471 Sep 29, 2026
24 checks passed
@Siyet Siyet mentioned this pull request Sep 29, 2026
Merged
pull Bot pushed a commit to Mu-L/msgspec that referenced this pull request Sep 29, 2026
@Siyet
Add the missing entries to the 0.22.0 changelog (msgspec#1215)
8a9b645 
Entries for msgspec#1194, msgspec#1196, msgspec#1197, msgspec#1199 and msgspec#1209 are missing from the
changelog of the 0.22.0 release. Of the changes merged after msgspec#1211
moved the unreleased entries into the 0.22.0 section, only msgspec#1207 added
its own entry. This adds the missing entries next to the related
entries in that section, and marks msgspec#1196 as a breaking change: objects
that expose `__dataclass_fields__` only through instance attribute
access are no longer encoded as dataclasses.

Co-authored-by: Tseluiko Aleksandr <4410812+Siyet@users.noreply.github.com>

This branch was successfully deployed

1 active deployment
docs-preview — bdd4614b Deployed Sep 29, 2026 by sobolevn via Deploy preview #568
View deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sobolevn sobolevn sobolevn approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Kumzy @provinzkraut @sobolevn