Support encoding more dataclass-like things - #501
Merged
Merged
Conversation
Previously we supported encoding dataclasses (determined as any object with a `__dataclass_fields__` attribute), provided those objects were implemented in a way similar-enough to how they were implemented in the standard library. The intent was to support stdlib dataclasses, and if alternative implementations (e.g. `pydantic.dataclasses`) happened to work then all the better. However, due to how we were detecting if an object was a dataclass, there was no way to override our builtin support if an alternative implementation (in this case `edgedb.Object`) didn't work. To fix this, we now make fewer assumptions about how the backing dataclass object is implemented. Pros: - We can now natively encode objects implemented using `dataclasses`, `pydantic.dataclasses`, and `edgedb.Object`. - We now only encode fields as declared on the dataclass object. Previously we encoded any attribute lacking a leading underscore, which was efficient and worked well in practice (it's also what `orjson` does). However, this can lead to weird behavior if some fields intentionally start with an `_` (like `_id` in mongodb) or if the object makes use of `functools.cached_property`. Cons: - This flexibility and correctness comes at a performance cost. The fast path is the common case (dataclass uses `__dict__`, doesn't override `__getattribute__`), but encoding is now ~20% slower than before. Before we encoded `__dict__` based dataclasses 20% faster than orjson; now we're faster for small classes (<= 8 items, on my machine) and slower for larger numbers of fields. For `__slots__` based classes we're still around 2x faster than `orjson`.
jcrist
force-pushed
the
refactor-dataclass-encode
branch
from
August 2, 2023 03:16
c3f3fed to
a9f3656
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously we supported encoding dataclasses (determined as any object with a
__dataclass_fields__attribute), provided those objects were implemented in a way similar-enough to how they were implemented in the standard library. The intent was to support stdlib dataclasses, and if alternative implementations (e.g.pydantic.dataclasses) happened to work then all the better.However, due to how we were detecting if an object was a dataclass, there was no way to override our builtin support if an alternative implementation (in this case
edgedb.Object) didn't work.To fix this, we now make fewer assumptions about how the backing dataclass object is implemented.
Pros:
dataclasses,pydantic.dataclasses, andedgedb.Object.orjsondoes). However, this can lead to weird behavior if some fields intentionally start with an_(like_idin mongodb) or if the object makes use offunctools.cached_property.Cons:
__dict__, doesn't override__getattribute__), but encoding is now ~20% slower than before. Before we encoded__dict__based dataclasses 20% faster than orjson; now we're faster for small classes and slower for larger numbers of fields (on my machine 12 fields is the crossover point). For__slots__based classes we're still around 2x faster thanorjson.Fixes #495.
TODO:
msgspec.json.encodemsgspec.msgpack.encodemsgspec.to_builtins