GH-133231: Changes to executor management to support proposed `sys._jit` module #133287

markshannon · 2025-05-02T10:08:08Z

This PR:

Tracks the current executor on the thread-state, not the previous one
Sets the current executor to NULL when entering PyEval_EvalDefault and resetting it when leaving.
Batch executors for deallocation to avoid having to constantly incref executors; this is an ad-hoc form of deferred reference counting.

Should simplify the implementation of sys._jit.is_enabled() as it is simply tstate->current_executor != NULL and doesn't require additional state to be tracked.

Issue: Add JIT utilities in sys._jit #133231

… Batch executors for deallocation to avoid having to constantly incref executors; this is an ad-hoc form of deferred reference counting.

markshannon · 2025-05-02T17:14:08Z

The msvc jit failure might be fixed by #132746

Fidget-Spinner · 2025-05-02T17:50:30Z

The msvc jit failure might be fixed by #132746

Wow I did not expect that to fix it.

brandtbucher

I think the overall approach is neat, but I think there are a couple of potential issues (and possible cleanups) here.

Long story short, anything that uses the entry_frame for this purpose is going to run into issues with the tail-calling interpreter (whether it's your approach, or mine). It's a hard problem.

brandtbucher · 2025-05-03T00:10:32Z

Python/ceval.c

+typedef struct {
+    _PyInterpreterFrame frame;
+    _PyStackRef stack[1];
+} _PyEntryFrame;


Is this needed? We only need one slot in localsplus, which _PyInterpreterFrame already has.

We now need two slots. One on the stack for the return value, as before, and one local variable to hold the current executor.

Ah, I see. Forgot about the return value.

Maybe add a comment that the one stack slot is for the return value, and the one local is for the executor.

Python/ceval.c

Python/optimizer.c

Python/bytecodes.c

Python/ceval.c

bedevere-app · 2025-05-03T03:10:13Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

brandtbucher · 2025-05-03T03:17:17Z

Idead: if we have a struct struct { PyExecutorObject *executor, struct node *prev} node;, we can stack allocate one of these in every GOTO_TIER_TWO call and restore it afterwards. Then keep the top on the tstate.

Something like this:

diff --git a/Python/ceval_macros.h b/Python/ceval_macros.h
index e1d2673848c..c77c2cafee7 100644
--- a/Python/ceval_macros.h
+++ b/Python/ceval_macros.h
@@ -360,10 +360,13 @@ do {                                                   \
     OPT_STAT_INC(traces_executed);                     \
     _PyExecutorObject *_executor = (EXECUTOR);         \
     jit_func jitted = _executor->jit_code;             \
+    node prev = tstate->current_executor;              \
+    tstate->current_executor = (node){_executor, &prev}; \
     /* Keep the shim frame alive via the executor: */  \
     Py_INCREF(_executor);                              \
     next_instr = jitted(frame, stack_pointer, tstate); \
     Py_DECREF(_executor);                              \
+    tstate->current_executor = prev;                   \
     Py_CLEAR(tstate->previous_executor);               \
     frame = tstate->current_frame;                     \
     stack_pointer = _PyFrame_GetStackPointer(frame);   \
@@ -379,6 +382,9 @@ do { \
     OPT_STAT_INC(traces_executed); \
     next_uop = (EXECUTOR)->trace; \
     assert(next_uop->opcode == _START_EXECUTOR); \
+    /* prev is declared local to the tier two interpreter: */ \
+    prev = tstate->current_executor; \
+    tstate->current_executor = (node){_executor, &prev}; \
     goto enter_tier_two; \
 } while (0)
 #endif
@@ -386,6 +392,7 @@ do { \
 #define GOTO_TIER_ONE(TARGET)                                         \
     do                                                                \
     {                                                                 \
+        tstate->current_executor = prev;                              \
         next_instr = (TARGET);                                        \
         OPT_HIST(trace_uop_execution_counter, trace_run_length_hist); \
         _PyFrame_SetStackPointer(frame, stack_pointer);               \

No entry frame needed.

markshannon · 2025-05-03T08:42:26Z

I don't see how that would work, as macros don't have scope. But its moot anyway as we can use the entry frame.

The entry frame exists in the tail-calling interpreter, it is the entry variable that we can't use. But we don't need to. Anywhere we return, either in INTERPRETER_EXIT or exit_unwind, the frame is the entry frame: frame == &entry.frame.

markshannon · 2025-05-03T10:53:39Z

I have made the requested changes; please review again

bedevere-app · 2025-05-03T10:53:44Z

Thanks for making the requested changes!

@brandtbucher: please review the changes made to this pull request.

brandtbucher · 2025-05-03T16:56:30Z

I don't see how that would work, as macros don't have scope.

I mean, it would just be a local declared in the _PyEval_EvalFrameDefault itself for the tail-calling interpreter, and would become local to whatever bytecode is using the macro on other builds.

But as you say...

But its moot anyway as we can use the entry frame.

The entry frame exists in the tail-calling interpreter, it is the entry variable that we can't use. But we don't need to. Anywhere we return, either in INTERPRETER_EXIT or exit_unwind, the frame is the entry frame: frame == &entry.frame.

Ah, this is what I missed. Makes sense: we're always able to set it when entering because we have entry handy. And we're always able to reset it when exiting because it's just the current frame. Thanks for explaining.

brandtbucher

Looks good, thanks. I'm just going to kick off some JIT benchmarks to be safe.

brandtbucher · 2025-05-03T17:01:16Z

Python/ceval_macros.h

        next_instr = (TARGET);                                        \
+        assert(tstate->current_executor == NULL);                     \


Minor: this seems redundant, given that we're setting it one line above.

brandtbucher · 2025-05-03T17:03:38Z

Tools/jit/template.c

    __attribute__((musttail)) return jitted(frame, stack_pointer, tstate); \
 } while (0)

 #undef GOTO_TIER_ONE
 #define GOTO_TIER_ONE(TARGET)                       \
 do {                                                \
+    tstate->current_executor = NULL;                \


You can either do this here, or in the second half of the _Py_JIT implementation of GOTO_TIER_TWO in ceval_macros.h, after the JIT code returns. No need to do it in both places.

brandtbucher · 2025-05-03T17:08:54Z

Python/bytecodes.c

@@ -5316,7 +5322,6 @@ dummy_func(
        }

        tier2 op(_START_EXECUTOR, (executor/4 --)) {


Side note, this instruction now exists solely to set the current_executor in the tier two interpreter. We should try to remove it as a follow-up (doesn't need to be in this PR).

We can replace all uses of current_executor with an operand, which will remove the current_executor local variable from the tier 2 interpreter and the _JIT_EXECUTOR patch from the JIT template.

We can also shrink _CHECK_VALIDITY a bit by replacing current_executor->vm_data.valid with *validity_ptr:

tier2 op(_CHECK_VALIDITY, (validity_ptr/4 --)) { DEOPT_IF(*validity_ptr == 0); }

For another PR.

brandtbucher · 2025-05-04T00:01:10Z

Looks fine.

* origin/main: (111 commits) pythongh-91048: Add filename and line number to external inspection routines (pythonGH-133385) pythongh-131178: Add tests for `ast` command-line interface (python#133329) Regenerate pcbuild.sln in Visual Studio 2022 (python#133394) pythongh-133042: disable HACL* HMAC on Emscripten (python#133064) pythongh-133351: Fix remote PDB's multi-line block tab completion (python#133387) pythongh-109700: Improve stress tests for interpreter creation (pythonGH-109946) pythongh-81793: Skip tests for os.link() to symlink on Android (pythonGH-133388) pythongh-126835: Rename `ast_opt.c` to `ast_preprocess.c` and related stuff after moving const folding to the peephole optimizier (python#131830) pythongh-91048: Relax test_async_global_awaited_by to fix flakyness (python#133368) pythongh-132457: make staticmethod and classmethod generic (python#132460) pythongh-132805: annotationlib: Fix handling of non-constant values in FORWARDREF (python#132812) pythongh-132426: Add get_annotate_from_class_namespace replacing get_annotate_function (python#132490) pythongh-81793: Always call linkat() from os.link(), if available (pythonGH-132517) pythongh-122559: Synchronize C and Python implementation of the io module about pickling (pythonGH-122628) pythongh-69605: Add PyREPL import autocomplete feature to 'What's New' (python#133358) bpo-44172: Keep reference to original window in curses subwindow objects (pythonGH-26226) pythonGH-133231: Changes to executor management to support proposed `sys._jit` module (pythonGH-133287) pythongh-133363: Fix Cmd completion for lines beginning with `! ` (python#133364) pythongh-132983: Introduce `_zstd` bindings module (pythonGH-133027) pythonGH-91048: Add utils for printing the call stack for asyncio tasks (python#133284) ...

Track the current executor on the thread-state, not the previous one.…

3a2ccda

… Batch executors for deallocation to avoid having to constantly incref executors; this is an ad-hoc form of deferred reference counting.

markshannon requested a review from brandtbucher May 2, 2025 10:08

markshannon requested a review from ericsnowcurrently as a code owner May 2, 2025 10:08

bedevere-app bot mentioned this pull request May 2, 2025

Add JIT utilities in sys._jit #133231

Closed

bedevere-app bot added the awaiting core review label May 2, 2025

Handle tail-calling interpreter

46ef280

markshannon added the skip news label May 2, 2025

Merge branch 'main' into current-executor

01ecf18

Merge branch 'main' into current-executor

89e4668

brandtbucher requested changes May 3, 2025

View reviewed changes

bedevere-app bot added awaiting changes and removed awaiting core review labels May 3, 2025

Don't use entry.frame in tailcalling interpreter

2f42cb0

markshannon added 2 commits May 3, 2025 09:57

Address review comments

f938d68

Fix compiler warning

a30e444

markshannon requested a review from savannahostrowski as a code owner May 3, 2025 09:06

bedevere-app bot added awaiting change review and removed awaiting changes labels May 3, 2025

bedevere-app bot requested a review from brandtbucher May 3, 2025 10:53

brandtbucher approved these changes May 3, 2025

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting change review labels May 3, 2025

Remove redundant code

0b9dcb7

markshannon merged commit ac7d5ba into python:main May 4, 2025
71 checks passed

bedevere-app bot removed the awaiting merge label May 4, 2025

markshannon deleted the current-executor branch May 5, 2025 11:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-133231: Changes to executor management to support proposed `sys._jit` module #133287

GH-133231: Changes to executor management to support proposed `sys._jit` module #133287

markshannon commented May 2, 2025 •

edited by bedevere-app bot

Loading

markshannon commented May 2, 2025

Fidget-Spinner commented May 2, 2025

brandtbucher left a comment

brandtbucher May 3, 2025

markshannon May 3, 2025 •

edited

Loading

brandtbucher May 3, 2025

bedevere-app bot commented May 3, 2025

brandtbucher commented May 3, 2025

markshannon commented May 3, 2025

markshannon commented May 3, 2025

bedevere-app bot commented May 3, 2025

brandtbucher commented May 3, 2025

brandtbucher left a comment

brandtbucher May 3, 2025

brandtbucher May 3, 2025

brandtbucher May 3, 2025 •

edited

Loading

markshannon May 4, 2025

brandtbucher commented May 4, 2025

		next_instr = (TARGET); \
		assert(tstate->current_executor == NULL); \

		@@ -5316,7 +5322,6 @@ dummy_func(
		}

		tier2 op(_START_EXECUTOR, (executor/4 --)) {

GH-133231: Changes to executor management to support proposed sys._jit module #133287

GH-133231: Changes to executor management to support proposed sys._jit module #133287

Conversation

markshannon commented May 2, 2025 • edited by bedevere-app bot Loading

markshannon commented May 2, 2025

Fidget-Spinner commented May 2, 2025

brandtbucher left a comment

Choose a reason for hiding this comment

brandtbucher May 3, 2025

Choose a reason for hiding this comment

markshannon May 3, 2025 • edited Loading

Choose a reason for hiding this comment

brandtbucher May 3, 2025

Choose a reason for hiding this comment

bedevere-app bot commented May 3, 2025

brandtbucher commented May 3, 2025

markshannon commented May 3, 2025

markshannon commented May 3, 2025

bedevere-app bot commented May 3, 2025

brandtbucher commented May 3, 2025

brandtbucher left a comment

Choose a reason for hiding this comment

brandtbucher May 3, 2025

Choose a reason for hiding this comment

brandtbucher May 3, 2025

Choose a reason for hiding this comment

brandtbucher May 3, 2025 • edited Loading

Choose a reason for hiding this comment

markshannon May 4, 2025

Choose a reason for hiding this comment

brandtbucher commented May 4, 2025

GH-133231: Changes to executor management to support proposed `sys._jit` module #133287

GH-133231: Changes to executor management to support proposed `sys._jit` module #133287

markshannon commented May 2, 2025 •

edited by bedevere-app bot

Loading

markshannon May 3, 2025 •

edited

Loading

brandtbucher May 3, 2025 •

edited

Loading