Skip to content

gh-150942: Optimize PicklerMemoProxy.copy() with _PyDict_SetItem_Take2#151608

Merged
corona10 merged 1 commit into
python:mainfrom
corona10:gh-150942-pickle
Jun 18, 2026
Merged

gh-150942: Optimize PicklerMemoProxy.copy() with _PyDict_SetItem_Take2#151608
corona10 merged 1 commit into
python:mainfrom
corona10:gh-150942-pickle

Conversation

@corona10

@corona10 corona10 commented Jun 17, 2026

Copy link
Copy Markdown
Member

@corona10

Copy link
Copy Markdown
Member Author

cc @eendebakpt

1-2% consistenatly faster with simple replace logic, I think that it's worth to do this.

PicklerMemoProxy.copy/2505entries: Mean +- std dev: [base] 136 us +- 4 us -> [opt] 133 us +- 5 us: 1.02x faster
PicklerMemoProxy.copy/50005entries: Mean +- std dev: [base] 4.57 ms +- 0.06 ms -> [opt] 4.52 ms +- 0.05 ms: 1.01x faster

Geometric mean: 1.02x faster
import io
import pickle
import pyperf


def make_proxy(n):
    buf = io.BytesIO()
    pk = pickle.Pickler(buf, protocol=pickle.HIGHEST_PROTOCOL)
    data = [object() for _ in range(n)]
    nested = {i: [object(), object()] for i in range(n // 2)}
    pk.dump(data)
    pk.dump(nested)
    proxy = pk.memo  # PicklerMemoProxy
    # Keep a reference to the source pickler alive via closure.
    return pk, proxy


def main():
    runner = pyperf.Runner()

    for n in (1000, 20000):
        pk, proxy = make_proxy(n)
        entries = len(proxy.copy())
        runner.bench_func(
            f"PicklerMemoProxy.copy/{entries}entries",
            proxy.copy,
        )


if __name__ == "__main__":
    main()

@corona10 corona10 requested a review from vstinner June 17, 2026 16:48
@eendebakpt

Copy link
Copy Markdown
Contributor

The change itself is ok (the dict is fresh, so no risk of concurrent threads mutating).

But is the proxy used in any computations? I thought the proxy was only for human inspection.

@corona10

Copy link
Copy Markdown
Member Author

Yeah, I also not sure how it will be practical

@eendebakpt

eendebakpt commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Yeah, I also not sure how it will be practical

I am fine with merging though. The code looks cleaner (although this is subjective), and if we do not merge we risk someone else making a similar PR.

@corona10 corona10 merged commit 8d7c6dc into python:main Jun 18, 2026
55 checks passed
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.

2 participants