gh-114087: Speed up dataclasses._asdict_inner#114088
Conversation
…cpython into dataclasses_comprehensions
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
dataclasses._asdict_inner
dataclasses._asdict_inner| @@ -0,0 +1 @@ | |||
| Speed up dataclasses.asdict by up to 1.35x. | |||
There was a problem hiding this comment.
The performance benchmarks in the PR are on _asdict_inner. Could you create benchmarks on the public .asdict as well? I suspect performance improvements will be a a bit less and it seems fair to report only the improvements for the public interface.
| Speed up dataclasses.asdict by up to 1.35x. | |
| Speed up ``dataclasses.asdict`` up to 1.35x. |
| obj_type = type(obj) | ||
| if obj_type in _ATOMIC_TYPES: |
There was a problem hiding this comment.
This change might make the case for atomic types a tiny bit slower, since we now have to assign to the variable obj_type . But it is faster for all other cases, so seems fine.
There was a problem hiding this comment.
Yes. This is mitigated by the fact that the first pass through _asdict_inner is always the dataclass instance itself, so the hasattr speedup (removing a function call) somewhat offsets the obj_type slowdown.
| (f.name, _asdict_inner(getattr(obj, f.name), dict_factory)) | ||
| for f in fields(obj) | ||
| ]) | ||
| # handle the builtin types first for speed; subclasses handled below |
There was a problem hiding this comment.
The implementation looks ok, but I am not really fond of the code duplication this introduces.
There was a problem hiding this comment.
I'm not concerned about this; there isn't duplication of boilerplate, just the essential differences between generating different types of values in the most efficient way.
carljm
left a comment
There was a problem hiding this comment.
Code looks good to me! Thanks for the PR.
I would also prefer to benchmark the public API (asdict) and report those numbers in the What's New entry.
Also, it looks to me like there is a typo in the benchmark script that means all the benchmarks are using dict_proxy as the dict_factory; would be worth fixing that and seeing if it changes the results.
| (f.name, _asdict_inner(getattr(obj, f.name), dict_factory)) | ||
| for f in fields(obj) | ||
| ]) | ||
| # handle the builtin types first for speed; subclasses handled below |
There was a problem hiding this comment.
I'm not concerned about this; there isn't duplication of boilerplate, just the essential differences between generating different types of values in the most efficient way.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Thanks for catching the typo! The results from fixing the benchmark and comparing this branch Both benchmarks are a little faster, presumably because of using For reference the benchmarking code now looks like: from collections import defaultdict
from dataclasses import dataclass, field, asdict
from time import time
from typing import NamedTuple, Union, Optional
from decimal import Decimal
@dataclass
class Node:
children: dict[str, "Node"] | None
def dict_proxy(arg):
return dict(arg)
class Child(NamedTuple):
child: Union["Child", None]
@dataclass
class NTContainer:
child: Child
@dataclass
class P:
ps: list["P"]
@dataclass
class RecipeParams:
diets: list[str] = field(default_factory=lambda: [])
ingredients: list[str] = field(default_factory=lambda: [])
limit: Optional[int] = None
recipe_ids: set[str] = field(default_factory=lambda: set())
search_str: Optional[str] = None
seconds_min: Optional[int] = None
seconds_max: Optional[int] = None
tags: list[str] = field(default_factory=lambda: [])
username: Optional[str] = None
uses_recipe_id: Optional[str] = None
@dataclass
class T:
children: tuple["T", ...]
class X:
def __init__(self, a, b):
self.a = a
self.b = b
@dataclass
class AtomicTypesAndFallThroughTypes:
s: str
i: int
f: float
d: Decimal
x: X
@dataclass
class DefDict:
dd: defaultdict[str, int]
ex_1 = Node(
{"a": Node(None),
"b": Node({}),
"c": Node({"d": Node(None)})}
)
ex_2 = NTContainer(
Child(child=Child(child=None))
)
ex_3 = P([P([]), P([]), P([P([])])])
ex_4 = RecipeParams(
diets=["vegan", "vegetarian"],
ingredients=["carrot", "olive oil", "salt"],
limit=10,
seconds_max=3600,
)
ex_5 = T(
(T(
(T(
()),
)),
),
)
ex_6 = AtomicTypesAndFallThroughTypes(
s="abcdefg",
i=12345,
f=1.1,
d=Decimal("3.14"),
x=X(1,2)
)
dd = defaultdict(int)
dd["a"] = 1
dd["b"] = 2
ex_7 = DefDict(dd)
total_duration = 0.0
for label, dataclass_instance, dict_factory in [
("non-dict dict_factory", ex_1, dict_proxy),
("namedtuple child", ex_2, dict),
("list child", ex_3, dict),
("recipe params", ex_4, dict),
("tuples", ex_5, dict),
("unoptimized", ex_6, dict),
("defaultdict", ex_7, dict)
]:
start = time()
for i in range(100000):
asdict(dataclass_instance, dict_factory=dict_factory)
result = time() - start
print(f"{label}: {result}")
total_duration += result
print(f"total: {total_duration}") |
carljm
left a comment
There was a problem hiding this comment.
This is a very nice speedup! LGTM, will wait a day or two before merging in case @ericvsmith has comments.
|
@carljm This looks good to me! |
gh-114087: Speed up
dataclasses._asdict_innerWhat I did
I made several adjustments to
_asdict_innerto improve performance:list,dict, andtupleexplicitly: this allows for the use of comprehensions, as opposed to calling the constructors with a generator expressiontype(obj)in a variable, so we aren't calling it more than oncetype(obj)in a variable, we don't want/need to call that again inside of_is_dataclass_instance(obj); instead, we inline the call tohasattr, which results in two fewer function callsdict_factoryis notdictBenchmark
I benchmarked a number of different kinds of
dataclasses with the following code:Results
Running on my M1, I got the following results:
mainbranchthis branch
According to the benchmark (caveats about imperfect benchmarks...) the overall performance is up about 1.19x. Particularly interesting to me is that the "list child" case's performance (a common case) is up about 1.35x. None of these examples saw a slowdown.