-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-121192: Add fast path to deepcopy()
for empty list/tuple/dict/set
#121193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
d7b07eb
to
d87901e
Compare
The optimization applied in the latest iteration of the PR is only applied when no memo is passed to The question then is whether a
raises a
results in
|
For reference: I performed a benchmark of main, this PR and the deepcopy C implementation from #91610. On benchmark
Results are
|
@eendebakpt it can happen for dataclass, example: from dataclasses import dataclass, as_dict
@dataclass
class Some:
field: set[int]
as_dict(Some(set())) |
Indeed. Here a more complete example, including the cases where this PR helps
|
Thanks for the example! This PR indeed shows a decent improvement here: import pyperf
runner = pyperf.Runner()
setup = """
from dataclasses import dataclass, asdict
@dataclass
class Some:
a: str
t: tuple[int]
field: set[int] # benefits
l : list[int]
d : dict[str, str]
fs: frozenset[int] # benefits
nonempty : set[int]
s = Some('x', tuple(), set(), [], d={}, fs=frozenset(), nonempty={1, 2})
"""
runner.timeit(name="dataclass asdict", stmt=f"b = asdict(s)", setup=setup)
|
Other use-cases that we can find in our own code:
|
deepcopy()
can be surprisingly slow when called with empty containers like lists, tuples, dicts, sets or frozensets.This adds a fast path for this case similar to #114266 which speeds up these cases by about 4x - 21x while having a small impact on the default path.
Here are some benchmarks comparing this PR against
main
. Let me know if there are more benchmarks that I can run to verify that there a no performance regressions for the common case:deepcopy()
for empty list/tuple/dict/set #121192