-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-141004: Document unstable perf map functions in ceval.h
#143492
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
base: main
Are you sure you want to change the base?
Conversation
Doc/c-api/frame.rst
Outdated
| .. versionadded:: 3.12 | ||
| .. c:macro:: PyUnstable_EXECUTABLE_KIND_SKIP |
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.
You used the branch from the other PR, please remove these changes here.
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.
I'm so sorry yes, fixing rn.
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.
I hope it's not an issue if i force push a new branch over this one to fix my error?
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.
In general, avoid them, in this case I think it is fine to remove the changes.
10b8fd7 to
9a85c56
Compare
| .. c:function:: int PyUnstable_CopyPerfMapFile(const char *parent_filename) | ||
| Open the ``/tmp/perf-$pid.map`` file and append the content of *parent_filename* |
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.
This should note what happens on Windows where this obviously won't work.
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.
That's the case for all of these functions; the info should be in the beginning of the section.
This doesn't open the perf map file directly; it should use the same wording as PyUnstable_WritePerfMapEntry: “Will call :c:func:PyUnstable_PerfMapState_Init…”
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.
@encukou Would this wording be suitable then as a replacement for line 56 as:
"Calls :c:func:PyUnstable_PerfMapState_Init if the perf map is not yet
initialized, then append the content of parent_filename to the perf map."
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.
@encukou or @StanFromIreland Could you verify this one too?
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.
I'd say only “Append contents of the file named parent_filename to the perf map.” in the opening paragraph, and add the note about _Init later (like in WritePerfMapEntry docs).
Doc/c-api/perfmaps.rst
Outdated
| Open the ``/tmp/perf-$pid.map`` file and append the content of *parent_filename* | ||
| to it. | ||
| :param parent_filename: The name of the file to copy. |
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.
Please don't use these (:param *: & :return:), like above use sentences.
| .. c:function:: int PyUnstable_CopyPerfMapFile(const char *parent_filename) | ||
| Open the ``/tmp/perf-$pid.map`` file and append the content of *parent_filename* |
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.
I'd say only “Append contents of the file named parent_filename to the perf map.” in the opening paragraph, and add the note about _Init later (like in WritePerfMapEntry docs).
| .. c:function:: int PyUnstable_PerfTrampoline_CompileCode(PyCodeObject *code) | ||
| Compile the given code object using the current perf trampoline. |
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.
What's the perf trampoline? How do you set the current one? What happens if there's no current trampoline?
| .. c:function:: int PyUnstable_PerfTrampoline_SetPersistAfterFork(int enable) | ||
| Set whether the perf trampoline should persist after a fork. |
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.
This isn't very clear to me; could you add/link more explanation about the behaviour on fork?
gh-141004: Document unstable perf map functions in
ceval.hThis PR documents the
PyUnstable_CopyPerfMapFile,PyUnstable_PerfTrampoline_CompileCode, andPyUnstable_PerfTrampoline_SetPersistAfterForkfunctions inDoc/c-api/perfmaps.rst.📚 Documentation preview 📚: https://cpython-previews--143492.org.readthedocs.build/