Skip to content

gh-122459: Optimize pickling by name objects without __module__ #122460

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

Merged
merged 2 commits into from
Aug 5, 2024

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jul 30, 2024

@mdboom
Copy link
Contributor

mdboom commented Jul 31, 2024

There are some pickle benchmarks in the benchmark suite. I don't know if they exercise this case (objects without __module__), but I'll run the benchmarks on this PR just to ensure there is no unintentional regression.

@mdboom
Copy link
Contributor

mdboom commented Jul 31, 2024

This does seem to make the unpickle_pure_python benchmark about 2% slower. I haven't dug in to see whether it exercises the case where __module__ is missing, but maybe we are making the common case slower to make an uncommon case faster here?

@serhiy-storchaka
Copy link
Member Author

2% is so small difference, that it is most likely a random fluctuation or the compiler fluke. Indeed, the test data does not contain functions or other objects that can be be affected by this change.

On other hand, the following example (pickling a function without __module__) runs more than twice faster with this change:

import sys
sys.modules['_pickle'] = None
import pickle, timeit
def f():
    pass

f.__module__ = None
print(min(timeit.repeat('pickle.dumps(f)', globals=globals(), number=10**4)))

@serhiy-storchaka serhiy-storchaka merged commit 1bb955a into python:main Aug 5, 2024
37 checks passed
@serhiy-storchaka serhiy-storchaka deleted the pickle-save-global3 branch August 5, 2024 13:21
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.

2 participants