GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||
GeezerVison is showing github.com — gh-96143: Allow Linux perf profiler to see Python calls by pablogsal · Pull Request #96123 · python/cpython Conversation
|
|
You can use preprocesor macros if you name the file |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Hummm, not sure I follow, could you maybe show me an example of what we can achieve with this? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
You can have multiple implementations in the same file: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
df2a40d to
dda9e04
Compare
f38dfc2 to
22fc892
Compare
There was an error while loading. Please reload this page.
a2f4182 to
1b35ed3
Compare
|
Why did you add a Windows build file? How about we do not define |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Then we need to add more ifdef every place is used but that works as well for sure |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
they now match the current code.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Two thoughts have been running around in my mind:
(1) We've got the command line flag and sys API, but what about just enabling this system wide or at least task/job wide within a container? An environment variable that turns it on would make that easy without plumbing options through. PYTHONPERFSUPPORT=1 as another equivalent to -Xperf? That's usually done in initconfig.c alongside the -X processing I believe.
Second: multiprocessing spawn start method. If the -Xperf flag was passed to the parent process I assume the "spawn" method should also pass that to its children.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
I think (1) makes sense and is indeed coherent with how we handle some of the other options like Regarding (2) I am not that sure. We don't do this for the rest of the flags that we pass around and you can achieve the same with the environment variable if you want. In any case, as that would require some tests I would prefer to do that in a separate PR as this is already gigantic |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I implemented the environment variable and solved a bunch of conflicts. Please check it out when you have time. I also had to solve a bunch of conflicts. @gpshead if you are ok with the current status I would like us to land if everything looks good as the size of the PR is already attracting a bunch of merge conflicts in the build system, clinic and other files. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ah wait, I need to document the environment variable. Will push a commit for that soon. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Done 👍 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@erlend-aasland has mentioned that he was a bunch of docs improvements that he will do in a separate PR. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
My comments are easy to address things that don't need further review - minor edits or added note/todo comments to our future selves.
Agreed that the docs can use some polishing up but all the important bits are there to seed that future work, thanks for writing them!
Thanks for taking on adding this feature! I expect we'll see how a backport fares internally.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Damn, @miss-islington has landed the PR without waiting for the CI to build the last commit I pushed so I created #96433. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks, everyone for the fantastic reviews and for helping to get this feature ready ❤️ You all rock 🤘 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@erlend-aasland You can make the doc improvement PR after #96433 lands. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for all for pushing for better interoperability with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Nice feature! There is a typo in sys.deactivate_stack_trampoline() docstring: "Dectivate the perf profiler trampoline": Deactivate. In the doc, The second command can be replaced with |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
See #96445 :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Is it possible to port these changes, from python 3.12, without breaking to previous python versions, say 3.11, 3.10, 3.9? Why? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Is possible, but as these versions are only accepting security fixes or bugfixes we cannot backport new features. This means that if you want you can backport them yourself but you need to maintain a fork of the interpreter. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Summary: Backport the perf-trampoline introduced in python/cpython#96123. The perf trampoline doesn't work properly with the JIT, so we have submitted a PR to have a C-API to unify writing to the perf-map files python/cpython#103546. Reviewed By: czardoz Differential Revision: D45419843 fbshipit-source-id: 16bd13d7981e48c9eb7bc0e5eef1c1f4748965f6
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.