Skip to content

bpo-45455: Set func.__doc__ lazily. #28704

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.

Sign up for GitHub

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

Jump to bottom
Closed
methane wants to merge 3 commits into from
Closed

bpo-45455: Set func.__doc__ lazily. #28704

methane wants to merge 3 commits into from

Conversation

@methane
Copy link
Member

@methane methane commented Oct 3, 2021 •
edited by bedevere-bot
Loading

serhiy-storchaka
serhiy-storchaka approved these changes Oct 3, 2021
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Yesterday I experimented with a patch which added the co_doc property and _PyCode_GetDoc() for extracting a docstring from code's constants. I was not sure in the usefulness of that feature, seems there are no use of co_doc in the Python code, and there is exactly one use of _PyCode_GetDoc() in the C code.

But your PR looks more interesting.

@warsaw
Copy link
Member

warsaw commented Oct 3, 2021

Do you have a bug #? Is the motivation to reduce memory overhead and performance with the docstring isn't used?

@methane
Copy link
Member Author

methane commented Oct 4, 2021 •
edited by bedevere-bot
Loading

@warsaw This PR is spin-off idea of bpo-36521. Motivation is faster function creation (including lambda) when docstring is not used.

But performance gain is only 3~5% in micro benchmark and having backward incompatibility:

>>> def foo(): "foo"
...
>>> def bar(): "bar"
...
>>> bar.__code__ = foo.__code__
>>> bar.__doc__
'foo'  # was 'bar'

P.S. I created new issue for this idea. bpo-45455.

@youknowone youknowone mentioned this pull request Oct 4, 2021
Open
@methane methane changed the title Set func.__doc__ lazy. bpo-45455: Set func.__doc__ lazy. Oct 13, 2021
@methane methane changed the title bpo-45455: Set func.__doc__ lazy. bpo-45455: Set func.__doc__ lazily. Oct 13, 2021
@github-actions
Copy link

github-actions bot commented Nov 13, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Nov 13, 2021
@rhettinger
Copy link
Contributor

rhettinger commented May 10, 2022

I don't see the benefit. We still use the same amount of the memory. The pointer just gets copied later rather than sooner. AFAICT, at best it saves a single INCREF.

@methane methane closed this May 11, 2022
@methane methane deleted the lazy-func-doc branch May 11, 2022 03:07
@methane
Copy link
Member Author

methane commented May 11, 2022

Yes. This have only 3~5% performance gain on microbenchmark.

@methane methane mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees

No one assigned

Labels

awaiting merge stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@methane @warsaw @rhettinger @serhiy-storchaka @the-knights-who-say-ni @bedevere-bot