| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Change the code field from PyMutex<PyRef<PyCode>> to PyAtomicRef<PyCode>, eliminating mutex lock/unlock on every function call. The setter uses swap_to_temporary_refs for safe deferred drop of the old code object.
📝 Walkthrough
WalkthroughThe pull request refactors PyFunction's code field from a mutex-protected reference to an atomic reference, eliminating lock-based access patterns across getters, setters, and JIT paths while maintaining thread-safe semantics. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agentsVerify each finding against the current code and only fix it if needed. Inline comments: In `@crates/vm/src/builtins/function.rs`: - Around line 682-684: When assigning a new code object in set___code__, also invalidate any cached JIT artifacts so the old machine code cannot be used; specifically, after self.code.swap_to_temporary_refs(...) and before/after self.func_version.store(0, Relaxed), clear the cached jitted_code (and any associated owner/version fields used by the JIT fast path) so invoke_with_locals cannot find/execute stale compiled code — update the fields that hold the compiled entry (jitted_code) to a neutral/empty state consistent with how they are checked in invoke_with_locals.
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between 0a6a6f8 and 5defc42.
📒 Files selected for processing (2)
Sorry, something went wrong.
| fn set___code__(&self, code: PyRef<PyCode>, vm: &VirtualMachine) { | ||
| self.code.swap_to_temporary_refs(code, vm); | ||
| self.func_version.store(0, Relaxed); |
There was a problem hiding this comment.
⚠️ Potential issue | 🟠 Major
Invalidate cached JIT code when __code__ is reassigned.
Line 683 swaps the code object, but the cached jitted_code is not reset. The JIT fast path in invoke_with_locals (Line 533 onward) still executes cached compiled code when present, so f.__code__ = ... can leave the function running stale machine code.
🤖 Prompt for AI AgentsVerify each finding against the current code and only fix it if needed. In `@crates/vm/src/builtins/function.rs` around lines 682 - 684, When assigning a new code object in set___code__, also invalidate any cached JIT artifacts so the old machine code cannot be used; specifically, after self.code.swap_to_temporary_refs(...) and before/after self.func_version.store(0, Relaxed), clear the cached jitted_code (and any associated owner/version fields used by the JIT fast path) so invoke_with_locals cannot find/execute stale compiled code — update the fields that hold the compiled entry (jitted_code) to a neutral/empty state consistent with how they are checked in invoke_with_locals.
Sorry, something went wrong.
…ustPython#7317) Change the code field from PyMutex<PyRef<PyCode>> to PyAtomicRef<PyCode>, eliminating mutex lock/unlock on every function call. The setter uses swap_to_temporary_refs for safe deferred drop of the old code object.
| Back | FazBrowse Home | New Git URL |
Change the code field from PyMutex<PyRef> to PyAtomicRef, eliminating mutex lock/unlock on every function call. The setter uses swap_to_temporary_refs for safe deferred drop of the old code object.
Summary by CodeRabbit