Skip to content

gh-114087: Speed up dataclasses._asdict_inner#114088

Merged
carljm merged 14 commits into
python:mainfrom
keithasaurus:dataclasses_comprehensions
Jan 18, 2024
Merged

gh-114087: Speed up dataclasses._asdict_inner#114088
carljm merged 14 commits into
python:mainfrom
keithasaurus:dataclasses_comprehensions

Conversation

@keithasaurus

Copy link
Copy Markdown
Contributor

gh-114087: Speed up dataclasses._asdict_inner

What I did

I made several adjustments to _asdict_inner to improve performance:

  • handle branches for builtin types for list, dict, and tuple explicitly: this allows for the use of comprehensions, as opposed to calling the constructors with a generator expression
  • store result of type(obj) in a variable, so we aren't calling it more than once
  • because we're storing type(obj) in a variable, we don't want/need to call that again inside of _is_dataclass_instance(obj); instead, we inline the call to hasattr, which results in two fewer function calls
  • use a list comprehension for the case where dict_factory is not dict

Benchmark

I benchmarked a number of different kinds of dataclasses with the following code:

from collections import defaultdict
from dataclasses import dataclass, field, _asdict_inner
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_inner(dataclass_instance, dict_factory=dict_proxy)

    result = time() - start

    print(f"{label}: {result}")
    total_duration += result

print(f"total: {total_duration}")

Results

Running on my M1, I got the following results:

main branch

non-dict dict_factory: 0.5098788738250732
namedtuple child: 0.146165132522583
list child: 0.5027599334716797
recipe params: 0.5992569923400879
tuples: 0.30904173851013184
unoptimized: 0.4593799114227295
defaultdict: 0.134871244430542
total: 2.661353826522827

this branch

non-dict dict_factory: 0.4076099395751953
namedtuple child: 0.14599394798278809
list child: 0.37206101417541504
recipe params: 0.5059669017791748
tuples: 0.23736095428466797
unoptimized: 0.4441537857055664
defaultdict: 0.12598085403442383
total: 2.2391273975372314

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.

@bedevere-app

bedevere-app Bot commented Jan 15, 2024

Copy link
Copy Markdown

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 skip news label instead.

@keithasaurus keithasaurus changed the title Dataclasses comprehensions Speed up dataclasses._asdict_inner Jan 15, 2024
@keithasaurus keithasaurus changed the title Speed up dataclasses._asdict_inner gh-114087: Speed up dataclasses._asdict_inner Jan 15, 2024
@@ -0,0 +1 @@
Speed up dataclasses.asdict by up to 1.35x.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Suggested change
Speed up dataclasses.asdict by up to 1.35x.
Speed up ``dataclasses.asdict`` up to 1.35x.

Comment thread Lib/dataclasses.py
Comment on lines +1335 to +1336
obj_type = type(obj)
if obj_type in _ATOMIC_TYPES:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Comment thread Lib/dataclasses.py
(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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The implementation looks ok, but I am not really fond of the code duplication this introduces.

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.

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.

@AlexWaygood
AlexWaygood requested a review from carljm January 16, 2024 10:42

@carljm carljm 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.

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.

Comment thread Lib/dataclasses.py
(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

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.

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.

@bedevere-app

bedevere-app Bot commented Jan 16, 2024

Copy link
Copy Markdown

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@keithasaurus

Copy link
Copy Markdown
Contributor Author

Thanks for catching the typo!

The results from fixing the benchmark and comparing asdict are:
main branch

non-dict dict_factory: 0.5187351703643799
namedtuple child: 0.13918018341064453
list child: 0.45549607276916504
recipe params: 0.5650432109832764
tuples: 0.28313183784484863
unoptimized: 0.4386301040649414
defaultdict: 0.12912297248840332
total: 2.529339551925659

this branch

non-dict dict_factory: 0.4153609275817871
namedtuple child: 0.13956284523010254
list child: 0.31925034523010254
recipe params: 0.48323607444763184
tuples: 0.20696306228637695
unoptimized: 0.4236268997192383
defaultdict: 0.11920905113220215
total: 2.1072092056274414

Both benchmarks are a little faster, presumably because of using dict instead of dict_proxy for most of the cases. The overall improvement is ~1.20x now.

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 carljm 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.

This is a very nice speedup! LGTM, will wait a day or two before merging in case @ericvsmith has comments.

@ericvsmith

Copy link
Copy Markdown
Member

@carljm This looks good to me!

@carljm
carljm merged commit 2d3f6b5 into python:main Jan 18, 2024
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants