GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-137959: Replace shim code in jitted code with a single trampoline function. by markshannon · Pull Request #137961 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Aug 19, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Call JIT code from the interpreter via a trampoline function, instead of inserting shim code in the jitted code #137959

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

auvipy reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
markshannon added 4 commits August 19, 2025 15:19
@markshannon
JIT compile a trampoline function to avoid shim code
0dd2c49
@markshannon
Remove old shim code
a98b1c8
@markshannon
Fix whitespace
26b2856
@markshannon
Add trampoline source
5b6334e
@bedevere-app bedevere-app Bot mentioned this pull request Aug 19, 2025
Call JIT code from the interpreter via a trampoline function, instead of inserting shim code in the jitted code #137959
Closed
@markshannon
Add news
14080cb
@markshannon
markshannon requested a review from brandtbucher August 19, 2025 15:09
@markshannon
Remove redundant code
ca9663a
@markshannon
markshannon marked this pull request as ready for review August 19, 2025 16:24
@markshannon
markshannon requested review from ZeroIntensity, diegorusso, ericsnowcurrently and savannahostrowski as code owners August 19, 2025 16:24
@bedevere-app bedevere-app Bot added the awaiting core review label Aug 19, 2025
@markshannon

markshannon commented Aug 20, 2025

Copy link
Copy Markdown
Member Author

Benchmark results show a hint of a speedup, but in the noise. No slowdown, though.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Fidget-Spinner
Fidget-Spinner approved these changes Aug 20, 2025
View reviewed changes

@Fidget-Spinner Fidget-Spinner left a comment

Copy link
Copy Markdown
Member

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.

Some things I observed. Correct me if I'm wrong:

  1. This actually splits out the tier 2 interpreter from the tier 1 interpreter. Which is likely a significant saving for our tier 2 debug build time and tier 2 interpreter. Nice!
  2. The shim/trampoline is only compiled once lazily and reused across the whole runtime. Then reused across all JIT functions. We currently do two jumps (one into the shim/trampoline, one into the exec->jit_code). You need to lock this with a mutex in case multiple threads try to compile the shim at the same time.

If my understanding above is correct, then this is alright to me.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Python/jit.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Python/jit.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Aug 20, 2025
@markshannon

markshannon commented Aug 21, 2025

Copy link
Copy Markdown
Member Author

Some things I observed. Correct me if I'm wrong:

  1. This actually splits out the tier 2 interpreter from the tier 1 interpreter. Which is likely a significant saving for our tier 2 debug build time and tier 2 interpreter. Nice!
  2. The shim/trampoline is only compiled once lazily and reused across the whole runtime. Then reused across all JIT functions. We currently do two jumps (one into the shim/trampoline, one into the exec->jit_code). You need to lock this with a mutex in case multiple threads try to compile the shim at the same time.

If my understanding above is correct, then this is alright to me.

That is all correct. I'll add a comment explaining it.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@markshannon
Add clarifying comment
f9279af
@markshannon
markshannon merged commit a8d9d94 into python:main Aug 21, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Aug 21, 2025
@brandtbucher

brandtbucher commented Aug 21, 2025

Copy link
Copy Markdown
Member

Nice, sorry I wasn't able to review in time, but it looks mostly good (love the unification of the two JIT execution engines).

Two notes:

  • I prefer the old name "shim" throughout to the new name "trampoline". Trampoline already has another meaning in the JIT compiler, and we renamed this to "shim" a while back to make the distinction clearer.
  • I don't love that we leak the shim code when the runtime is finalized. We should free it in Py_Finalize or whatever.
diegorusso reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Oct 18, 2025
GH-137959: fix warning 'visibility' attribute ignored in jit/trampoline.c #140286
Closed
JIT: Fix compiler warning from visibility attribute in typedef #139981
Merged
This was referenced Nov 13, 2025
gh-137959: Fix TIER1_TO_TIER2 macro name in JIT InternalDocs #141496
Merged
[3.14] gh-140104: Set next_instr properly in the JIT during exceptions (GH-140233) #141495
Merged
This was referenced Dec 19, 2025
JIT: Rename trampoline.c to shim.c #142974
Merged
JIT: don't leak shim memory when shutting down the interpreter #142984
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes
@brandtbucher brandtbucher Awaiting requested review from brandtbucher brandtbucher is a code owner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@ZeroIntensity ZeroIntensity Awaiting requested review from ZeroIntensity ZeroIntensity is a code owner
@savannahostrowski savannahostrowski Awaiting requested review from savannahostrowski
@diegorusso diegorusso Awaiting requested review from diegorusso diegorusso is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@markshannon @brandtbucher @Fidget-Spinner
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.