Skip to content

gh-154594: Fix copy.deepcopy() re-copying objects whose __deepcopy__ returns None - #154819

Closed
sreehariannam wants to merge 2 commits into
python:mainfrom
sreehariannam:gh-154594-deepcopy-none-memo
Closed

gh-154594: Fix copy.deepcopy() re-copying objects whose __deepcopy__ returns None#154819
sreehariannam wants to merge 2 commits into
python:mainfrom
sreehariannam:gh-154594-deepcopy-none-memo

Conversation

@sreehariannam

@sreehariannamsreehariannam commented Jul 28, 2026

Copy link
Copy Markdown

Fixesgh-154594.

copy.deepcopy() uses memo.get(d, None) to check the memo dict, treating None both as the "not cached" sentinel and as a value a __deepcopy__ can legitimately return. When an object's __deepcopy__ returns None, the memo hit is indistinguishable from a miss, so the object gets deep-copied again on every subsequent reference instead of once:

importcopycall_count=0classC:
def__deepcopy__(self, memo):
globalcall_countcall_count+=1memo[id(self)] =NonereturnNoneobj=C()
copy.deepcopy([obj, obj, obj])
print(call_count) # 3, expected 1

This was introduced by gh-132657 / GH-138429, which replaced the previous _nil = [] sentinel with None specifically because None is an immortal singleton in CPython, avoiding refcount contention under free-threading that a plain sentinel object would reintroduce.

The fix switches the lookup to if d in memo: return memo[d] (as suggested by @pochmann in the issue discussion), rather than reintroducing a sentinel object or special-casing None. This distinguishes every possible cached value (including None) from a genuine cache miss, without any shared sentinel object and without the exception-handling overhead a try/except KeyError form would add on every miss.

I benchmarked three candidate approaches (current is not None check, try/except KeyError, and d in memo) with pyperf on hit-heavy and miss-heavy workloads, and with Tools/ftscalingbench under free-threading:

workloadbaseline (is not None)try/exceptd in memo (this PR)
hit-heavy (pyperf)781 µs877 µs810 µs
miss-heavy (pyperf)9.86 ms11.5 ms9.62 ms
ftscalingbench, 8 threads~2.8x scaling~2.4x scaling~2.8x scaling

d in memo ties or beats the current baseline on every axis. The earlier try/except version of this PR was ~17% slower on misses (the common case, per discussion in the issue) and scaled worse under free-threading contention; d in memo doesn't have either regression.

Added a regression test and a changelog entry.

…opy__ returns None
deepcopy() used None both as the memo dict's "not cached" sentinel and
as a value callers can legitimately store there, so a __deepcopy__ that
returns None was indistinguishable from a cache miss and got invoked
again on every subsequent reference to the same object.
Use a direct memo[d] lookup (matching the existing pattern in
_deepcopy_tuple/_deepcopy_frozendict) instead of memo.get(d, None), so
every return value -- including None -- is memoized correctly. This
also avoids reintroducing the non-immortal-sentinel refcount
contention under free-threading that pythongh-132657/pythongh-138429 fixed, since
no sentinel object is used at all.
@eendebakpt

Copy link
Copy Markdown
Contributor

@sreehariannam Can you create benchmarks for your approach (in the issue discussion there are other suggestions that seem promising as well). For the benchmarks please use pyperf. For the free-threading contention you will need some other benchmark tool (there are some scripts under Tools/ftscalingbench that might work).

pochmann benchmarked this issue's candidate fixes on pythongh-154594 and found
the try/except form is ~3.5x slower on a memo miss (the common case in
real deepcopy workloads) despite being fastest on a hit. `d in memo` ties
or beats the current `is not None` check on both hit and miss, and
matches where the competing PR (pythongh-154595) landed after the same
discussion.
@sreehariannam

Copy link
Copy Markdown
Author

@eendebakpt Per your request, I benchmarked this with pyperf and Tools/ftscalingbench, comparing three candidates: the current baseline (memo.get(d, None) / is not None), this PR's original try/except KeyError approach, and d in memo (as suggested by @pochmann above).

pyperf (single-threaded, release build, hit-heavy = repeated shared objects, miss-heavy = all-unique objects):

workloadbaselinetry/exceptd in memo
hit-heavy781 µs ± 90 µs877 µs ± 75 µs810 µs ± 113 µs
miss-heavy9.86 ms ± 1.44 ms11.5 ms ± 1.2 ms9.62 ms ± 1.02 ms

Tools/ftscalingbench deepcopy benchmark (free-threading build, 8 threads, 3 runs each):

variantscaling factor
baseline2.5x / 3.0x / 3.0x
try/except2.3x / 2.6x / 2.3x
d in memo2.6x / 2.5x / 3.3x

Takeaways:

  • try/except is consistently the slowest on misses (which dominate real deepcopy workloads, per this thread) and shows worse free-threading scaling than baseline.
  • d in memo ties or slightly beats baseline on every measure, with no free-threading regression.

I've updated this PR to use d in memo instead of try/except, matching where the discussion here landed. Happy to share the benchmark scripts if useful.

@eendebakpt

Copy link
Copy Markdown
Contributor

@sreehariannam Thank you for the benchmarks. It seems to d in memo approach works best. I will close this PR, and go with the other PR which was opened earlier.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

copy.deepcopy memo lookup fails when __deepcopy__ returns None

2 participants

@sreehariannam@eendebakpt