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-143421: Move `JitOptContext` from stack allocation to per-thread heap allocation to avoid stack overflow. by cocolato · Pull Request #143536 · python/cpython

Conversation

@cocolato

@cocolato cocolato commented Jan 8, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • Issue: JIT optimizer cleanups #143421
  • JIT: Implement unique reference tracking in Tier 2 for reference count optimizations #143414 (comment)

Sorry, something went wrong.

Uh oh!

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

All reactions
@cocolato
move JitOptContext to _PyThreadStateImpl
8b38039
@cocolato
cocolato requested review from Fidget-Spinner, ZeroIntensity, ericsnowcurrently, markshannon and tomasr8 as code owners January 8, 2026 03:22
@bedevere-app bedevere-app Bot added the awaiting review label Jan 8, 2026
@cocolato cocolato changed the title Move JitOptContext from stack allocation to per-thread heap allocation to avoid stack overflow. gh143421: Move JitOptContext from stack allocation to per-thread heap allocation to avoid stack overflow. Jan 8, 2026
@cocolato cocolato changed the title gh143421: Move JitOptContext from stack allocation to per-thread heap allocation to avoid stack overflow. gh-143421: Move JitOptContext from stack allocation to per-thread heap allocation to avoid stack overflow. Jan 8, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Jan 8, 2026
JIT optimizer cleanups #143421
Closed
@cocolato
remove redundant func parameter
9717629
Fidget-Spinner
Fidget-Spinner reviewed Jan 8, 2026
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.

Pretty close!

Sorry, something went wrong.

Uh oh!

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

cocolato reacted with heart emoji
All reactions
  • ❤️ 1 reaction
Comment thread Python/pystate.c Resolved

Uh oh!

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

Comment thread Python/optimizer_analysis.c Resolved

Uh oh!

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

Comment thread Include/internal/pycore_tstate.h Resolved

Uh oh!

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

@cocolato
add _PyJitOptState
4e7a918
@cocolato

cocolato commented Jan 8, 2026

Copy link
Copy Markdown
Member Author

Updated!

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner Fidget-Spinner added the skip news label Jan 8, 2026
Fidget-Spinner
Fidget-Spinner approved these changes Jan 8, 2026
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.

Great, thanks!

Sorry, something went wrong.

Uh oh!

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

cocolato reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Jan 8, 2026
kumaraditya303
kumaraditya303 reviewed Jan 8, 2026
View reviewed changes
Comment thread Include/internal/pycore_optimizer.h Resolved

Uh oh!

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

@cocolato @kumaraditya303
Update Include/internal/pycore_optimizer.h
f25f6c8
Co-authored-by: Kumar Aditya <[email protected]>
@markshannon

markshannon commented Jan 8, 2026

Copy link
Copy Markdown
Member

Can we avoid unnecessary pointer chasing by embedding the array in the jit state struct, and allocate the entire struct together instead of having part of it embedded in the thread state, and part in a new allocation?

ie. instead of

typedef struct _PyJitOptState {
    struct _JitOptContext *opt_context;
} _PyJitOptState;

typedef struct _PyJitTracerState {
    _PyUOpInstruction *code_buffer;
    _PyJitOptState opt_state;
    _PyJitTracerInitialState initial_state;
    _PyJitTracerPreviousState prev_state;
    _PyJitTracerTranslatorState translator_state;
} _PyJitTracerState;

we would have

typedef struct _PyJitTracerState {
    _PyJitTracerInitialState initial_state;
    _PyJitTracerPreviousState prev_state;
    _PyJitTracerTranslatorState translator_state;
     _JitOptContext opt_context;
    _PyUOpInstruction code_buffer[UOP_MAX_TRACE_LENGTH];
} _PyJitTracerState;

Doing so would also decouple the thread state header from the JIT headers, as only a pointer to an opaque struct would need to be declared in the thread state header.

I'm also concerned about the amount of allocation and freeing required to compile a small trace. Maybe allocate once and only free when the thread is destroyed?

cocolato 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.

@cocolato

cocolato commented Jan 8, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

we would have

typedef struct _PyJitTracerState {
    _PyJitTracerInitialState initial_state;
    _PyJitTracerPreviousState prev_state;
    _PyJitTracerTranslatorState translator_state;
     _JitOptContext opt_context;
    _PyUOpInstruction code_buffer[UOP_MAX_TRACE_LENGTH];
} _PyJitTracerSt

I can try implementing it this way, but I'd like to know if this should be done in a new PR/issue. Perhaps we could use benchmarks to determine potential performance issues from allocating _PyJitTracerState once.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner

Fidget-Spinner commented Jan 8, 2026

Copy link
Copy Markdown
Member

I'm also concerned about the amount of allocation and freeing required to compile a small trace. Maybe allocate once and only free when the thread is destroyed?

Isnt that already being done?

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner

Fidget-Spinner commented Jan 8, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

@cocolato I think we can apply Mark's changes, it means we would need no new allocations anymore as its all heap allocated with the _PyThreadStateImpl struct.

cocolato 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.

cocolato
cocolato commented Jan 8, 2026
View reviewed changes
Comment thread Include/internal/pycore_tstate.h Resolved

Uh oh!

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

@cocolato cocolato closed this Jan 8, 2026
@cocolato cocolato reopened this Jan 8, 2026
Fidget-Spinner
Fidget-Spinner reviewed Jan 8, 2026
View reviewed changes
Comment thread Python/pystate.c Resolved

Uh oh!

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

@cocolato
Embed JitOptContext and code_buffer in _PyThreadStateImpl
32dc7fb
@cocolato

cocolato commented Jan 8, 2026

Copy link
Copy Markdown
Member Author

I'm not sure if the current implementation of the new pycore_optimizer_types.h is correct. I'd appreciate any suggestions for improvements.

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 Jan 8, 2026
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.

Very cool, thank you!

Sorry, something went wrong.

Uh oh!

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

All reactions
@Fidget-Spinner
Fidget-Spinner merged commit aeb3403 into python:main Jan 8, 2026
62 checks passed

Uh oh!

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

@bedevere-app bedevere-app Bot removed the awaiting merge label Jan 8, 2026
@Fidget-Spinner

Fidget-Spinner commented Jan 9, 2026

Copy link
Copy Markdown
Member

@markshannon

This caused a 100% slowdown in bench_thread_pool benchmark https://doesjitgobrrr.com/run/2026-01-08

We need to lazily allocate it and pointer chase, otherwise we're slowing down significantly the spawning of threads.

All reactions

Sorry, something went wrong.

Uh oh!

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

Fidget-Spinner added a commit to Fidget-Spinner/cpython that referenced this pull request Jan 9, 2026
@Fidget-Spinner
Revert "pythongh-143421: Move JitOptContext from stack allocation t…
738696f
…o per-thread heap allocation (pythonGH-143536)"

This reverts commit aeb3403.
@cocolato

cocolato commented Jan 9, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

@markshannon

This caused a 100% slowdown in bench_thread_pool benchmark https://doesjitgobrrr.com/run/2026-01-08

We need to lazily allocate it and pointer chase, otherwise we're slowing down significantly the spawning of threads.

So should we revert to the first version?4e7a918

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner

Fidget-Spinner commented Jan 9, 2026

Copy link
Copy Markdown
Member

So should we revert to the first version?4e7a918

I will have a PR up.

cocolato reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

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

@cocolato

cocolato commented Jan 9, 2026

Copy link
Copy Markdown
Member Author

Sorry, I will run a local benchmark first before making important changes in the next time.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner

Fidget-Spinner commented Jan 9, 2026

Copy link
Copy Markdown
Member

Sorry, I will run a local benchmark first before making important changes in the next time.

No it's fine. That's the point of having a benchmark runner, to catch things like this! It's hard to predict what a change will affect, so that's the point of having benchark runners to save human time.

cocolato reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

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

@Fidget-Spinner

Fidget-Spinner commented Jan 9, 2026

Copy link
Copy Markdown
Member

PR up at #143597

All reactions

Sorry, something went wrong.

Uh oh!

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

@markshannon markshannon mentioned this pull request Jan 9, 2026
gh-143421: Lazily allocate tracer code and opt buffers #143597
Merged
reidenong pushed a commit to reidenong/cpython that referenced this pull request Jan 12, 2026
@cocolato @kumaraditya303 @reidenong
pythongh-143421: Move JitOptContext from stack allocation to per-th…
86e3e81
…read heap allocation (pythonGH-143536)

* move JitOptContext to _PyThreadStateImpl
* make _PyUOpInstruction buffer a part of _PyThreadStateImpl

Co-authored-by: Kumar Aditya <[email protected]>
@cocolato
cocolato deleted the feat/move_jitoptctx_to_pystate branch January 20, 2026 14:12
thunder-coding pushed a commit to thunder-coding/cpython that referenced this pull request Feb 15, 2026
@cocolato @kumaraditya303 @thunder-coding
pythongh-143421: Move JitOptContext from stack allocation to per-th…
d7bb693
…read heap allocation (pythonGH-143536)

* move JitOptContext to _PyThreadStateImpl
* make _PyUOpInstruction buffer a part of _PyThreadStateImpl

Co-authored-by: Kumar Aditya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 left review comments
@Fidget-Spinner Fidget-Spinner Fidget-Spinner approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@ZeroIntensity ZeroIntensity Awaiting requested review from ZeroIntensity ZeroIntensity is a code owner
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner
@tomasr8 tomasr8 Awaiting requested review from tomasr8 tomasr8 is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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.

4 participants

@cocolato @markshannon @Fidget-Spinner @kumaraditya303
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.