Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-109587: Allow "precompiled" perf-trampolines to largely mitigate the cost of enabling perf-trampolines#109666
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
Conversation
gsallam commented Sep 21, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Lib/test/test_stable_abi_ctypes.py Outdated
| "PyUnstable_PerfTrampoline_CompileCode", | ||
| "PyUnstable_PerfTrampoline_SetPersistAfterFork", |
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.
Here too?
Uh oh!
There was an error while loading. Please reload this page.
czardoz commented Oct 5, 2023
@pablogsal , @gpshead Would love to hear your thoughts on this (whenever you get a chance to take a look) :) |
czardoz commented Oct 6, 2023
Ok I resolved the merge conflict, but I think the new MacOS failure is unrelated. |
pablogsal left a comment
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.
LGTM
Seems that we have some conflicts that we need to address before landing, @gsallam can you take a look?
Head branch was pushed to by a user without write access
pablogsal commented Oct 20, 2023
@gsallam We still have some failures in the Windows and macOS CI. I think this is because the symbol doesn't exist in those cases: |
Summary: As titled. To be on sync with python/cpython#109666. Reviewed By: czardoz Differential Revision: D50329585 fbshipit-source-id: d4c52143216a1d9970196ad71ef656098be50120
pablogsal commented Oct 26, 2023
We still have some failing tests it seems |
czardoz commented Oct 27, 2023
This approach is likely quite silly, but I don't have a Windows/Mac machine handy, so I'm kicking off these builds to test/debug. |
czardoz commented Oct 27, 2023
I will look at this over the weekend on my Windows box, this trial-and-error is not an optimal way :) |
pablogsal commented Oct 27, 2023
Don't worry, I pushed a commit that should fix it |
czardoz commented Oct 27, 2023
Thanks a lot @pablogsal! 😄 |
…gate the cost of enabling perf-trampolines (python#109666)
…gate the cost of enabling perf-trampolines (python#109666)
Summary: Straight up port of python/cpython#109666 Reviewed By: jbower-fb Differential Revision: D66118319 fbshipit-source-id: bd7d4181c16d35526f57500cebb6e16fc2df58aa
Summary: Straight up port of python/cpython#109666 Reviewed By: jbower-fb Differential Revision: D66118319 fbshipit-source-id: bd7d4181c16d35526f57500cebb6e16fc2df58aa
This pull request implements a proposal in #109587 to allow precompiled perf-trampolines. This would mitigate the costs of enabling perf-trampolines, such as the disk IO and memory overhead.
The proposal introduces two new C-API functions:
PyUnstable_PerfTrampoline_CompileCode(): Creates a new trampoline and registers it.PyUnstable_PerfTrampoline_SetPersistAfterFork(): Controls whether to re-initialize trampolines in child processes.These functions can be used by extension modules to initialize trampolines eagerly, after the application is "warmed up". This would make it possible to have perf-trampoline running in an always-enabled fashion.
Benefits for a forked multiprocess model: