Skip to content

Commit b537519

Browse files
committed
Also check globals is exact dict for LOAD_GLOBAL fast path
get_chain_exact bypasses __missing__ on dict subclasses. Move get_chain_exact to PyExact<PyDict> impl with debug_assert, and have get_chain delegate to it. Store builtins_dict as Option<&PyExact<PyDict>> to enforce exact type at compile time. Use PyRangeIterator::next_fast() instead of pub(crate) fields. Fix comment style issues.
1 parent 5cb18ed commit b537519

File tree

4 files changed

+53
-24
lines changed

4 files changed

+53
-24
lines changed

crates/vm/src/builtins/dict.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use super::{
55
use crate::common::lock::LazyLock;
66
use crate::object::{Traverse, TraverseFn};
77
use crate::{
8-
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult,
8+
AsObject, Context, Py, PyExact, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult,
99
TryFromObject, atomic_func,
1010
builtins::{
1111
PyTuple,
@@ -681,22 +681,29 @@ impl Py<PyDict> {
681681
let self_exact = self.exact_dict(vm);
682682
let other_exact = other.exact_dict(vm);
683683
if self_exact && other_exact {
684-
self.entries.get_chain(&other.entries, vm, key)
684+
// SAFETY: exact_dict checks passed
685+
let self_exact = unsafe { PyExact::ref_unchecked(self) };
686+
let other_exact = unsafe { PyExact::ref_unchecked(other) };
687+
self_exact.get_chain_exact(other_exact, key, vm)
685688
} else if let Some(value) = self.get_item_opt(key, vm)? {
686689
Ok(Some(value))
687690
} else {
688691
other.get_item_opt(key, vm)
689692
}
690693
}
694+
}
691695

692-
/// Like `get_chain` but skips the exact_dict type checks. Use when both
693-
/// dicts are known to be exact dict types (e.g. globals + builtins).
694-
pub fn get_chain_exact<K: DictKey + ?Sized>(
696+
impl PyExact<PyDict> {
697+
/// Look up `key` in `self`, falling back to `other`.
698+
/// Both dicts must be exact `dict` types (enforced by `PyExact`).
699+
pub(crate) fn get_chain_exact<K: DictKey + ?Sized>(
695700
&self,
696701
other: &Self,
697702
key: &K,
698703
vm: &VirtualMachine,
699704
) -> PyResult<Option<PyObjectRef>> {
705+
debug_assert!(self.class().is(vm.ctx.types.dict_type));
706+
debug_assert!(other.class().is(vm.ctx.types.dict_type));
700707
self.entries.get_chain(&other.entries, vm, key)
701708
}
702709
}

crates/vm/src/builtins/range.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -607,10 +607,23 @@ impl IterNext for PyLongRangeIterator {
607607
#[pyclass(module = false, name = "range_iterator")]
608608
#[derive(Debug)]
609609
pub struct PyRangeIterator {
610-
pub(crate) index: AtomicCell<usize>,
611-
pub(crate) start: isize,
612-
pub(crate) step: isize,
613-
pub(crate) length: usize,
610+
index: AtomicCell<usize>,
611+
start: isize,
612+
step: isize,
613+
length: usize,
614+
}
615+
616+
impl PyRangeIterator {
617+
/// Advance and return next value without going through the iterator protocol.
618+
#[inline]
619+
pub(crate) fn next_fast(&self) -> Option<isize> {
620+
let index = self.index.fetch_add(1);
621+
if index < self.length {
622+
Some(self.start + (index as isize) * self.step)
623+
} else {
624+
None
625+
}
626+
}
614627
}
615628

616629
impl PyPayload for PyRangeIterator {

crates/vm/src/frame.rs

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
#[cfg(feature = "flame")]
22
use crate::bytecode::InstructionMetadata;
33
use crate::{
4-
AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine,
4+
AsObject, Py, PyExact, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject,
5+
VirtualMachine,
56
builtins::{
67
PyBaseException, PyBaseExceptionRef, PyCode, PyCoroutine, PyDict, PyDictRef, PyGenerator,
78
PyInterpolation, PyList, PySet, PySlice, PyStr, PyStrInterned, PyTemplate, PyTraceback,
@@ -446,7 +447,14 @@ impl Py<Frame> {
446447
locals: &self.locals,
447448
globals: &self.globals,
448449
builtins: &self.builtins,
449-
builtins_dict: self.builtins.downcast_ref_if_exact::<PyDict>(vm),
450+
builtins_dict: if self.globals.class().is(vm.ctx.types.dict_type) {
451+
self.builtins
452+
.downcast_ref_if_exact::<PyDict>(vm)
453+
// SAFETY: downcast_ref_if_exact already verified exact type
454+
.map(|d| unsafe { PyExact::ref_unchecked(d) })
455+
} else {
456+
None
457+
},
450458
lasti: &self.lasti,
451459
object: self,
452460
state: &mut state,
@@ -529,9 +537,11 @@ struct ExecutingFrame<'a> {
529537
locals: &'a ArgMapping,
530538
globals: &'a PyDictRef,
531539
builtins: &'a PyObjectRef,
532-
/// Cached downcast of builtins to PyDict. builtins never changes during
533-
/// frame execution, so we avoid repeating the downcast on every LOAD_GLOBAL.
534-
builtins_dict: Option<&'a Py<PyDict>>,
540+
/// Cached downcast of builtins to PyDict for fast LOAD_GLOBAL.
541+
/// Only set when both globals and builtins are exact dict types (not
542+
/// subclasses), so that `__missing__` / `__getitem__` overrides are
543+
/// not bypassed.
544+
builtins_dict: Option<&'a PyExact<PyDict>>,
535545
object: &'a Py<Frame>,
536546
lasti: &'a PyAtomic<u32>,
537547
state: &'a mut FrameState,
@@ -3015,8 +3025,10 @@ impl ExecutingFrame<'_> {
30153025
#[inline]
30163026
fn load_global_or_builtin(&self, name: &Py<PyStr>, vm: &VirtualMachine) -> PyResult {
30173027
if let Some(builtins_dict) = self.builtins_dict {
3018-
// Fast path: both globals (PyDictRef) and builtins are exact dicts
3019-
self.globals
3028+
// Fast path: both globals and builtins are exact dicts
3029+
// SAFETY: builtins_dict is only set when globals is also exact dict
3030+
let globals_exact = unsafe { PyExact::ref_unchecked(self.globals.as_ref()) };
3031+
globals_exact
30203032
.get_chain_exact(builtins_dict, name, vm)?
30213033
.ok_or_else(|| {
30223034
vm.new_name_error(format!("name '{name}' is not defined"), name.to_owned())
@@ -3701,13 +3713,10 @@ impl ExecutingFrame<'_> {
37013713

37023714
// FOR_ITER_RANGE: bypass generic iterator protocol for range iterators
37033715
if let Some(range_iter) = top.downcast_ref_if_exact::<PyRangeIterator>(vm) {
3704-
let index = range_iter.index.fetch_add(1);
3705-
if index < range_iter.length {
3706-
let value = range_iter.start + (index as isize) * range_iter.step;
3716+
if let Some(value) = range_iter.next_fast() {
37073717
self.push_value(vm.ctx.new_int(value).into());
37083718
return Ok(true);
37093719
}
3710-
// Exhausted
37113720
if vm.use_tracing.get() && !vm.is_none(&self.object.trace.lock()) {
37123721
let stop_exc = vm.new_stop_iteration(None);
37133722
self.fire_exception_trace(&stop_exc, vm)?;
@@ -4063,7 +4072,7 @@ impl ExecutingFrame<'_> {
40634072
self.push_value(vm.ctx.new_bool(result).into());
40644073
return Ok(None);
40654074
}
4066-
// COMPARE_OP_FLOAT
4075+
// COMPARE_OP_FLOAT: leaf type, cannot recurse — skip rich_compare dispatch
40674076
if let (Some(a_f), Some(b_f)) = (
40684077
a.downcast_ref_if_exact::<PyFloat>(vm),
40694078
b.downcast_ref_if_exact::<PyFloat>(vm),

crates/vm/src/protocol/object.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,16 +277,16 @@ impl PyObject {
277277

278278
// Perform a comparison, raising TypeError when the requested comparison
279279
// operator is not supported.
280-
// see: CPython PyObject_RichCompare / do_richcompare
280+
// see: PyObject_RichCompare / do_richcompare
281281
#[inline] // called by ExecutingFrame::execute_compare with const op
282282
fn _cmp(
283283
&self,
284284
other: &Self,
285285
op: PyComparisonOp,
286286
vm: &VirtualMachine,
287287
) -> PyResult<Either<PyObjectRef, bool>> {
288-
// Single recursion guard for the entire comparison (matching CPython's
289-
// Py_EnterRecursiveCallTstate placement in do_richcompare).
288+
// Single recursion guard for the entire comparison
289+
// (do_richcompare in Objects/object.c).
290290
vm.with_recursion("in comparison", || self._cmp_inner(other, op, vm))
291291
}
292292

0 commit comments

Comments
 (0)