Skip to content
4 changes: 1 addition & 3 deletions majit/majit-backend/src/resume_guard_descr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@
//! `bridge_dispatch_load()` necessarily holds such an `Arc` for the
//! borrow lifetime, so drop cannot interleave with the load → retain
//! window.
//! - The only background thread spawned by the driver
//! (`jitdriver.rs:762 invalidation_thread`) touches a
//! `Mutex<QuasiImmut>` and never reaches into `ResumeGuardDescr`.
//! - The driver spawns no background thread at all.
//!
//! These three facts together close the race CodeRabbit and Codex
//! flagged on PR #68 (Critical #6/#10/#13). Any future change that
Expand Down
59 changes: 0 additions & 59 deletions majit/majit-metainterp/src/jitdriver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1221,14 +1221,6 @@ pub struct JitDriver<S: JitState> {
/// PyPy JitDriver(is_recursive=True): enables max_unroll_recursion
/// for recursive portal calls (pyjitpl.py _opimpl_recursive_call).
is_recursive: bool,
/// Shared quasi-immutable notifier for periodic loop invalidation.
/// RPython compile.py:205: loop.quasi_immutable_deps registration.
/// All compiled loops register their invalidation flag here.
/// A background thread periodically calls invalidate() to force
/// GUARD_NOT_INVALIDATED exits in compiled code.
epoch_qmut: std::sync::Arc<std::sync::Mutex<crate::quasiimmut::QuasiImmut>>,
/// Handle for the background invalidation thread.
_invalidation_thread: Option<std::thread::JoinHandle<()>>,
/// Driver-shared `Assembler`: holds the
/// `all_liveness` payload (`assembler.py:30`) populated incrementally
/// by `__JitMeta::install_canonical_liveness` (canonical entry) and
Expand Down Expand Up @@ -1358,45 +1350,10 @@ fn install_state_field_fvc(data: &StateFieldFvcData) {
impl<S: JitState> JitDriver<S> {
/// Create a new JitDriver with the given hot-counting threshold.
pub fn new(threshold: u32) -> Self {
Self::with_options(threshold, true)
}

/// Create a new JitDriver, optionally skipping the background timer that
/// periodically invalidates all compiled loops.
///
/// The periodic invalidation is a portable stand-in for RPython's
/// GC/signal-triggered invalidation, used by quasi-immutable-bearing
/// consumers (a Python JIT). A consumer with no quasi-immutable state — e.g.
/// a fixed-bytecode interpreter over plain integer reds — has nothing to
/// invalidate; for it the timer only forces pointless re-tracing (and
/// exercises the GUARD_NOT_INVALIDATED resume path needlessly), so it should
/// pass `periodic_invalidation = false`.
pub fn with_options(threshold: u32, periodic_invalidation: bool) -> Self {
let mut meta = MetaInterp::new(threshold);
if let Some(info) = S::__build_virtualizable_info() {
meta.set_virtualizable_info(info);
}
let epoch_qmut =
std::sync::Arc::new(std::sync::Mutex::new(crate::quasiimmut::QuasiImmut::new()));
// Background thread: periodically invalidate all registered loops.
// RPython uses GC/signal-triggered invalidation; we use a timer as
// a portable equivalent. Period matches PyPy's checkinterval (~10ms).
#[cfg(not(target_arch = "wasm32"))]
let invalidation_thread = if periodic_invalidation {
let qmut = epoch_qmut.clone();
Some(std::thread::spawn(move || {
loop {
std::thread::sleep(std::time::Duration::from_millis(50));
if let Ok(mut qmut) = qmut.lock() {
if qmut.has_watchers() {
qmut.invalidate();
}
}
}
}))
} else {
None
};
JitDriver {
meta,
sym: None,
Expand All @@ -1409,11 +1366,6 @@ impl<S: JitState> JitDriver<S> {
bridge_body_start_op_count: None,
entry_points: Vec::new(),
is_recursive: false,
epoch_qmut,
#[cfg(not(target_arch = "wasm32"))]
_invalidation_thread: invalidation_thread,
#[cfg(target_arch = "wasm32")]
_invalidation_thread: None,
blackhole_allocator: None,
portal_runner: None,
portal_jd_index: None,
Expand Down Expand Up @@ -6332,17 +6284,6 @@ impl<S: JitState> JitDriver<S> {
}
pre_run();

// RPython compile.py:205-207: register loop token with
// quasi-immutable deps so the background invalidation thread
// can force GUARD_NOT_INVALIDATED exits periodically.
if let Some(token) = self.meta.get_loop_token(key_hash) {
if let Ok(mut qmut) = self.epoch_qmut.lock() {
for flag in token.all_invalidation_flags() {
qmut.register(&flag);
}
}
}

loop {
let (
is_finish,
Expand Down
2 changes: 0 additions & 2 deletions majit/majit-metainterp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ pub mod optimize;
pub mod optimizeopt;
pub(crate) mod parity;
mod pyjitpl;
pub mod quasiimmut;
pub mod recorder;
pub mod resoperation;
pub mod resume;
Expand Down Expand Up @@ -160,7 +159,6 @@ pub use pyjitpl::{
set_record_inline_application_traceback_hook, struct_fields_write_effect_info, trace_jitcode,
trace_jitcode_from_merge_point, trace_jitcode_with_args, trace_jitcode_with_args_and_runtime,
};
pub use quasiimmut::QuasiImmut;
pub use resume_box_reader::{
BridgeVirtualCache, decode_fieldnum, default_bridge_array_descr, emit_pending_field_op,
materialize_bridge_virtual, rebuilt_value_to_opref, replay_pending_fields,
Expand Down
100 changes: 84 additions & 16 deletions majit/majit-metainterp/src/optimizeopt/heap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,68 @@ impl OptHeap {
descr_identity(descr)
}

/// `quasiimmut.py:147-159 QuasiImmutDescr.is_still_valid_for`, the check
/// `heap.py:802-804` turns into `InvalidLoop('quasi immutable field changed
/// during tracing')`.
///
/// ```text
/// def is_still_valid_for(self, structconst):
/// assert self.struct
/// if self.struct != structconst.getref_base():
/// return False
/// qmut = get_current_qmut_instance(cpu, self.struct,
/// self.mutatefielddescr)
/// if qmut is not self.qmut:
/// return False
/// else:
/// currentbox = self.get_current_constant_fieldvalue()
/// assert self.constantfieldbox.same_constant(currentbox)
/// return True
/// ```
///
/// Upstream detects the change through the `qmut` object's identity —
/// invalidation nulls the hidden `mutate_*` field, so
/// `get_current_qmut_instance` hands back a fresh instance and the `is not`
/// test fires; the field-value comparison is the assert that backs it up.
/// Pyre has no per-read `QuasiImmutDescr` to hang that identity on, so the
/// value comparison is the test itself: the tracer captured the field on
/// `arg(1)` (`state::current_quasiimmut_field_value`) and a live re-read
/// through `get_runtime_field` is `get_current_constant_fieldvalue`.
///
/// Returns `true` — keep the loop — whenever the comparison cannot be made:
/// a struct that did not fold to a constant is the case heap.py:794-796
/// ignores outright, and an op without the captured value is the namespace
/// twin, which carries a slot index there instead.
fn quasiimmut_field_still_valid(
op: &Op,
obj: OpRef,
descr: &DescrRef,
ctx: &mut OptContext,
) -> bool {
if op.num_args() < 2 {
return true;
}
let Some(constantfieldbox) = op.arg(1).const_value() else {
return true;
};
// heap.py:794-796 `if not structvalue.is_constant(): return`.
if ctx
.get_box_replacement_operand_opt(obj)
.and_then(|b| ctx.get_constant_ptr_box(&b))
.is_none()
{
return true;
}
let Some(currentbox) = ctx
.get_runtime_field(obj, descr)
.and_then(|r| r.inline_const_to_value())
else {
return true;
};
// history.py:204 `Const.same_constant`.
currentbox == constantfieldbox
}

/// Compute the `PtrInfo._fields` slot for a field descriptor.
///
/// RPython uses `descr.get_index()` only for `info._fields[index]`
Expand Down Expand Up @@ -3239,23 +3301,29 @@ impl OptHeap {
// already emitted one via generate_guard (pyjitpl.py:1087).
// Records quasi_immutable_deps for invalidation tracking.
let obj = op.arg(0).to_opref();
// RPython optimize_QUASIIMMUT_FIELD: collect quasi-immutable
// dependencies. Add (obj_ptr, field_idx) to quasi_immutable_deps
// for per-slot watcher registration after compilation.
// field_idx comes from descr (GC object fields) or arg(1)
// (namespace slot index).
let (dep_field_idx, cache_field_key) = if let Some(descr) = op.getdescr() {
(
Some(Self::field_effect_index(&descr)),
// heap.py:798-804 — the field can have changed between the
// tracer reading it and this pass running. Abandon the loop
// when it has; the traced value is baked in as a constant and
// nothing downstream will re-prove it.
if let Some(descr) = op.getdescr() {
if !Self::quasiimmut_field_still_valid(op, obj, &descr, ctx) {
return OptimizationResult::InvalidLoop(
"quasi immutable field changed during tracing",
);
}
}
// heap.py:807-809 `self.optimizer.quasi_immutable_deps[
// qmutdescr.qmut] = None`. Upstream keys the set on the
// `QuasiImmut` instance the descr already resolved; pyre records
// the pair that identifies it — the owning object and which of
// its quasi-immutable fields — and `register_quasi_immutable_deps`
// resolves the instance after compilation.
let (dep_field_idx, cache_field_key) = match op.getdescr() {
Some(descr) => (
Some(descr.index()),
Some(Self::field_cache_identity(&descr)),
)
} else if op.num_args() > 1 {
let idx = ctx
.get_constant_int_box(&op.arg(1).get_box_replacement(false))
.map(|v| v as u32);
(idx, idx.map(|v| v as usize))
} else {
(None, None)
),
None => (None, None),
};
if let Some(idx) = dep_field_idx {
// The quasi-immutable dependency object (namespace dict /
Expand Down
Loading
Loading