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

@sreehariannam sreehariannam commented Jul 28, 2026

Copy link
Copy Markdown

Fixes gh-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:

import copy

call_count = 0

class C:
    def __deepcopy__(self, memo):
        global call_count
        call_count += 1
        memo[id(self)] = None
        return None

obj = 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:

workload baseline (is not None) try/except d in memo (this PR)
hit-heavy (pyperf) 781 µs 877 µs 810 µs
miss-heavy (pyperf) 9.86 ms 11.5 ms 9.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):

workload baseline try/except d in memo
hit-heavy 781 µs ± 90 µs 877 µs ± 75 µs 810 µs ± 113 µs
miss-heavy 9.86 ms ± 1.44 ms 11.5 ms ± 1.2 ms 9.62 ms ± 1.02 ms

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

variant scaling factor
baseline 2.5x / 3.0x / 3.0x
try/except 2.3x / 2.6x / 2.3x
d in memo 2.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.

@eendebakpt eendebakpt closed this Aug 4, 2026
Sign up for free to 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