Skip to content
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

Pickle raw tuples in FileData cache #3877

Merged
merged 4 commits into from
Sep 10, 2023
Merged

Conversation

JelleZijlstra
Copy link
Collaborator

No description provided.

@JelleZijlstra
Copy link
Collaborator Author

Discussion and perf numbers in #3867 (comment).

I also tried pickling and pickling raw tuples is ~2x as fast as pickling NamedTuples for me:

In [88]: %timeit pickle.dumps([(*FileData(i, "x", 1.0),) for i in range(100)])
63.7 µs ± 5.33 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

In [89]: %timeit pickle.dumps([FileData(i, "x", 1.0) for i in range(100)])
124 µs ± 3.71 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

These numbers are without mypyc compilation so they may not completely reflect reality, but if anything I'd expect mypyc to magnify the difference (because the raw tuple version does more work in user code that would be sped up by mypyc, and the NamedTuple version does more work inside pickle that mypyc can't help with).

@github-actions
Copy link

diff-shades reports zero changes comparing this PR (ad54fdd) to main (c83ad6c).


What is this? | Workflow run | diff-shades documentation

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(can also use pickle protocol 5 if we want to now)

@hauntsaninja hauntsaninja merged commit 751583a into psf:main Sep 10, 2023
29 checks passed
@JelleZijlstra JelleZijlstra deleted the simplepickle branch September 11, 2023 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants