GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Align specialization guards and caching with CPython by youknowone · Pull Request #7341 · RustPython/RustPython Conversation
| ||||||||||||||||||||||||||||||||||||||||||||
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors adaptive counters into out-of-line bit‑encoded AtomicU16 storage and updates CodeUnits layout/APIs; routes JIT compilation over cleaned bytecode bytes; adds dict hint/get_hint fast‑path helpers; adds PyBoundMethod accessors and JIT guards; tightens tuple index bounds handling and expands instrumentation trace detection. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
d1dc3d6 to
2d7c2f7
Compare
c03189d to
6d096fe
Compare
github-actions
Bot
commented
Mar 4, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] lib: cpython/Lib/pickle.py dependencies:
dependent tests: (97 tests)
Legend:
|
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
- Disable ForIterGen specialization (falls through to generic path) because inline generator frame resumption is needed for correct debugger StopIteration visibility (test_bdb) - Use downcast_ref_if_exact for PyNativeFunction in CALL specialization guards - Add can_specialize_call guard for class __init__ specialization - Remove expectedFailure for test_bad_newobj_args (now passing)
6d096fe to
d464119
Compare
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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/vm/src/dict_inner.rs`:
- Around line 366-374: Snapshot the entry's key and value while holding the dict
read guard, drop the guard, then perform equality; specifically, in the block
that uses self.read() and inner.entries.get(hint) (and currently calls
key.key_is and key.key_eq on entry.key), clone/copy the entry.key and
entry.value out of the inner guard, release the guard, and then run
key.key_is(&cloned_key) and only if needed call key.key_eq(vm, &cloned_key) to
decide the result and return the cloned value; this avoids invoking key_eq while
holding the read lock.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 1b01ab44-3238-428f-8cd8-393e5623cbb5
⛔ Files ignored due to path filters (1)
Lib/test/test_pickle.pyis excluded by!Lib/**
📒 Files selected for processing (8)
crates/compiler-core/src/bytecode.rscrates/jit/src/instructions.rscrates/vm/src/builtins/dict.rscrates/vm/src/builtins/function.rscrates/vm/src/builtins/tuple.rscrates/vm/src/dict_inner.rscrates/vm/src/frame.rscrates/vm/src/stdlib/sys/monitoring.rs
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Uh oh!
There was an error while loading. Please reload this page.
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d464119937
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| builtins: PyObjectRef, | ||
| closure: &[PyCellRef], | ||
| func_obj: Option<PyObjectRef>, |
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.
Restore Frame API compatibility for callers
This change drops the use_datastack parameter from Frame::new (and removes related frame-local APIs) without updating the existing call sites, so the crate no longer builds: cargo check -p rustpython-vm fails with unresolved methods/arity mismatches in builtins/function.rs, builtins/frame.rs, builtins/super.rs, and vm/mod.rs. As written, the commit is build-blocking and cannot be integrated until the callers are updated or compatibility shims are restored.
Useful? React with 👍 / 👎.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Introduce FastLocalsData enum with Heap and DataStack variants so non-generator/coroutine frames allocate localsplus on the VM datastack instead of the heap. Includes materialize_to_heap for migration when needed (e.g. generator suspension).
968372a to
0f044d6
Compare
Move key_eq call outside the read lock guard to avoid potential deadlock when Python __eq__ re-enters dict mutation paths. Matches the existing pattern in lookup().
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc3fd09611
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| @@ -2805,7 +2828,7 @@ impl ExecutingFrame<'_> { | |||
| Ok(None) | |||
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.
Treat ResumeCheck as non-traceable at frame entry
By dispatching Instruction::ResumeCheck through the normal execution path here, functions whose first opcode is RESUME_CHECK now run, but the trace-event filters in run() still only skip Resume/InstrumentedResume (see the matches! checks around line and opcode tracing in crates/vm/src/frame.rs). That causes extra line/opcode callbacks at function entry for RESUME_CHECK code objects (for example, a code object with the first opcode patched from RESUME to RESUME_CHECK emits an extra event on the def line), which breaks tracing parity with the existing RESUME path.
Useful? React with 👍 / 👎.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Uh oh!
There was an error while loading. Please reload this page.
* vm: complete specialized opcode dispatch paths * vm: cache LOAD_GLOBAL with dict entry hints * vm: align adaptive specialization counters with CPython backoff * vm: apply cooldown counter on specialization success paths * vm: retain LOAD_GLOBAL specializations on misses * vm: keep attr and call specializations on guard misses * vm: retain store-attr and store-subscr specializations on misses * vm: retain specialization opcodes on generic fallback paths * vm: align jump-backward specialization defaults with CPython * vm: retain exact-args call specializations on misses * vm: retain SEND_GEN specialization on non-coroutine sends * vm: specialize list.append calls like CPython CALL_LIST_APPEND * vm: set cooldown on LOAD_ATTR_CLASS specialization * vm: specialize bound method object CALL paths * vm: specialize CALL_KW for bound method objects * vm: use current-state function version for CALL_KW specialization * vm: align CALL/CALL_KW pyfunction specialization with CPython * vm: drop call-site identity caches in generic CALL specializations * vm: align builtin type call specializations with CPython guards * vm: align builtin CALL guards with CPython self_or_null semantics * vm: require exact list in CALL_LIST_APPEND fast path * vm: align CALL builtin/class specialization flow with CPython * vm: tighten len/isinstance CALL specializations to builtin guards * vm: gate CALL_BUILTIN_CLASS on type vectorcall like CPython * vm: run non-py CALL specializations via direct vectorcall * vm: align class-call specialization branching with CPython * Fix CI: disable ForIterGen, tighten CALL guards - Disable ForIterGen specialization (falls through to generic path) because inline generator frame resumption is needed for correct debugger StopIteration visibility (test_bdb) - Use downcast_ref_if_exact for PyNativeFunction in CALL specialization guards - Add can_specialize_call guard for class __init__ specialization - Remove expectedFailure for test_bad_newobj_args (now passing)
* vm: complete specialized opcode dispatch paths * vm: cache LOAD_GLOBAL with dict entry hints * vm: align adaptive specialization counters with CPython backoff * vm: apply cooldown counter on specialization success paths * vm: retain LOAD_GLOBAL specializations on misses * vm: keep attr and call specializations on guard misses * vm: retain store-attr and store-subscr specializations on misses * vm: retain specialization opcodes on generic fallback paths * vm: align jump-backward specialization defaults with CPython * vm: retain exact-args call specializations on misses * vm: retain SEND_GEN specialization on non-coroutine sends * vm: specialize list.append calls like CPython CALL_LIST_APPEND * vm: set cooldown on LOAD_ATTR_CLASS specialization * vm: specialize bound method object CALL paths * vm: specialize CALL_KW for bound method objects * vm: use current-state function version for CALL_KW specialization * vm: align CALL/CALL_KW pyfunction specialization with CPython * vm: drop call-site identity caches in generic CALL specializations * vm: align builtin type call specializations with CPython guards * vm: align builtin CALL guards with CPython self_or_null semantics * vm: require exact list in CALL_LIST_APPEND fast path * vm: align CALL builtin/class specialization flow with CPython * vm: tighten len/isinstance CALL specializations to builtin guards * vm: gate CALL_BUILTIN_CLASS on type vectorcall like CPython * vm: run non-py CALL specializations via direct vectorcall * vm: align class-call specialization branching with CPython * Fix CI: disable ForIterGen, tighten CALL guards - Disable ForIterGen specialization (falls through to generic path) because inline generator frame resumption is needed for correct debugger StopIteration visibility (test_bdb) - Use downcast_ref_if_exact for PyNativeFunction in CALL specialization guards - Add can_specialize_call guard for class __init__ specialization - Remove expectedFailure for test_bad_newobj_args (now passing)
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
1 participant
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.