gh-136541: Fix several problems of perf trampolines in x86_64 and aarch64#136500

pablogsal · 2025-07-10T10:43:03Z

Fix the following problems:

The x86_64 trampolines are not preserving frame pointers
The hardcoded offsets to the code segment from the FDE only worked properly for x64_64
The CIE data was not following conventions of aarch64

Issue: Consider enabling perf trampoline on macOS #136459

Issue: perf trampolines are not reliable in aarch64 #136541

bedevere-bot · 2025-07-10T10:43:55Z

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 8b228c0 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136500%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

pablogsal · 2025-07-10T10:47:40Z

@diegorusso @brandtbucher @savannahostrowski

pablogsal · 2025-07-10T11:16:52Z

!buildbot perf

bedevere-bot · 2025-07-10T11:16:56Z

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit b174fe8 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136500%2Fmerge

The command will test the builders whose names match following regular expression: perf

The builders matched are:

AMD64 Arch Linux Perf PR

canova

Thanks!

Do you think it's possible to remove this skipUnless now?

cpython/Lib/test/test_perf_profiler.py

Lines 407 to 410 in d754f75

@unittest.skipUnless(
is_unwinding_reliable_with_frame_pointers(),
"Unwinding is unreliable with frame pointers",
)

(also I think there is a typo in that string. Probably was meant to say "Unwinding is unreliable without frame pointers".)

pablogsal · 2025-07-10T12:12:00Z

I don't think we can. I am pretty sure Perf will still choke without frame pointers in the outer functions and fail the test.

The sentence is indeed confusing but is trying to say that unwinding using frame pointers is unreliable as "not working"

pablogsal · 2025-07-10T12:13:44Z

@canova I am AFK do you mind checking in case we are lucky?

canova · 2025-07-10T12:13:44Z

Ah that's right. If we add a test with samply, then we might be able to run that test with and without frame pointers then.

canova · 2025-07-10T12:14:41Z

@canova I am AFK do you mind checking in case we are lucky?

Just did, and unfortunately, we are not lucky :') still fails.

canova · 2025-07-10T12:19:27Z

Added the same comment to the other PR, but adding here for posterity:

Tested this PR using samply. I can verify that it fixes the stack walking! Here are before and after profiles:
Before your patch / After your patch

pablogsal · 2025-07-10T12:37:17Z

!buildbot Perf

bedevere-bot · 2025-07-10T12:37:22Z

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit b174fe8 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136500%2Fmerge

The command will test the builders whose names match following regular expression: Perf

The builders matched are:

AMD64 Arch Linux Perf PR

mstange · 2025-07-10T12:58:44Z

Thank you for making this change!

If the Python C code is compiled with framepointers, I believe this change will also improve unwinding when you use Linux perf with framepointer unwinding (perf record -g); it should now give complete stacks, whereas in the past, I think the immediate caller of the trampoline (py_trampoline_evaluator) would have been missing from the stack.
If you use perf with DWARF unwinding, what does it do for JIT code without unwind info? Do you know if it falls back to using framepointers, the way samply does? If that were the case, I think you'd be able to simplify things drastically because you'd no longer have to emit unwind records to the jitdump file.

Oh, and one other thing I want to mention: samply supports both macOS and Linux, and this patch helps with both, so it's not strictly related to just #136459.

canova · 2025-07-10T13:02:49Z

Oh, and one other thing I want to mention: samply supports both macOS and Linux, and this patch helps with both, so it's not strictly related to just #136459.

The profiles that I captured were actually from Linux x86_64. I should have mentioned that too :)

pablogsal · 2025-07-10T15:26:37Z

If the Python C code is compiled with framepointers, I believe this change will also improve unwinding when you use Linux perf with framepointer unwinding (perf record -g); it should now give complete stacks, whereas in the past, I think the immediate caller of the trampoline (py_trampoline_evaluator) would have been missing from the stack.

It does not unfortunately. See #136500 (comment) and #136500 (review)

pablogsal · 2025-07-10T15:27:49Z

If you use perf with DWARF unwinding, what does it do for JIT code without unwind info? Do you know if it falls back to using framepointers, the way samply does? If that were the case, I think you'd be able to simplify things drastically because you'd no longer have to emit unwind records to the jitdump file.

it chokes on it and stops at that frame (like any other unwinder on the landscape) :S We are having that problem here for the big JIT and is a real pain: #126910

pablogsal · 2025-07-11T13:04:29Z

!buildbot Fedora Stable

bedevere-bot · 2025-07-11T13:04:33Z

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 8a81457 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136500%2Fmerge

The command will test the builders whose names match following regular expression: Fedora Stable

The builders matched are:

s390x Fedora Stable Clang PR
PPC64LE Fedora Stable Clang PR
aarch64 Fedora Stable LTO PR
PPC64LE Fedora Stable LTO + PGO PR
PPC64LE Fedora Stable PR
PPC64LE Fedora Stable LTO PR
aarch64 Fedora Stable Clang PR
AMD64 Fedora Stable LTO + PGO PR
aarch64 Fedora Stable PR
PPC64LE Fedora Stable Refleaks PR
s390x Fedora Stable PR
AMD64 Fedora Stable LTO PR
AMD64 Fedora Stable Clang PR
aarch64 Fedora Stable Clang Installed PR
aarch64 Fedora Stable LTO + PGO PR
AMD64 Fedora Stable PR
PPC64LE Fedora Stable Clang Installed PR
s390x Fedora Stable Clang Installed PR
AMD64 Fedora Stable Refleaks PR
AMD64 Fedora Stable Clang Installed PR
aarch64 Fedora Stable Refleaks PR
s390x Fedora Stable LTO PR
s390x Fedora Stable LTO + PGO PR
s390x Fedora Stable Refleaks PR

miss-islington-app · 2025-07-11T13:32:39Z

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

…nd aarch64 (pythonGH-136500) This commit fixes the following problems: * The x86_64 trampolines are not preserving frame pointers * The hardcoded offsets to the code segment from the FDE only worked properly for x64_64 * The CIE data was not following conventions of aarch64 * The eh_frame for aarch64 was not fully correct (cherry picked from commit 236f733) Co-authored-by: Pablo Galindo Salgado <[email protected]>

miss-islington-app · 2025-07-11T13:32:46Z

Sorry, @pablogsal, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 236f733d8ffb3d587e1167fa0a0248c24512e7fd 3.13

bedevere-app · 2025-07-11T13:32:48Z

GH-136544 is a backport of this pull request to the 3.14 branch.

…86_64 and aarch64 (pythonGH-136500) This commit fixes the following problems: * The x86_64 trampolines are not preserving frame pointers * The hardcoded offsets to the code segment from the FDE only worked properly for x64_64 * The CIE data was not following conventions of aarch64 * The eh_frame for aarch64 was not fully correct (cherry picked from commit 236f733) Co-authored-by: Pablo Galindo Salgado <[email protected]>

bedevere-app · 2025-07-11T13:40:38Z

GH-136545 is a backport of this pull request to the 3.13 branch.

…and aarch64 (GH-136500) (#136545) This commit fixes the following problems: * The x86_64 trampolines are not preserving frame pointers * The hardcoded offsets to the code segment from the FDE only worked properly for x64_64 * The CIE data was not following conventions of aarch64 * The eh_frame for aarch64 was not fully correct (cherry picked from commit 236f733)

…and aarch64 (GH-136500) (#136544) gh-136541: Fix several problems of perf trampolines in x86_64 and aarch64 (GH-136500) This commit fixes the following problems: * The x86_64 trampolines are not preserving frame pointers * The hardcoded offsets to the code segment from the FDE only worked properly for x64_64 * The CIE data was not following conventions of aarch64 * The eh_frame for aarch64 was not fully correct (cherry picked from commit 236f733) Co-authored-by: Pablo Galindo Salgado <[email protected]>

…nd aarch64 (python#136500) This commit fixes the following problems: * The x86_64 trampolines are not preserving frame pointers * The hardcoded offsets to the code segment from the FDE only worked properly for x64_64 * The CIE data was not following conventions of aarch64 * The eh_frame for aarch64 was not fully correct

…86_64 and aarch64 (pythonGH-136500) (python#136544) pythongh-136541: Fix several problems of perf trampolines in x86_64 and aarch64 (pythonGH-136500) This commit fixes the following problems: * The x86_64 trampolines are not preserving frame pointers * The hardcoded offsets to the code segment from the FDE only worked properly for x64_64 * The CIE data was not following conventions of aarch64 * The eh_frame for aarch64 was not fully correct (cherry picked from commit 236f733) Co-authored-by: Pablo Galindo Salgado <[email protected]>

pythongh-136459: Use frame pointers in the x86_64 perf trampolines
8b228c0

pablogsal requested review from brandtbucher, diegorusso and savannahostrowski as code owners July 10, 2025 10:43

bedevere-appbot added the awaiting core review label Jul 10, 2025

bedevere-appbot mentioned this pull request Jul 10, 2025
Consider enabling perf trampoline on macOS #136459
Closed

pablogsal mentioned this pull request Jul 10, 2025
gh-136459: Add perf trampoline support for macOS #136461
Merged

pablogsal added skip news 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Jul 10, 2025

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025

pablogsal force-pushed the gh-136459 branch from 5846993 to 3fdd2a7Compare July 10, 2025 11:11

Calculate the padding automatically
b174fe8

pablogsal force-pushed the gh-136459 branch from 3fdd2a7 to b174fe8Compare July 10, 2025 11:16

python deleted a comment from bedevere-bot Jul 10, 2025

canova reviewed Jul 10, 2025
View reviewed changes

pablogsal changed the title ~~gh-136459: Fix several problems of perf trampolines in x86_64 and aarch64~~gh-136541: Fix several problems of perf trampolines in x86_64 and aarch64Jul 11, 2025

bedevere-appbot mentioned this pull request Jul 11, 2025
perf trampolines are not reliable in aarch64 #136541
Closed

pablogsal added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Jul 11, 2025

python deleted a comment from bedevere-bot Jul 11, 2025

pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 11, 2025

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 11, 2025

python deleted a comment from bedevere-bot Jul 11, 2025

pablogsal merged commit 236f733 into python:mainJul 11, 2025
68 of 74 checks passed

pablogsal deleted the gh-136459 branch July 11, 2025 13:32

bedevere-appbot removed the awaiting core review label Jul 11, 2025

miss-islington-appbot assigned pablogsal Jul 11, 2025

bedevere-appbot removed the needs backport to 3.14 bugs and security fixes label Jul 11, 2025

bedevere-appbot removed the needs backport to 3.13 bugs and security fixes label Jul 11, 2025

	@unittest.skipUnless(
	is_unwinding_reliable_with_frame_pointers(),
	"Unwinding is unreliable with frame pointers",
	)

Uh oh!

gh-136541: Fix several problems of perf trampolines in x86_64 and aarch64#136500

gh-136541: Fix several problems of perf trampolines in x86_64 and aarch64 #136500

Uh oh!

Conversation

pablogsal commented Jul 10, 2025• edited by bedevere-app botLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Jul 10, 2025

Uh oh!

pablogsal commented Jul 10, 2025

Uh oh!

pablogsal commented Jul 10, 2025

Uh oh!

bedevere-bot commented Jul 10, 2025

Uh oh!

canova left a comment

Choose a reason for hiding this comment

Uh oh!

pablogsal commented Jul 10, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pablogsal commented Jul 10, 2025

Uh oh!

canova commented Jul 10, 2025

Uh oh!

canova commented Jul 10, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

canova commented Jul 10, 2025

Uh oh!

pablogsal commented Jul 10, 2025

Uh oh!

bedevere-bot commented Jul 10, 2025

Uh oh!

mstange commented Jul 10, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

canova commented Jul 10, 2025

Uh oh!

pablogsal commented Jul 10, 2025

Uh oh!

pablogsal commented Jul 10, 2025

Uh oh!

pablogsal commented Jul 11, 2025

Uh oh!

bedevere-bot commented Jul 11, 2025

Uh oh!

Uh oh!

miss-islington-appbot commented Jul 11, 2025

Uh oh!

miss-islington-appbot commented Jul 11, 2025

Uh oh!

bedevere-appbot commented Jul 11, 2025

Uh oh!

bedevere-appbot commented Jul 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pablogsal commented Jul 10, 2025•
edited by bedevere-app bot
Loading

pablogsal commented Jul 10, 2025•
edited
Loading

canova commented Jul 10, 2025•
edited
Loading

mstange commented Jul 10, 2025•
edited
Loading