-
-
Notifications
You must be signed in to change notification settings - Fork 781
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
Document PGO in devguide #1153
Document PGO in devguide #1153
Conversation
add configure optimization options
add optimization option
option doc add
@hugovk @CAM-Gerlach @AlexWaygood Dear reviewers, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, it's really useful to document this! This looks pretty good.
Could you maybe start the section with a brief description of why you might or might not want to use this option? I worry that at the moment, people new to Python might think that this is the best option that they should use all the time (since "optimized" is a very positive adjective!)
Maybe the section could start with something like this?
If you are trying to improve the performance of Python, you will probably want to use an optimized build of CPython. It can take a lot longer to build CPython with optimizations enabled, and it's usually not necessary to do so. However, it's essential if you want accurate benchmark results for a proposed performance optimization.
Could you also please wrap each line to 80 characters, like the other paragraphs in this document?
Closing and reopening to retrigger the CLA bot |
bfbf2d8
to
5cbed59
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like to update the PR with comments from other reviewers?
@corona10 @AlexWaygood |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, nearly there! A few more small suggestions below. If you like my suggestions, you can apply them by clicking the "commit suggestion" button below each suggestion :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on how much detail you want to go into, you could link the specific options themselves with :option`python:--enable-optimizations`
or :option`python:--with-lto`
.
Related, I was a little surprised to see configure options in d.p.o -- thinking about it, should we consider moving it to the devguide? I'm not sure many end-users build their own Python from sources...
A
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks great!
@AlexWaygood
@AA-Turner |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall! Thanks @KilJaeeun and again congratulations on your first PR!
A
Co-authored-by: Adam Turner <[email protected]>
Thanks again @KilJaeeun, this was a great first PR! |
Hi! this is my first open source Pull Request in CPython Sprint at PyCon KR so it might be awkward. Added description of enable optimization option. Please take good care of me!
Related issue
I made this PullRequest with these references
📚 Documentation preview 📚: https://cpython-devguide--1153.org.readthedocs.build/