-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
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
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?
@pablogsal , @gpshead Would love to hear your thoughts on this (whenever you get a chance to take a look) :) |
Ok I resolved the merge conflict, but I think the new MacOS failure is unrelated. |
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
@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
We still have some failing tests it seems |
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. |
I will look at this over the weekend on my Windows box, this trial-and-error is not an optimal way :) |
Don't worry, I pushed a commit that should fix it |
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: