Skip to content

Keep monitoring dispatch and event metadata interpreter-local - #9439

Draft
youknowdot wants to merge 8 commits into
RustPython:mainfrom
youknowdot:youknowdot/free-threading-remaining-batch-315-20261028
Draft

youknowdot wants to merge 8 commits into
RustPython:mainfrom
youknowdot:youknowdot/free-threading-remaining-batch-315-20261028

Conversation

@youknowdot

@youknowdot youknowdot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Keep monitoring dispatch and layer state interpreter-local without rewriting shared bytecode into monitoring wrappers.

The metadata substrate no longer depends on #9450: use byte-width atomic instruction reads and a bounded base-family walk that skips inline caches, rather than a permanent construction-provenance copy. Remove the new external-bytecode rejection/trust framework and frozen/import fallibility. Preserve existing constructor, code.replace, and marshal admission behavior, ordinary owning-code normalization, and literal GET_ITER construction order.

This follows CPython's base-operation/deoptimization conventions, not a claim to port its critical-section or thread-local bytecode machinery. Existing generic internal Clone/cache safety is outside this correction. The current-PC legacy trace path avoids an unnecessary prefix scan; arbitrary-offset metadata lookups remain bounded linear scans.

Verification

The final correction is byte-for-byte identical across all 12 changed paths to the locally validated source on b85582d. Main 698c196 was merged normally afterward; its seven SQLite/overlapped/regex-related paths do not overlap this correction. No fresh runtime pass is claimed for that merged base; current-head CI remains pending.

  • Scoped compiler-core (--lib --tests) and VM (--lib, compiler/threading/host_env/stdio/doc/importlib) check and strict Clippy passed on the identical monitoring correction before the disjoint current-main dictionary update.
  • On base b85582d (before the latest main integration), guarded fixture and CLI builds passed. An initial missing -lffi linker name was resolved using an explicit local link to the existing system libffi; no repository or system-library modification.
  • Three exact scalar reader/walker/cache-width tests passed.
  • Eleven selected canonical methods passed on both RustPython and pinned CPython 3.15 d220409a, with zero skips, failures, or expected failures: code replacement; marshal code/filename behavior; coroutine/generator methods; LINE/INSTRUCTION installation order; simple/loop line events; basic legacy tracing.
  • Two valid-input first-read normalization orders passed in each engine: warm fresh distinct code objects without reading subject co_code, enable both monitoring events, first-read normalized bytes, replace/marshal round trips, execute ordinary copies. Comparisons are within each engine, not physical bytecode equivalence across engines.

Full workspace/C-API suites, broad monitoring/free-threading suites, concurrency/GC/stress probes, and performance validation were not run. The reviewer's performance-before-merge request remains open; this correction does not claim to resolve overall dispatch overhead. No merge readiness claim.

AI assistance: OpenAI Codex assisted source audit, implementation, scoped validation, and this description; model version unavailable.

Serialize syslog masks and mmap cursor operations, preserve distinct mmap sequence bytes, and migrate ten unchanged CPython 3.15 concurrency suites after their native prerequisites.

Assisted-by: OpenAI Codex: model version unavailable
@coderabbitai

coderabbitai Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] test: cpython/Lib/test/test_free_threading

dependencies:

dependent tests: (no tests depend on free_threading)

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

Assisted-by: OpenAI Codex: model version unavailable
@codspeed

codspeed Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will degrade performance by 18.7%

1 warning

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

❌ 24 regressed benchmarks
✅ 38 untouched benchmarks
⏩ 4 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ nbody.py[rustpython] 1.3 s 1.7 s -25.08%
❌ generators.py[rustpython] 4.3 s 5.6 s -24.31%
❌ rustpython[comprehension_list.py] 847.8 µs 1,103.8 µs -23.19%
❌ nqueens.py[rustpython] 4.2 s 5.4 s -22.39%
❌ rustpython[20000] 2 s 2.6 s -21.27%
❌ rustpython[30000] 3 s 3.9 s -21.26%
❌ rustpython[10000] 1 s 1.3 s -21.05%
❌ richards.py[rustpython] 2.3 s 2.9 s -20.66%
❌ rustpython[strings.py] 40.4 ms 50.4 ms -19.74%
❌ fannkuch.py[rustpython] 16.1 s 20.1 s -19.67%
❌ rustpython[cmp.py] 67.6 ms 84.1 ms -19.54%
❌ rustpython[loop_string.py] 838.6 µs 1,039.4 µs -19.32%
❌ float.py[rustpython] 315.8 ms 388.9 ms -18.8%
❌ unpickle.py[rustpython] 182.8 ms 221.6 ms -17.52%
❌ decimal_factorial.py[rustpython] 1.1 s 1.3 s -16.87%
❌ rustpython[loop_append.py] 2.2 ms 2.6 ms -16.85%
❌ rustpython[comprehension_set.py] 1.3 ms 1.6 ms -16.59%
❌ deepcopy.py[rustpython] 133.5 ms 159.6 ms -16.34%
❌ pickle.py[rustpython] 639 ms 762.8 ms -16.24%
❌ deltablue.py[rustpython] 241.2 ms 286.4 ms -15.8%
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing youknowdot:youknowdot/free-threading-remaining-batch-315-20261028 (f26f073) with main (84d7afc)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

codex added 2 commits October 29, 2026 07:04
Use immutable source metadata and native code identity for virtual monitoring layers; preserve callback continuations and ordinary branch event sites.

Assisted-by: OpenAI Codex: model version unavailable
Assisted-by: OpenAI Codex: model version unavailable
@youknowone

Copy link
Copy Markdown
Member

hmm, let's check performance before merge

@codspeedbot explain performance drop

@codspeed

codspeed Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

hmm, let's check performance before merge

@codspeedbot explain performance drop

Why performance dropped

This is a real regression, not noise. All of it comes from f8a7823 ("Keep monitoring dispatch in interpreter-local state"):

  • The run for c672599 (dict/CSV/mmap/syslog changes) matches main.
  • Comparing c672599 → f8a7823 shows the same 24 regressions (−10% to −25%) as the PR report.
  • The regressions cover every interpreter-heavy benchmark (nbody, generators, pystone, richards, fannkuch, the microbenchmarks). That pattern points to a cost paid on every bytecode instruction.

What the flamegraph shows (nbody.py, Simulation)

c672599 f8a7823
Interpreter loop self time (run_until → run_until_inner) 366.6 ms 534.0 ms
monitoring::dispatch_operation self time — 175.8 ms
mem::replace<Option<u32>> (inlined) — 39.3 ms
Total 1.3 s 1.7 s

The float, list and call code costs the same in both runs (e.g. default_dealloc<PyFloat> 67.9 → 67.2 ms, PyFloat::into_ref 31.7 → 31.8 ms). The added time is all overhead in the dispatch loop, even though no monitoring tool is active.

Where it comes from in the code

  1. A function call before every instruction (crates/vm/src/frame.rs:3909):

    let mut virtual_dispatch = if PREPARE_GENERATOR { None }
        else { monitoring::dispatch_operation(vm, self.code, idx) };

    dispatch_operation (sys/monitoring.rs:718) is not inlined. Even its fast path (vm.state.monitoring_events.load() == 0) costs a call, a pointer chase through vm.state and an atomic load per instruction. It also returns an Option<(MonitoringInstruction, Instruction)> that the loop then branches on. On main, the loop goes straight to read_unit_from. This adds extra live values in the hottest loop, which also explains why the loop's own self time grew by ~45%.

  2. Extra writes per instruction in execute_instruction (frame.rs:4665-4666):

    let index = self.lasti().saturating_sub(1);
    self.monitoring_previous = self.monitoring_current.replace(Some(index));

    This re-loads lasti, even though the loop already holds idx, and writes two Option<u32> frame fields on every instruction, monitored or not.

  3. Smaller additions: the virtual_line/legacy_line_processed checks and the DispatchMode/caches tuple on the decode path. Each one is cheap, but they all sit in the loop.

Ways to recover it

  • Make the disabled case close to free. Split dispatch_operation into an #[inline(always)] check on monitoring_events == 0 and a #[cold] #[inline(never)] slow path. Better still, follow CPython: check instrumentation only at RESUME, backward jumps or the eval breaker, and keep a frame-local "monitored" flag. The plain path would then match main.
  • Track monitoring_previous/monitoring_current only in monitored mode, and take the index from idx instead of re-loading lasti. If the generator-close LINE fix (f1c31d7) needs the previous position in the normal path too, it could be saved only when a frame suspends instead of on every instruction.
  • Re-run the benchmarks after either change. A plain path that matches main should bring these benchmarks back to their baseline.

I haven't changed any code on the branch. Comment @codspeedbot try making the monitoring fast path zero-cost when no tool is active and I'll prototype the fix and benchmark it.

Summary
  • Warming up the performance wizard...
  • Isolate the commit causing the regression — c672599 (dict/CSV) is flat vs main; all 24 regressions appear at f8a7823 (monitoring dispatch).
  • Compare flamegraphs to find hot spots — New per-instruction work in the dispatch loop: out-of-line monitoring::dispatch_operation call plus monitoring_previous/current bookkeeping.
  • Report findings — Report findings
  • Handling comment — Comment handling finished successfully

Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.

codex added 2 commits October 29, 2026 10:43
Preserve original RustPython#9439 ancestry and the reviewed float-power integration while preparing its functionality split.

Assisted-by: OpenAI Codex: model version unavailable
Move dict, CSV, mmap, syslog and library-only modules into their coherent independent units while preserving complete monitoring source and its whole canonical test. Keep the provenance overlap explicit until its independent PR lands.

Assisted-by: OpenAI Codex: model version unavailable
@youknowdot youknowdot changed the title Update remaining free-threading Lib tests with native prerequisites Keep monitoring dispatch and event metadata interpreter-local Oct 29, 2026
youknowone pushed a commit that referenced this pull request Oct 29, 2026
Extract the six canonical library-only modules from #9439. Regenerated with scripts/update_lib from CPython d220409a13b77de7a6c30abe16ea90db01528edc.

Assisted-by: OpenAI Codex: model version unavailable

Co-authored-by: Codex <[email protected]>
youknowone pushed a commit that referenced this pull request Oct 29, 2026
Extract the syslog lock and whole canonical free-threading test from #9439. Regenerate the test with scripts/update_lib from CPython d220409a13b77de7a6c30abe16ea90db01528edc.

Assisted-by: OpenAI Codex: model version unavailable

Co-authored-by: Codex <[email protected]>
youknowone pushed a commit that referenced this pull request Oct 29, 2026
Extract mmap cursor and sequence behavior plus its whole canonical free-threading test from #9439. Regenerate the test with scripts/update_lib from CPython d220409a13b77de7a6c30abe16ea90db01528edc.

Assisted-by: OpenAI Codex: model version unavailable

Co-authored-by: Codex <[email protected]>
youknowone pushed a commit that referenced this pull request Oct 29, 2026
Extract CSV reader synchronization with its PyOperation facade and float-error helper from #9439. Include the whole canonical free-threading CSV test regenerated by scripts/update_lib from CPython d220409a13b77de7a6c30abe16ea90db01528edc.

Assisted-by: OpenAI Codex: model version unavailable

Co-authored-by: Codex <[email protected]>
youknowone pushed a commit that referenced this pull request Oct 29, 2026
Extract dictionary insertion and setdefault re-probing from #9439 with its five structural fixtures and whole canonical free-threading test. Regenerate the Lib file with scripts/update_lib from CPython d220409a13b77de7a6c30abe16ea90db01528edc.

Assisted-by: OpenAI Codex: model version unavailable

Co-authored-by: Codex <[email protected]>
codex added 2 commits October 29, 2026 19:54
Assisted-by: OpenAI Codex: model version unavailable
Use byte-width instruction reads and bounded base-family traversal. Remove the external-bytecode trust and frozen import policy changes from this monitoring patch.

Assisted-by: OpenAI Codex: model version unavailable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants