FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-150942: Optimize PicklerMemoProxy.copy() with _PyDict_SetItem_Take2 by corona10 · Pull Request #151608 · python/cpython · GitHub

Repository navigation

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

corona10 merged 1 commit into
python:mainfrom
corona10:gh-150942-pickle

Conversation

corona10 commented Jun 17, 2026 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

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 requested a review from vstinner June 17, 2026 16:48

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.

Copy link
Copy Markdown
Member Author

Yeah, I also not sure how it will be practical

eendebakpt commented Jun 17, 2026 •
edited
Loading

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 merged commit 8d7c6dc into python:main Jun 18, 2026
55 checks passed
pyctrl pushed a commit to pyctrl/cpython that referenced this pull request Jul 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL