From df0e37904eb2a307e71ba54a0914d0b071322578 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Mon, 10 Aug 2026 22:47:10 +0900 Subject: [PATCH 1/3] majit: order the annotator's pointer-keyed reflow sets and retain notify's block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RPythonAnnotator::notify`'s position set, the graph maps built in `complete` and `seed_all_annotated_return_vars`, and `ListItem::read_locations` were `HashSet`/`HashMap` keyed on values derived from `Rc::as_ptr`. Each of those loops issues `reflowfromposition` calls, so their iteration order is a work order, and hashing an address makes that order differ between processes. They now use `IndexSet`/`IndexMap`, the containers `annotated`, `added_blocks`, `blocked_blocks` and `genpendingblocks` already use in the same file. `notify` additionally stored only the block's address and never a reference to it, and it is the only `BlockKey`-keyed map that is never pruned — its siblings are all `shift_remove`d when a block leaves the annotator's view. Entries now carry the `BlockRef` they are keyed on, so the address behind a live key cannot be handed to a later block. The prepass census flip reported in gh#1139 still occurs with these changes applied. Assisted-by: Claude --- .../src/annotator/annrpython.rs | 53 +++++++++++++------ .../majit-translate/src/annotator/listdef.rs | 9 ++-- 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/majit/majit-translate/src/annotator/annrpython.rs b/majit/majit-translate/src/annotator/annrpython.rs index 02626c9da66..8b4edbbcac2 100644 --- a/majit/majit-translate/src/annotator/annrpython.rs +++ b/majit/majit-translate/src/annotator/annrpython.rs @@ -11,7 +11,7 @@ //! comment verbatim and a `todo!()` stub so every stub surfaces at //! runtime rather than silently no-op'ing. -use indexmap::IndexMap; +use indexmap::{IndexMap, IndexSet}; use std::cell::RefCell; use std::collections::{HashMap, HashSet}; use std::rc::Rc; @@ -31,6 +31,15 @@ use crate::translator::translator::TranslationContext; /// route output to stderr. pub static LOG: AnsiLogger = AnsiLogger::new("annrpython"); +/// One entry of [`RPythonAnnotator::notify`]. +pub(crate) struct NotifyEntry { + /// Retains the block so its address cannot be recycled by a later + /// block — the identity the key encodes must stay unique for as + /// long as the entry lives. + block: BlockRef, + positions: IndexSet, +} + /// RPython `class RPythonAnnotator(object)` (annrpython.py:22). /// /// "Block annotator for RPython." @@ -91,7 +100,9 @@ pub struct RPythonAnnotator { /// RPython `self.links_followed = {}` (annrpython.py:39). pub links_followed: RefCell>, /// RPython `self.notify = {}` (annrpython.py:40). - pub(crate) notify: RefCell>>, + /// The ordered container is required because the key hashes on a + /// pointer and the loop over the values produces a work order. + pub(crate) notify: RefCell>, /// RPython `self.fixed_graphs = {}` (annrpython.py:41). Graphs /// that have already been rtyped — `addpendingblock` rejects new /// pending entries against these. @@ -965,7 +976,11 @@ impl RPythonAnnotator { self.notify .borrow_mut() .entry(BlockKey::of(&returnblock)) - .or_default() + .or_insert_with(|| NotifyEntry { + block: Rc::clone(&returnblock), + positions: IndexSet::new(), + }) + .positions .insert(pk); } @@ -1314,7 +1329,10 @@ impl RPythonAnnotator { // owning graph; treat `None` (= False) entries as // "blocked". let annotated = self.annotated.borrow(); - let mut graphs: HashMap = HashMap::new(); + // The ordered container is required because the key + // hashes on a pointer and the loop over the values + // produces a work order. + let mut graphs: IndexMap = IndexMap::new(); let mut got_blocked = false; for bkey in added_set.keys() { match annotated @@ -1417,7 +1435,10 @@ impl RPythonAnnotator { pub(crate) fn seed_all_annotated_return_vars(&self) { let graphs: Vec = { let annotated = self.annotated.borrow(); - let mut seen: HashMap = HashMap::new(); + // The ordered container is required because the key hashes + // on a pointer and the loop over the values produces a work + // order. + let mut seen: IndexMap = IndexMap::new(); for g in annotated.values().flatten() { seen.entry(GraphKey::of(g)).or_insert_with(|| Rc::clone(g)); } @@ -2877,12 +2898,14 @@ impl RPythonAnnotator { // self.notify[block]: self.reflowfromposition(position)`. let positions: Vec = { let bkey = BlockKey::of(block); - self.notify - .borrow() - .get(&bkey) - .cloned() - .map(|set| set.into_iter().collect()) - .unwrap_or_default() + let notify = self.notify.borrow(); + match notify.get(&bkey) { + Some(entry) => { + debug_assert_eq!(BlockKey::of(&entry.block), bkey); + entry.positions.iter().cloned().collect() + } + None => Vec::new(), + } }; for position in positions { // upstream: `self.reflowfromposition(position)` @@ -3387,10 +3410,11 @@ mod tests { // position key. let returnblock = callee.borrow().returnblock.clone(); let notify = ann.notify.borrow(); - let positions = notify + let entry = notify .get(&BlockKey::of(&returnblock)) .expect("notify entry missing"); - assert_eq!(positions.len(), 1); + assert!(Rc::ptr_eq(&entry.block, &returnblock)); + assert_eq!(entry.positions.len(), 1); } #[test] @@ -3489,8 +3513,7 @@ mod tests { .notify .borrow() .get(&BlockKey::of(&returnblock)) - .cloned() - .map(|set| set.into_iter().collect()) + .map(|entry| entry.positions.iter().cloned().collect()) .unwrap_or_default(); assert_eq!(positions.len(), 1); for position in &positions { diff --git a/majit/majit-translate/src/annotator/listdef.rs b/majit/majit-translate/src/annotator/listdef.rs index 607f2c2f2db..1ba2567a07e 100644 --- a/majit/majit-translate/src/annotator/listdef.rs +++ b/majit/majit-translate/src/annotator/listdef.rs @@ -34,10 +34,11 @@ //! but the semantics match byte-for-byte. use std::cell::{Cell, RefCell}; -use std::collections::HashSet; use std::fmt; use std::rc::{Rc, Weak}; +use indexmap::IndexSet; + use super::repr_guard::ReprGuard; use super::bookkeeper::{Bookkeeper, PositionKey}; @@ -188,7 +189,9 @@ pub struct ListItem { /// every owner currently using this `ListItem`. pub(crate) itemof: Vec, /// RPython `self.read_locations = set()` (listdef.py:33). - pub(crate) read_locations: HashSet, + /// The ordered container is required because the key hashes on a + /// pointer and the loop over the members produces a work order. + pub(crate) read_locations: IndexSet, /// Flattened `DictKey.custom_eq_hash` (dictdef.py:13). `false` for /// every non-DictKey ListItem. pub custom_eq_hash: bool, @@ -215,7 +218,7 @@ impl ListItem { immutable: false, must_not_resize: false, itemof: Vec::new(), - read_locations: HashSet::new(), + read_locations: IndexSet::new(), // Flattened DictKey defaults (dictdef.py:8-9, 13). custom_eq_hash: false, s_rdict_eqfn: SomeValue::Impossible, From 0d16b541c669962449f3baacd1a0bd440bb1acc4 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 11 Aug 2026 18:49:04 +0900 Subject: [PATCH 2/3] majit: retain links_followed's links and order the serialized descr set keys `RPythonAnnotator::links_followed` keyed links by `Rc::as_ptr` while holding no reference to them. `transform_dead_code` drops a link's last `Rc` in the same loop in which `cutoff_alwaysraising_block` allocates a fresh one, so a new link can land on a freed but still-recorded address and read back as already followed; the dead arm then survives and the block's cutoff never runs. Hold the `LinkRef` in the map, as `all_blocks` and `notify` do. The `seen` map that `annotate`'s block-subset path builds is also keyed on a pointer and iterated to produce a work order; make it ordered. `canonicalize_keyed_descrs` returned `descr_set_keys` in `Arc` address order, so the serialized effect info carried a different member order per process. Sort the keys by content, which `DescrSetMember` now derives `Ord` for. The raw `_*_descrs_*` sets keep pointer order, which the `descr_ptr_id` binary_search in `compute_bitstrings` requires; the reader rebuilds those sets from the keys and re-canonicalises, so the two orders are independent. Assisted-by: Claude --- majit/majit-ir/src/effectinfo.rs | 2 +- .../src/annotator/annrpython.rs | 24 +++++++++++++------ majit/majit-translate/src/codewriter/call.rs | 3 +++ .../src/translator/transform.rs | 19 +++++++++++---- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/majit/majit-ir/src/effectinfo.rs b/majit/majit-ir/src/effectinfo.rs index 29b55e6db79..f2336fe0a14 100644 --- a/majit/majit-ir/src/effectinfo.rs +++ b/majit/majit-ir/src/effectinfo.rs @@ -35,7 +35,7 @@ impl std::error::Error for UnsupportedFieldExc {} /// these tuples. The key alone identifies the slot; what it does *not* carry /// is the layout needed to create the descr when the slot is empty, which is /// what [`DescrMintSpec`] adds. -#[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] pub enum DescrSetMember { /// `descr.py:218-239 get_field_descr(gccache, STRUCT, fieldname)` — /// `_cache_field[Struct(struct_id)][field_name]`. diff --git a/majit/majit-translate/src/annotator/annrpython.rs b/majit/majit-translate/src/annotator/annrpython.rs index 8b4edbbcac2..554ad052b08 100644 --- a/majit/majit-translate/src/annotator/annrpython.rs +++ b/majit/majit-translate/src/annotator/annrpython.rs @@ -13,7 +13,7 @@ use indexmap::{IndexMap, IndexSet}; use std::cell::RefCell; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::rc::Rc; use super::super::flowspace::model::{ @@ -98,7 +98,10 @@ pub struct RPythonAnnotator { /// entire monotonically-growing annotator session for every subject. subject_annotation_snapshots: RefCell>>, /// RPython `self.links_followed = {}` (annrpython.py:39). - pub links_followed: RefCell>, + /// Retains each link because the key is its address: a link removed from a + /// block's `exits` would otherwise be freed, and a later link allocated at + /// that address would read back as already followed. + pub links_followed: RefCell>, /// RPython `self.notify = {}` (annrpython.py:40). /// The ordered container is required because the key hashes on a /// pointer and the loop over the values produces a work order. @@ -533,7 +536,7 @@ impl RPythonAnnotator { all_blocks: RefCell::new(IndexMap::new()), added_blocks: RefCell::new(None), subject_annotation_snapshots: RefCell::new(None), - links_followed: RefCell::new(HashSet::new()), + links_followed: RefCell::new(IndexMap::new()), notify: RefCell::new(IndexMap::new()), fixed_graphs: RefCell::new(IndexMap::new()), blocked_blocks: RefCell::new(IndexMap::new()), @@ -1168,7 +1171,10 @@ impl RPythonAnnotator { let graphs: Vec = match block_subset { None => self.translator.graphs.borrow().clone(), Some(blocks) => { - let mut seen: HashMap = HashMap::new(); + // The ordered container is required because the key hashes + // on a pointer and the loop over the values produces a work + // order. + let mut seen: IndexMap = IndexMap::new(); let annotated = self.annotated.borrow(); for block in blocks { let key = BlockKey::of(block); @@ -1911,7 +1917,7 @@ impl RPythonAnnotator { let lkey = LinkKey::of(link); drop(link_borrow); - self.links_followed.borrow_mut().insert(lkey); + self.links_followed.borrow_mut().insert(lkey, link.clone()); // Internal flowin produces concrete SomeValue per link arg (None // is upstream's "unannotated caller arg" signal that originates // from the call-site interface, not from intra-graph flow). @@ -2101,7 +2107,7 @@ impl RPythonAnnotator { } let lkey = LinkKey::of(link); - self.links_followed.borrow_mut().insert(lkey); + self.links_followed.borrow_mut().insert(lkey, link.clone()); let inputs_s_opt: Vec> = inputs_s.into_iter().map(Some).collect(); self.addpendingblock(graph, &target_rc, &inputs_s_opt); } @@ -3323,7 +3329,11 @@ mod tests { ann.follow_link(&graph, &link, &HashMap::new()); // links_followed should record this link. - assert!(ann.links_followed.borrow().contains(&LinkKey::of(&link))); + assert!( + ann.links_followed + .borrow() + .contains_key(&LinkKey::of(&link)) + ); // target.inputargs[0].annotation should be SomeInteger. let bound = { let t = target.borrow(); diff --git a/majit/majit-translate/src/codewriter/call.rs b/majit/majit-translate/src/codewriter/call.rs index 9d967ed43c6..96b7f686a9c 100644 --- a/majit/majit-translate/src/codewriter/call.rs +++ b/majit/majit-translate/src/codewriter/call.rs @@ -6146,6 +6146,9 @@ fn canonicalize_keyed_descrs( descrs.push(descr); keys.push(key?); } + // The raw set stays in pointer order for identity dedup and lookup, while + // the member order crossing into the artifact is determined by content. + keys.sort(); Some((descrs, keys)) } diff --git a/majit/majit-translate/src/translator/transform.rs b/majit/majit-translate/src/translator/transform.rs index 54721983073..3e5d56e5902 100644 --- a/majit/majit-translate/src/translator/transform.rs +++ b/majit/majit-translate/src/translator/transform.rs @@ -244,7 +244,10 @@ pub fn transform_dead_code(ann: &RPythonAnnotator, block_subset: &[BlockRef]) { // rebinds the attribute which changes subsequent comparisons). let exits_snapshot: Vec = block.borrow().exits.to_vec(); for link in exits_snapshot { - let followed = ann.links_followed.borrow().contains(&LinkKey::of(&link)); + let followed = ann + .links_followed + .borrow() + .contains_key(&LinkKey::of(&link)); if followed { continue; } @@ -390,7 +393,7 @@ pub fn cutoff_alwaysraising_block(ann: &RPythonAnnotator, block: &BlockRef) { // upstream: `self.links_followed[errlink] = True`. ann.links_followed .borrow_mut() - .insert(LinkKey::of(&errlink)); + .insert(LinkKey::of(&errlink), errlink.clone()); // upstream: `etype, evalue = graph.exceptblock.inputargs`. let (etype_rc, evalue_rc) = { @@ -1019,7 +1022,9 @@ mod tests { // Mark the single exit as followed so transform_dead_code // doesn't prune it. for link in &start.borrow().exits { - ann.links_followed.borrow_mut().insert(LinkKey::of(link)); + ann.links_followed + .borrow_mut() + .insert(LinkKey::of(link), link.clone()); } transform_graph(&ann, None, Some(&[start.clone()])); @@ -1066,7 +1071,9 @@ mod tests { .borrow_mut() .insert(BlockKey::of(&start), start.clone()); for link in &start.borrow().exits { - ann.links_followed.borrow_mut().insert(LinkKey::of(link)); + ann.links_followed + .borrow_mut() + .insert(LinkKey::of(link), link.clone()); } transform_graph(&ann, None, Some(&[start.clone()])); @@ -1116,7 +1123,9 @@ mod tests { start.closeblock(vec![left.clone(), right.clone()]); // Only the false branch is followed. - ann.links_followed.borrow_mut().insert(LinkKey::of(&left)); + ann.links_followed + .borrow_mut() + .insert(LinkKey::of(&left), left.clone()); ann.annotated .borrow_mut() .insert(BlockKey::of(&start), Some(graph.clone())); From 5fa150dfe281828f5a7415033c93690e327bda00 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 11 Aug 2026 21:12:20 +0900 Subject: [PATCH 3/3] majit: trace and stabilize prepass determinism --- .../src/annotator/annrpython.rs | 46 ++++ .../src/annotator/bookkeeper.rs | 68 +++++- .../src/annotator/classdesc.rs | 54 ++++- .../majit-translate/src/annotator/listdef.rs | 28 +++ .../src/annotator/specialize.rs | 4 +- .../src/flowspace/generator.rs | 8 +- majit/majit-translate/src/flowspace/model.rs | 219 +++++++++++++++++- .../majit-translate/src/flowspace/pygraph.rs | 3 +- majit/majit-translate/src/lib.rs | 8 + .../src/translator/rtyper/cutover.rs | 115 ++++++++- .../translator/rtyper/flowspace_adapter.rs | 2 +- .../src/translator/rtyper/normalizecalls.rs | 2 +- .../translator/rtyper/pyre_call_registry.rs | 40 +++- .../src/translator/rtyper/rpbc.rs | 4 +- .../src/translator/rtyper/rtyper.rs | 2 +- .../src/translator/transform.rs | 78 +++++++ .../src/translator/translator.rs | 2 +- 17 files changed, 625 insertions(+), 58 deletions(-) diff --git a/majit/majit-translate/src/annotator/annrpython.rs b/majit/majit-translate/src/annotator/annrpython.rs index 554ad052b08..040a20bec2a 100644 --- a/majit/majit-translate/src/annotator/annrpython.rs +++ b/majit/majit-translate/src/annotator/annrpython.rs @@ -15,6 +15,7 @@ use indexmap::{IndexMap, IndexSet}; use std::cell::RefCell; use std::collections::HashMap; use std::rc::Rc; +use std::sync::atomic::{AtomicU64, Ordering}; use super::super::flowspace::model::{ BlockKey, BlockRef, GraphKey, GraphRef, Hlvalue, LinkKey, LinkRef, Variable, checkgraph, @@ -31,6 +32,38 @@ use crate::translator::translator::TranslationContext; /// route output to stderr. pub static LOG: AnsiLogger = AnsiLogger::new("annrpython"); +// Exists to localise prepass nondeterminism (gh#1139). +static REFLOW_COUNT: AtomicU64 = AtomicU64::new(0); + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn reflow_count() -> u64 { + REFLOW_COUNT.load(Ordering::Relaxed) +} + +// Exists to localise prepass nondeterminism (gh#1139). +static PROCESSBLOCK_COUNT: AtomicU64 = AtomicU64::new(0); + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn processblock_count() -> u64 { + PROCESSBLOCK_COUNT.load(Ordering::Relaxed) +} + +// Exists to localise prepass nondeterminism (gh#1139). +static REFLOW_FROM_NOTIFY: AtomicU64 = AtomicU64::new(0); + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn reflow_from_notify_count() -> u64 { + REFLOW_FROM_NOTIFY.load(Ordering::Relaxed) +} + +// Exists to localise prepass nondeterminism (gh#1139). +static NOTIFY_HIT_ON_REUSED: AtomicU64 = AtomicU64::new(0); + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn notify_hit_on_reused_count() -> u64 { + NOTIFY_HIT_ON_REUSED.load(Ordering::Relaxed) +} + /// One entry of [`RPythonAnnotator::notify`]. pub(crate) struct NotifyEntry { /// Retains the block so its address cannot be recycled by a later @@ -2130,6 +2163,7 @@ impl RPythonAnnotator { /// the shared `notify` map, leaving a position whose `Weak` /// still upgrades against a session-retained `Rc`. pub(crate) fn reflowfromposition(&self, position_key: &PositionKey) { + REFLOW_COUNT.fetch_add(1, Ordering::Relaxed); // upstream: `graph, block, index = position_key` let Some(graph) = position_key.graph() else { return; @@ -2908,6 +2942,16 @@ impl RPythonAnnotator { match notify.get(&bkey) { Some(entry) => { debug_assert_eq!(BlockKey::of(&entry.block), bkey); + if crate::determinism_trace_enabled() + && crate::flowspace::model::block_address_was_reused(block) + { + NOTIFY_HIT_ON_REUSED.fetch_add(1, Ordering::Relaxed); + eprintln!( + "[DTRACE-REUSE] notify hit on recycled block addr={} npos={}", + bkey.as_usize(), + entry.positions.len() + ); + } entry.positions.iter().cloned().collect() } None => Vec::new(), @@ -2915,6 +2959,7 @@ impl RPythonAnnotator { }; for position in positions { // upstream: `self.reflowfromposition(position)` + REFLOW_FROM_NOTIFY.fetch_add(1, Ordering::Relaxed); self.reflowfromposition(&position); } @@ -2950,6 +2995,7 @@ impl RPythonAnnotator { graph: &GraphRef, block: &BlockRef, ) -> Result<(), crate::annotator::model::AnnotatorError> { + PROCESSBLOCK_COUNT.fetch_add(1, Ordering::Relaxed); let bkey = BlockKey::of(block); // upstream: `self.annotated[block] = graph`. self.annotated diff --git a/majit/majit-translate/src/annotator/bookkeeper.rs b/majit/majit-translate/src/annotator/bookkeeper.rs index 2312f59d019..f5f6a7a6f68 100644 --- a/majit/majit-translate/src/annotator/bookkeeper.rs +++ b/majit/majit-translate/src/annotator/bookkeeper.rs @@ -29,6 +29,7 @@ use indexmap::{IndexMap, IndexSet}; use std::cell::RefCell; use std::collections::{HashMap, HashSet}; use std::rc::{Rc, Weak}; +use std::sync::atomic::{AtomicU64, Ordering}; use super::argument::{ArgumentsForTranslation, simple_args}; use super::classdesc::{ClassDef, ClassDesc}; @@ -50,6 +51,22 @@ use crate::flowspace::bytecode::cpython_code_signature; use crate::flowspace::model::{BlockRef, ConstValue, Constant, GraphKey, GraphRef, HostObject}; use crate::tool::algo::unionfind::UnionFind; +// Exists to localise prepass nondeterminism (gh#1139). +static REFLOW_FROM_ATTR: AtomicU64 = AtomicU64::new(0); + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn reflow_from_attr_count() -> u64 { + REFLOW_FROM_ATTR.load(Ordering::Relaxed) +} + +// Exists to localise prepass nondeterminism (gh#1139). +static REFLOW_FROM_PBC: AtomicU64 = AtomicU64::new(0); + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn reflow_from_pbc_count() -> u64 { + REFLOW_FROM_PBC.load(Ordering::Relaxed) +} + /// RPython `bookkeeper.position_key` (bookkeeper.py:147) — the tuple /// identifying "where in the flow graph the annotator is currently /// reading/writing a value". @@ -1152,8 +1169,17 @@ impl Bookkeeper { // upstream: `locations.add(self.position_key)` if let Some(pk) = self.current_position_key() { let mut classdef_mut = classdef.borrow_mut(); + let trace_class_name = + crate::determinism_trace_enabled().then(|| classdef_mut.name.clone()); if let Some(attrdef) = classdef_mut.attrs.get_mut(attrname) { - attrdef.read_locations.insert(pk); + if attrdef.read_locations.insert(pk) { + if let Some(class_name) = trace_class_name { + eprintln!( + "[DTRACE-ATTR] record_getattr class={class_name} attr={attrname} nloc={}", + attrdef.read_locations.len() + ); + } + } } } Ok(()) @@ -1183,6 +1209,13 @@ impl Bookkeeper { let classdesc = clsdef.borrow().classdesc.clone(); // upstream: `locations = self.getattr_locations(clsdef.classdesc, attrdef.name)` let locations = self.getattr_locations(&classdesc, attr_name)?; + if crate::determinism_trace_enabled() { + eprintln!( + "[DTRACE-ATTR] update_attr class={} attr={attr_name} nloc={}", + clsdef.borrow().name, + locations.len() + ); + } // upstream: `for position in locations: self.annotator.reflowfromposition(position)` let Some(ann) = self.annotator.borrow().upgrade() else { // upstream always has `self.annotator`; if the backlink is @@ -1192,6 +1225,7 @@ impl Bookkeeper { return Ok(()); }; for position in locations { + REFLOW_FROM_ATTR.fetch_add(1, Ordering::Relaxed); ann.reflowfromposition(&position); } // upstream: `attrdef.validate(homedef=clsdef)` @@ -1778,6 +1812,11 @@ impl Bookkeeper { self: &Rc, root: &str, ) -> Result>, AnnotatorError> { + let determinism_trace = crate::determinism_trace_enabled(); + let trace_root = root; + if determinism_trace { + eprintln!("[DTRACE-CLASS] struct_root root={trace_root}"); + } let redirected = self.redirect_withdrawn_struct_leaf(root); let root: &str = redirected.as_deref().unwrap_or(root); // Pass 1 — traverse the registry's struct-field graph from `root`, @@ -1854,6 +1893,13 @@ impl Bookkeeper { // the ordered result list without a clone. graph.push(n); } + if determinism_trace { + eprintln!( + "[DTRACE-CLASS] struct_graph root={trace_root} n={} structs={}", + graph.len(), + graph.join(",") + ); + } // Pass 2 — project each reachable node's fields into its // `classdef.attrs`. All nodes are published from pass 1, so a // field referencing another struct resolves to the registered @@ -1977,8 +2023,9 @@ impl Bookkeeper { pub fn register_trait_family( self: &Rc, base_root: &str, - base_members: HashMap, - impls: Vec<(String, HashMap)>, + // The ordered containers preserve the host class-dict insertion order. + base_members: IndexMap, + impls: Vec<(String, IndexMap)>, ) -> Result>, AnnotatorError> { // Base first — its identity-keyed HostObject must be published in // `pyre_struct_root_classes` before the subclasses intern, so each @@ -2976,6 +3023,7 @@ impl Bookkeeper { let positions = pbc_family.read_locations(); if let Some(ann) = self.annotator.borrow().upgrade() { for pos in positions { + REFLOW_FROM_PBC.fetch_add(1, Ordering::Relaxed); ann.reflowfromposition(&pos); } } @@ -6514,7 +6562,7 @@ mod tests { use crate::annotator::model::SomeValue; use crate::translator::rtyper::rpbc::MethodsPBCRepr; use crate::translator::rtyper::rtyper::RPythonTyper; - use std::collections::HashMap; + use indexmap::IndexMap; let ann = RPythonAnnotator::new(None, None, None, false); let bk = ann.bookkeeper.clone(); @@ -6532,11 +6580,12 @@ mod tests { // Base trait `T` with a default-body method `shared`; three impls // A/B/C each override `shared`. This is the ≥2-impl multi-impl // family shape that annotates classdef-less today. - let mut base_members = HashMap::new(); + // The ordered container preserves the host class-dict insertion order. + let mut base_members = IndexMap::new(); base_members.insert("shared".to_string(), member("T::shared")); let mut impls = Vec::new(); for impl_name in ["A", "B", "C"] { - let mut m = HashMap::new(); + let mut m = IndexMap::new(); m.insert( "shared".to_string(), member(&format!("{impl_name}::shared")), @@ -6608,7 +6657,7 @@ mod tests { fn register_trait_family_subclass_only_method_surfaces_on_base() { use crate::annotator::classdesc::ClassDef; use crate::annotator::model::SomeValue; - use std::collections::HashMap; + use indexmap::IndexMap; let ann = RPythonAnnotator::new(None, None, None, false); let bk = ann.bookkeeper.clone(); @@ -6623,12 +6672,13 @@ mod tests { // required-method-with-no-default-body shape. let mut impls = Vec::new(); for impl_name in ["A", "B", "C"] { - let mut m = HashMap::new(); + // The ordered container preserves the host class-dict insertion order. + let mut m = IndexMap::new(); m.insert("req".to_string(), member(&format!("{impl_name}::req"))); impls.push((impl_name.to_string(), m)); } let base_cd = bk - .register_trait_family("T", HashMap::new(), impls) + .register_trait_family("T", IndexMap::new(), impls) .expect("register_trait_family must succeed"); ClassDef::check_missing_attribute_update(&base_cd, "req").expect("attr population"); diff --git a/majit/majit-translate/src/annotator/classdesc.rs b/majit/majit-translate/src/annotator/classdesc.rs index 21adb578457..9becce7dba0 100644 --- a/majit/majit-translate/src/annotator/classdesc.rs +++ b/majit/majit-translate/src/annotator/classdesc.rs @@ -57,6 +57,7 @@ use std::cell::RefCell; use std::collections::{HashMap, HashSet}; use std::rc::{Rc, Weak}; use std::sync::Arc; +use std::sync::atomic::{AtomicU64, Ordering}; use super::bookkeeper::{Bookkeeper, EmulatedPbcCallKey, PositionKey}; use super::description::ClassAttrFamily; @@ -67,6 +68,14 @@ use crate::flowspace::model::{ use crate::tool::flattenrec::FlattenRecursion; use crate::translator::rtyper::rclass::ClassRepr; +// Exists to localise prepass nondeterminism (gh#1139). +static REFLOW_FROM_SUBCLASS: AtomicU64 = AtomicU64::new(0); + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn reflow_from_subclass_count() -> u64 { + REFLOW_FROM_SUBCLASS.load(Ordering::Relaxed) +} + thread_local! { /// RPython `ClassDef._see_instance_flattenrec = FlattenRecursion()` /// (classdesc.py:402). Upstream's `FlattenRecursion` inherits @@ -721,7 +730,9 @@ pub struct ClassDesc { /// (classdesc.py:506). The [`ClassDictEntry`] sum carries either /// constant values or stored `DescEntry`s produced by /// `add_source_attribute`'s mixin-FunctionType branch. - pub(crate) classdict: HashMap, + /// The ordered container is required because a `HashMap`'s iteration + /// order varies per process and these keys produce a work order. + pub(crate) classdict: IndexMap, /// RPython `self.immutable_fields` — `set(cls._immutable_fields_)`. pub immutable_fields: HashSet, /// RPython class attribute `instance_level = False` @@ -749,7 +760,7 @@ impl ClassDesc { basedesc: None, classdef: None, all_enforced_attrs: None, - classdict: HashMap::new(), + classdict: IndexMap::new(), immutable_fields: HashSet::new(), instance_level: false, detect_invalid_attrs: None, @@ -824,7 +835,8 @@ impl ClassDesc { cls: HostObject, name: Option, basedesc: Option>>, - classdict: Option>, + // The ordered container preserves the source class-dict insertion order. + classdict: Option>, ) -> Result>, AnnotatorError> { // classdesc.py:497-498 — __NOT_RPYTHON__ guard. if cls.class_has("__NOT_RPYTHON__") { @@ -1218,7 +1230,20 @@ impl ClassDesc { this: &Rc>, cls: &HostObject, ) -> Result<(), AnnotatorError> { - for (name, value) in cls.class_dict_items() { + let items = cls.class_dict_items(); + if crate::determinism_trace_enabled() { + let attrs = items + .iter() + .map(|(name, _)| name.as_str()) + .collect::>() + .join(","); + eprintln!( + "[DTRACE-CLASS] add_sources_for_class cls={} n={} attrs={attrs}", + cls.qualname(), + items.len() + ); + } + for (name, value) in items { Self::add_source_attribute(this, &name, value, false)?; } Ok(()) @@ -1313,7 +1338,9 @@ impl ClassDesc { // classdesc.py:686-689 — classsources = {attr: self for attr in classdict}. let source = AttrSource::Class(Rc::downgrade(this)); let attr_names: Vec = this.borrow().classdict.keys().cloned().collect(); - let mut classsources: HashMap = HashMap::new(); + // The ordered container is required because a `HashMap`'s iteration + // order varies per process and this map produces a work order. + let mut classsources: IndexMap = IndexMap::new(); for n in attr_names { classsources.insert(n, source.clone()); } @@ -2490,7 +2517,8 @@ impl ClassDef { /// RPython `ClassDef.setup(self, sources)` (classdesc.py:161-166). fn setup( this: &Rc>, - sources: HashMap, + // The ordered container preserves the source-attribute work order. + sources: IndexMap, ) -> Result<(), AnnotatorError> { for (name, source) in sources { Self::add_source_for_attribute(this, &name, source)?; @@ -2591,7 +2619,7 @@ impl ClassDef { if let Some(cdef) = Self::get_owner(this, attr) { return Ok(cdef); } - Self::generalize_attr_internal(this, attr, None)?; + Self::generalize_attr_internal(this, attr, None, "locate")?; Ok(this.clone()) } @@ -2901,9 +2929,9 @@ impl ClassDef { s_value: Option, ) -> Result<(), AnnotatorError> { if let Some(cdef) = Self::get_owner(this, attr) { - Self::generalize_attr_internal(&cdef, attr, s_value) + Self::generalize_attr_internal(&cdef, attr, s_value, "owner") } else { - Self::generalize_attr_internal(this, attr, s_value) + Self::generalize_attr_internal(this, attr, s_value, "self") } } @@ -2913,7 +2941,14 @@ impl ClassDef { this: &Rc>, attr: &str, s_value: Option, + trace_source: &'static str, ) -> Result<(), AnnotatorError> { + if crate::determinism_trace_enabled() { + eprintln!( + "[DTRACE-ATTR] generalize class={} attr={attr} via={trace_source}", + this.borrow().name + ); + } let mut newattr = Attribute::new(attr); if let Some(sv) = s_value { newattr.s_value = sv; @@ -3032,6 +3067,7 @@ impl ClassDef { && let Some(ann) = bk.annotator.borrow().upgrade() { for position in positions { + REFLOW_FROM_SUBCLASS.fetch_add(1, Ordering::Relaxed); ann.reflowfromposition(&position); } } diff --git a/majit/majit-translate/src/annotator/listdef.rs b/majit/majit-translate/src/annotator/listdef.rs index 1ba2567a07e..53862859ff0 100644 --- a/majit/majit-translate/src/annotator/listdef.rs +++ b/majit/majit-translate/src/annotator/listdef.rs @@ -36,6 +36,7 @@ use std::cell::{Cell, RefCell}; use std::fmt; use std::rc::{Rc, Weak}; +use std::sync::atomic::{AtomicU64, Ordering}; use indexmap::IndexSet; @@ -44,6 +45,30 @@ use super::repr_guard::ReprGuard; use super::bookkeeper::{Bookkeeper, PositionKey}; use super::model::{AnnotatorError, SomeList, SomeValue, UnionError}; +// Exists to localise prepass nondeterminism (gh#1139). +static REFLOW_FROM_LISTITEM: AtomicU64 = AtomicU64::new(0); + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn reflow_from_listitem_count() -> u64 { + REFLOW_FROM_LISTITEM.load(Ordering::Relaxed) +} + +// Exists to localise prepass nondeterminism (gh#1139). +static LISTITEM_WIDEN: AtomicU64 = AtomicU64::new(0); + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn listitem_widen_count() -> u64 { + LISTITEM_WIDEN.load(Ordering::Relaxed) +} + +// Exists to localise prepass nondeterminism (gh#1139). +static LISTITEM_NOTIFY_UPDATE: AtomicU64 = AtomicU64::new(0); + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn listitem_notify_update_count() -> u64 { + LISTITEM_NOTIFY_UPDATE.load(Ordering::Relaxed) +} + /// RPython `class TooLateForChange(AnnotatorError)` (listdef.py:6-7). /// Raised when mutation is attempted on a `dont_change_any_more` /// listitem. @@ -243,6 +268,7 @@ impl ListItem { /// state is a guarded path that already errors via /// [`TooLateForChange`]. pub fn notify_update(&self) { + LISTITEM_NOTIFY_UPDATE.fetch_add(1, Ordering::Relaxed); let Some(bk) = self.bookkeeper.as_ref() else { return; }; @@ -250,6 +276,7 @@ impl ListItem { return; }; for position_key in &self.read_locations { + REFLOW_FROM_LISTITEM.fetch_add(1, Ordering::Relaxed); ann.reflowfromposition(position_key); } } @@ -271,6 +298,7 @@ impl ListItem { }); } self.s_value = s_new_value; + LISTITEM_WIDEN.fetch_add(1, Ordering::Relaxed); self.notify_update(); } Ok(updated) diff --git a/majit/majit-translate/src/annotator/specialize.rs b/majit/majit-translate/src/annotator/specialize.rs index 7c06bd7e6bd..31c85e6d1f7 100644 --- a/majit/majit-translate/src/annotator/specialize.rs +++ b/majit/majit-translate/src/annotator/specialize.rs @@ -428,7 +428,7 @@ impl MemoTable { // entrypoint); self.graph.defaults = self.funcdesc.defaults`. let signature = Signature::new(argnames.clone(), None, None); let pygraph = Rc::new(PyGraph { - graph: Rc::new(RefCell::new(fg)), + graph: fg.into_ref(), func, signature: RefCell::new(signature), defaults: RefCell::new(Some(defaults)), @@ -738,7 +738,7 @@ impl MemoSynth<'_> { ))); let signature = Signature::new(sub_argnames.clone(), None, None); let pygraph = Rc::new(PyGraph { - graph: Rc::new(RefCell::new(fg)), + graph: fg.into_ref(), func: func.clone(), signature: RefCell::new(signature), defaults: RefCell::new(Some(Vec::new())), diff --git a/majit/majit-translate/src/flowspace/generator.rs b/majit/majit-translate/src/flowspace/generator.rs index ed9393927d9..e832fc51234 100644 --- a/majit/majit-translate/src/flowspace/generator.rs +++ b/majit/majit-translate/src/flowspace/generator.rs @@ -6,6 +6,8 @@ use std::cell::RefCell; use std::collections::HashMap; use std::rc::Rc; +use indexmap::IndexMap; + use super::flowcontext::{FlowContextError, FlowingError}; use super::model::{ Block, BlockKey, BlockRef, BlockRefExt, ConstValue, Constant, FunctionGraph, GraphFunc, @@ -67,12 +69,12 @@ pub fn make_generatoriterator_class(func: &GraphFunc, var_names: &[String]) -> H let entry = HostObject::new_class_with_members( format!("{}.Entry", func.name), vec![], - HashMap::from([("varnames".to_string(), tuple_of_strings(var_names))]), + IndexMap::from([("varnames".to_string(), tuple_of_strings(var_names))]), ); HostObject::new_class_with_members( format!("{}.GeneratorIterator", func.name), vec![], - HashMap::from([("Entry".to_string(), ConstValue::HostObject(entry))]), + IndexMap::from([("Entry".to_string(), ConstValue::HostObject(entry))]), ) } @@ -439,7 +441,7 @@ pub fn tweak_generator_body_graph( let resume = HostObject::new_class_with_members( format!("Resume{}", mappings.len()), vec![], - HashMap::from([("_attrs_".to_string(), tuple_of_strings(&resume_varnames))]), + IndexMap::from([("_attrs_".to_string(), tuple_of_strings(&resume_varnames))]), ); // upstream generator.py:140-142 — `Resume.block = newblock; // mappings.append(Resume)`. Carry both into our list so the diff --git a/majit/majit-translate/src/flowspace/model.rs b/majit/majit-translate/src/flowspace/model.rs index 74d511c4a83..a778ca370ba 100644 --- a/majit/majit-translate/src/flowspace/model.rs +++ b/majit/majit-translate/src/flowspace/model.rs @@ -22,16 +22,89 @@ //! Rust has no class-mutable-state, this is the minimum deviation. use std::cell::RefCell; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::hash::{Hash, Hasher}; use std::rc::{Rc, Weak}; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Arc, LazyLock, Mutex, OnceLock}; +use indexmap::IndexMap; + use super::bytecode::HostCode; use crate::annotator::model::SomeValue; use crate::translator::rtyper::lltypesystem::lltype::_ptr; +// Exists to localise prepass nondeterminism (gh#1139). +static BLOCK_ADDR_REUSE: AtomicU64 = AtomicU64::new(0); +// Exists to localise prepass nondeterminism (gh#1139). +static GRAPH_ADDR_REUSE: AtomicU64 = AtomicU64::new(0); +// Exists to localise prepass nondeterminism (gh#1139). +static SEEN_BLOCK_ADDRS: OnceLock>> = OnceLock::new(); +// Exists to localise prepass nondeterminism (gh#1139). +static REUSED_BLOCK_ADDRS: OnceLock>> = OnceLock::new(); +// Exists to localise prepass nondeterminism (gh#1139). +static SEEN_GRAPH_ADDRS: OnceLock>> = OnceLock::new(); + +// Exists to localise prepass nondeterminism (gh#1139). +fn record_block_address(block: &BlockRef) { + if !crate::determinism_trace_enabled() { + return; + } + let addr = Rc::as_ptr(block) as usize; + let inserted = SEEN_BLOCK_ADDRS + .get_or_init(|| Mutex::new(HashSet::new())) + .lock() + .expect("block address-reuse detector mutex poisoned") + .insert(addr); + if !inserted { + BLOCK_ADDR_REUSE.fetch_add(1, Ordering::Relaxed); + REUSED_BLOCK_ADDRS + .get_or_init(|| Mutex::new(HashSet::new())) + .lock() + .expect("reused-block address detector mutex poisoned") + .insert(addr); + } +} + +// Exists to localise prepass nondeterminism (gh#1139). +fn record_graph_address(graph: &GraphRef) { + if !crate::determinism_trace_enabled() { + return; + } + let addr = Rc::as_ptr(graph) as usize; + let inserted = SEEN_GRAPH_ADDRS + .get_or_init(|| Mutex::new(HashSet::new())) + .lock() + .expect("graph address-reuse detector mutex poisoned") + .insert(addr); + if !inserted { + GRAPH_ADDR_REUSE.fetch_add(1, Ordering::Relaxed); + } +} + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn block_addr_reuse_count() -> u64 { + BLOCK_ADDR_REUSE.load(Ordering::Relaxed) +} + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn graph_addr_reuse_count() -> u64 { + GRAPH_ADDR_REUSE.load(Ordering::Relaxed) +} + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn block_address_was_reused(block: &BlockRef) -> bool { + if !crate::determinism_trace_enabled() { + return false; + } + let addr = Rc::as_ptr(block) as usize; + REUSED_BLOCK_ADDRS + .get_or_init(|| Mutex::new(HashSet::new())) + .lock() + .expect("reused-block address detector mutex poisoned") + .contains(&addr) +} + // RPython `Variable.annotation` holds a `SomeObject` subclass instance // (annotator/model.py:SomeObject). Rust stores `Option>` // directly — `flowspace` and `annotator` are sibling modules inside @@ -106,7 +179,9 @@ enum HostObjectKind { /// function, property, …)이므로 `ConstValue` carrier 로 담는다. Class { bases: Vec, - members: Mutex>, + /// The ordered container is required because a `HashMap`'s iteration + /// order varies per process and these entries produce a work order. + members: Mutex>, reusable_prebuilt_instance: OnceLock, }, /// Python module object. `members` 는 module dict — `getattr` 조회 @@ -129,7 +204,9 @@ enum HostObjectKind { Instance { class_obj: HostObject, args: Vec, - instance_dict: Mutex>, + /// The ordered container is required because a `HashMap`'s iteration + /// order varies per process and these keys produce a work order. + instance_dict: Mutex>, }, /// `Constant.value` 에 담긴 임의의 host object — flowspace 가 구조 /// 를 모르지만 보존해야 하는 값(예: 포팅되지 않은 `ConstantData` @@ -495,7 +572,7 @@ impl HostObject { module_name, kind: HostObjectKind::Class { bases, - members: Mutex::new(HashMap::new()), + members: Mutex::new(IndexMap::new()), reusable_prebuilt_instance: OnceLock::new(), }, }), @@ -510,7 +587,8 @@ impl HostObject { pub fn new_class_with_members( qualname: impl Into, bases: Vec, - members: HashMap, + // The ordered container preserves the caller's class-dict insertion order. + members: IndexMap, ) -> Self { let qualname = qualname.into(); let (name, module_name) = split_attr_name_module(&qualname); @@ -644,7 +722,7 @@ impl HostObject { kind: HostObjectKind::Instance { class_obj, args, - instance_dict: Mutex::new(HashMap::new()), + instance_dict: Mutex::new(IndexMap::new()), }, }), } @@ -2702,6 +2780,11 @@ fn clean_name(name: &str) -> String { /// returns a *new* Variable with the same prefix). static NEXT_VAR_ID: AtomicU64 = AtomicU64::new(0); +// Exists to localise prepass nondeterminism (gh#1139). +pub fn next_var_id() -> u64 { + NEXT_VAR_ID.load(Ordering::Relaxed) +} + fn alloc_var_id() -> u64 { NEXT_VAR_ID.fetch_add(1, Ordering::Relaxed) } @@ -3008,8 +3091,9 @@ pub struct Constant { impl Constant { /// RPython `Constant.__init__(value, concretetype=None)`. pub fn new(value: ConstValue) -> Self { + let id = alloc_constant_id(&value); Constant { - id: NEXT_CONSTANT_ID.fetch_add(1, Ordering::Relaxed), + id, value, concretetype: None, } @@ -3017,8 +3101,9 @@ impl Constant { /// RPython `Constant.__init__(value, concretetype)`. pub fn with_concretetype(value: ConstValue, concretetype: ConcretetypePlaceholder) -> Self { + let id = alloc_constant_id(&value); Constant { - id: NEXT_CONSTANT_ID.fetch_add(1, Ordering::Relaxed), + id, value, concretetype: Some(concretetype), } @@ -3118,6 +3203,96 @@ impl Constant { static NEXT_CONSTANT_ID: AtomicU64 = AtomicU64::new(1); +// Exists to localise prepass nondeterminism (gh#1139). +fn alloc_constant_id(value: &ConstValue) -> u64 { + let id = NEXT_CONSTANT_ID.fetch_add(1, Ordering::Relaxed); + let Some(&(from, to)) = constant_trace_window() else { + return id; + }; + if id < from || id >= to { + return id; + } + + let mut debug_value = format!("{value:?}"); + if let Some((byte_index, _)) = debug_value.char_indices().nth(120) { + debug_value.truncate(byte_index); + } + eprintln!( + "[DTRACE-CONST] id={id} kind={} val={debug_value}", + const_value_variant_name(value) + ); + if constant_trace_backtrace_enabled() { + let backtrace = std::backtrace::Backtrace::force_capture(); + eprintln!("[DTRACE-CONST-BT] id={id}\n{backtrace}\n[DTRACE-CONST-BT-END] id={id}"); + } + id +} + +// Exists to localise prepass nondeterminism (gh#1139). +fn constant_trace_backtrace_enabled() -> bool { + static ENABLED: OnceLock = OnceLock::new(); + *ENABLED.get_or_init(|| { + std::env::var("PYRE_DTRACE_CONST_BT") + .as_deref() + .is_ok_and(|value| value == "1") + }) +} + +// Exists to localise prepass nondeterminism (gh#1139). +fn constant_trace_window() -> Option<&'static (u64, u64)> { + static WINDOW: OnceLock> = OnceLock::new(); + WINDOW + .get_or_init(|| { + if !crate::determinism_trace_enabled() { + return None; + } + let from = std::env::var("PYRE_DTRACE_CONST_FROM") + .ok() + .and_then(|value| value.parse().ok()) + .unwrap_or(0); + let to = std::env::var("PYRE_DTRACE_CONST_TO") + .ok() + .and_then(|value| value.parse().ok()) + .unwrap_or(0); + (to != 0).then_some((from, to)) + }) + .as_ref() +} + +// Exists to localise prepass nondeterminism (gh#1139). +fn const_value_variant_name(value: &ConstValue) -> &'static str { + match value { + ConstValue::Atom(_) => "Atom", + ConstValue::Placeholder => "Placeholder", + ConstValue::Int(_) => "Int", + ConstValue::Int128(_) => "Int128", + ConstValue::UInt128(_) => "UInt128", + ConstValue::Float(_) => "Float", + ConstValue::Dict(_) => "Dict", + ConstValue::ByteStr(_) => "ByteStr", + ConstValue::UniStr(_) => "UniStr", + ConstValue::Tuple(_) => "Tuple", + ConstValue::List(_) => "List", + ConstValue::Bool(_) => "Bool", + ConstValue::None => "None", + ConstValue::Code(_) => "Code", + ConstValue::Function(_) => "Function", + ConstValue::Graphs(_) => "Graphs", + ConstValue::LowLevelType(_) => "LowLevelType", + ConstValue::LLPtr(_) => "LLPtr", + ConstValue::LLAddress(_) => "LLAddress", + ConstValue::HostObject(_) => "HostObject", + ConstValue::AddressOffset(_) => "AddressOffset", + ConstValue::SpecTag(_) => "SpecTag", + ConstValue::InheritanceId { .. } => "InheritanceId", + } +} + +// Exists to localise prepass nondeterminism (gh#1139). +pub fn next_constant_id() -> u64 { + NEXT_CONSTANT_ID.load(Ordering::Relaxed) +} + impl PartialEq for Constant { fn eq(&self, other: &Self) -> bool { if Self::uses_hashable_identity_fallback(&self.value) @@ -3828,7 +4003,9 @@ impl Block { /// graph types take `Rc>`, so callers usually /// want `Block::shared(...)` instead of `Block::new(...)`. pub fn shared(inputargs: Vec) -> BlockRef { - Rc::new(RefCell::new(Block::new(inputargs))) + let block = Rc::new(RefCell::new(Block::new(inputargs))); + record_block_address(&block); + block } /// RPython `Block.is_final_block()` — `self.operations == ()`. @@ -4309,6 +4486,14 @@ pub struct FunctionGraph { } impl FunctionGraph { + /// Wrap this graph in the shared reference used throughout the + /// translation pipeline. + pub fn into_ref(self) -> GraphRef { + let graph = Rc::new(RefCell::new(self)); + record_graph_address(&graph); + graph + } + /// RPython `FunctionGraph.__init__(name, startblock, /// return_var=None)`. pub fn new(name: impl Into, startblock: BlockRef) -> Self { @@ -5888,13 +6073,27 @@ mod tests { #[test] fn new_class_with_members_seeds_initial_dict() { - let mut seed: HashMap = HashMap::new(); + // The ordered container is required because a `HashMap`'s iteration + // order varies per process and these entries produce a work order. + let mut seed: IndexMap = IndexMap::new(); seed.insert("_mixin_".into(), ConstValue::Bool(true)); + seed.insert("second".into(), ConstValue::Int(2)); let cls = HostObject::new_class_with_members("pkg.Foo", vec![], seed); match cls.class_get("_mixin_") { Some(ConstValue::Bool(true)) => {} other => panic!("expected Bool(true), got {other:?}"), } + assert_eq!(cls.class_dict_keys(), vec!["_mixin_", "second"]); + } + + #[test] + fn instance_dict_keys_preserve_insertion_order() { + let cls = HostObject::new_class("pkg.Foo", vec![]); + let instance = HostObject::new_instance(cls, vec![]); + instance.instance_set("second", ConstValue::Int(2)); + instance.instance_set("first", ConstValue::Int(1)); + + assert_eq!(instance.instance_dict_keys(), vec!["second", "first"]); } #[test] diff --git a/majit/majit-translate/src/flowspace/pygraph.rs b/majit/majit-translate/src/flowspace/pygraph.rs index 95144c7b34f..08368cf2faf 100644 --- a/majit/majit-translate/src/flowspace/pygraph.rs +++ b/majit/majit-translate/src/flowspace/pygraph.rs @@ -15,7 +15,6 @@ //! inheritance provides upstream. use std::cell::{Cell, RefCell}; -use std::rc::Rc; use super::argument::Signature; use super::bytecode::HostCode; @@ -81,7 +80,7 @@ impl PyGraph { // upstream: `self.signature = code.signature` / `self.defaults = ...`. PyGraph { - graph: Rc::new(RefCell::new(graph)), + graph: graph.into_ref(), signature: RefCell::new(code.signature.clone()), defaults: RefCell::new(Some(func.defaults.clone())), access_directly: Cell::new(false), diff --git a/majit/majit-translate/src/lib.rs b/majit/majit-translate/src/lib.rs index 91fd2e10705..073ac6ef784 100644 --- a/majit/majit-translate/src/lib.rs +++ b/majit/majit-translate/src/lib.rs @@ -65,6 +65,14 @@ pub use pipeline::{ use serde::{Deserialize, Serialize}; +// Exists to localise prepass nondeterminism (gh#1139). +pub(crate) fn determinism_trace_enabled() -> bool { + static ENABLED: std::sync::OnceLock = std::sync::OnceLock::new(); + *ENABLED.get_or_init(|| { + std::env::var_os("PYRE_DETERMINISM_TRACE").is_some_and(|value| value == "1") + }) +} + /// Configuration for the canonical graph/pipeline analyzer. /// /// Consumers supply graph-rewrite metadata such as virtualizable diff --git a/majit/majit-translate/src/translator/rtyper/cutover.rs b/majit/majit-translate/src/translator/rtyper/cutover.rs index ceecfddc392..af9b8b33916 100644 --- a/majit/majit-translate/src/translator/rtyper/cutover.rs +++ b/majit/majit-translate/src/translator/rtyper/cutover.rs @@ -2232,7 +2232,7 @@ fn build_stub_pygraph_with_result_shell( ))); startblock.closeblock(vec![link]); Rc::new(PyGraph { - graph: Rc::new(RefCell::new(graph_inner)), + graph: graph_inner.into_ref(), func, signature: RefCell::new(signature), defaults: RefCell::new(Some(Vec::new())), @@ -3255,11 +3255,35 @@ fn emit_disposition_histogram(phase: &str, reasons: &[String]) { } } +// Exists to localise prepass nondeterminism (gh#1139). +fn emit_determinism_trace(phase: &str, index: usize, canonical_key: &str) { + eprintln!( + "[DTRACE] {phase} {index} {canonical_key} var={} const={} reflow={} block={} rf_list={} rf_sub={} rf_attr={} rf_pbc={} rf_notify={} widen={} nupd={} breuse={} greuse={} nhitreused={}", + crate::flowspace::model::next_var_id(), + crate::flowspace::model::next_constant_id(), + crate::annotator::annrpython::reflow_count(), + crate::annotator::annrpython::processblock_count(), + crate::annotator::listdef::reflow_from_listitem_count(), + crate::annotator::classdesc::reflow_from_subclass_count(), + crate::annotator::bookkeeper::reflow_from_attr_count(), + crate::annotator::bookkeeper::reflow_from_pbc_count(), + crate::annotator::annrpython::reflow_from_notify_count(), + crate::annotator::listdef::listitem_widen_count(), + crate::annotator::listdef::listitem_notify_update_count(), + crate::flowspace::model::block_addr_reuse_count(), + crate::flowspace::model::graph_addr_reuse_count(), + crate::annotator::annrpython::notify_hit_on_reused_count(), + ); +} + fn run_two_phase_prepass_inner( call_registry: &PyreCallRegistry, candidate_graphs: &HashSet, function_graphs: &crate::codewriter::call::GraphStore, ) { + // Exists to localise prepass nondeterminism (gh#1139). + let determinism_trace = crate::determinism_trace_enabled(); + // Deterministic order (R3): candidate_graphs is a HashSet; iterating it // directly would make classdef numbering (and thus Match/Skip // classification) vary run-to-run. @@ -3271,11 +3295,14 @@ fn run_two_phase_prepass_inner( // ── Phase A — annotate-all over the portal closure ─────────────── let mut phase_a_reasons: Vec = Vec::new(); - for path in &paths { + for (index, path) in paths.iter().enumerate() { let Some(legacy) = function_graphs.get(path) else { continue; }; let session_at_entry = SubjectSessionSnapshot::capture(call_registry); + if determinism_trace { + emit_determinism_trace("phaseA", index, &path.canonical_key()); + } let attempt = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { drive_subject(legacy, call_registry, /* do_rtype = */ false) })); @@ -3383,7 +3410,7 @@ fn run_two_phase_prepass_inner( // ── Phase B — rtype-all with per-graph isolation ───────────────── // Writes its rtype_skipped set into the cache incrementally so partial // progress survives even if this call is cut short. - run_phase_b_rtype_isolated(call_registry, function_graphs); + run_phase_b_rtype_isolated(call_registry, function_graphs, determinism_trace, &paths); } /// Phase B: rtype every annotated block in one whole-program pass, tolerant of @@ -3397,12 +3424,31 @@ fn run_two_phase_prepass_inner( fn run_phase_b_rtype_isolated( call_registry: &PyreCallRegistry, lift_sources: &crate::codewriter::call::GraphStore, + determinism_trace: bool, + paths: &[&crate::parse::CallPath], ) { use crate::flowspace::model::{BlockRef, GraphKey, GraphRef}; let Ok((annotator, rtyper)) = call_registry.ensure_session() else { return; }; let mut phase_b_reasons: Vec = Vec::new(); + // Exists to localise prepass nondeterminism (gh#1139). + let phase_b_trace_paths = determinism_trace.then(|| { + let cache = call_registry.two_phase(); + paths + .iter() + .enumerate() + .filter_map(|(index, path)| { + let canonical_key = path.canonical_key(); + cache + .subjects + .get(&canonical_key) + .map(|subject| (subject.graph_key.clone(), (index, canonical_key))) + }) + .collect::>() + }); + // Exists to localise prepass nondeterminism (gh#1139). + let mut phase_b_traced = determinism_trace.then(Vec::new); // Upstream `RPythonTyper.specialize()` step 1 (rtyper.py:180-181): // `if not dont_simplify_again: self.annotator.simplify()`. pyre's @@ -3428,18 +3474,63 @@ fn run_phase_b_rtype_isolated( { use crate::flowspace::model::BlockKey; use crate::translator::transform::{ - fully_annotated_blocks, transform_dead_code, transform_dead_op_vars, + cutoff_block_trace, fully_annotated_blocks, transform_dead_code, transform_dead_op_vars, }; let annotated_blocks = fully_annotated_blocks(&annotator); + if determinism_trace { + use md5::{Digest, Md5}; + + let graph_names: Vec = { + let annotated = annotator.annotated.borrow(); + annotated_blocks + .iter() + .map(|block| { + annotated + .get(&BlockKey::of(block)) + .and_then(|graph| graph.as_ref()) + .map(|graph| graph.borrow().name.clone()) + .unwrap_or_else(|| "".to_string()) + }) + .collect() + }; + let mut digest = Md5::new(); + for name in &graph_names { + digest.update((name.len() as u64).to_le_bytes()); + digest.update(name.as_bytes()); + } + eprintln!( + "[DTRACE-CUTSET] count={} md5={:x} first={:?}", + graph_names.len(), + digest.finalize(), + &graph_names[..graph_names.len().min(20)], + ); + } + // Pass 1 — `transform_dead_code` (transform.py:145): prune the // unfollowed const-switch dead arms. Per-block panic isolation // (`cutoff_alwaysraising_block`'s consistency asserts can fire on an // unrelated malformed always-raising block). + let mut cutoff_panics = 0usize; for block in &annotated_blocks { - let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let trace = determinism_trace.then(|| cutoff_block_trace(&annotator, block)); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { transform_dead_code(&annotator, std::slice::from_ref(block)); })); + if let Err(payload) = result { + cutoff_panics += 1; + if let Some(trace) = trace { + let payload = payload + .downcast_ref::<&str>() + .map(|message| (*message).to_string()) + .or_else(|| payload.downcast_ref::().cloned()) + .unwrap_or_else(|| "".to_string()); + eprintln!("[DTRACE-CUTPANIC] {trace} payload={payload:?}"); + } + } + } + if determinism_trace { + eprintln!("[DTRACE-CUTPANIC] total={cutoff_panics}"); } // Pass 2 — `transform_dead_op_vars` (simplify.py:422, transform.py:137): @@ -3515,6 +3606,20 @@ fn run_phase_b_rtype_isolated( continue; } let session_at_entry = SubjectSessionSnapshot::capture_fixed_only(call_registry); + if let (Some(g), Some(trace_paths), Some(traced)) = + (&gopt, &phase_b_trace_paths, &mut phase_b_traced) + { + let graph_key = GraphKey::of(g); + if !traced.contains(&graph_key) { + traced.push(graph_key.clone()); + if let Some((_, (index, canonical_key))) = trace_paths + .iter() + .find(|(candidate, _)| candidate == &graph_key) + { + emit_determinism_trace("phaseB", *index, canonical_key); + } + } + } let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { rtyper.specialize_block(&block) })); diff --git a/majit/majit-translate/src/translator/rtyper/flowspace_adapter.rs b/majit/majit-translate/src/translator/rtyper/flowspace_adapter.rs index f9847d21239..eb4873e5054 100644 --- a/majit/majit-translate/src/translator/rtyper/flowspace_adapter.rs +++ b/majit/majit-translate/src/translator/rtyper/flowspace_adapter.rs @@ -3704,7 +3704,7 @@ pub fn function_graph_to_flowspace( exceptblock_ref.borrow_mut().inputargs = except_inputargs; } - let graph_ref = Rc::new(RefCell::new(graph)); + let graph_ref = graph.into_ref(); // Map the canonical finals so any legacy Link targeting them // resolves to the flowspace finals constructed above. diff --git a/majit/majit-translate/src/translator/rtyper/normalizecalls.rs b/majit/majit-translate/src/translator/rtyper/normalizecalls.rs index 91adac5ccf9..0affb7d5dfb 100644 --- a/majit/majit-translate/src/translator/rtyper/normalizecalls.rs +++ b/majit/majit-translate/src/translator/rtyper/normalizecalls.rs @@ -1104,7 +1104,7 @@ pub fn create_instantiate_function( // returnblock/exceptblock allocated inside the ctor. let graph = FunctionGraph::new(name, block.clone()); let returnblock = graph.returnblock.clone(); - let graph_rc = Rc::new(RefCell::new(graph)); + let graph_rc = graph.into_ref(); // upstream: `block.closeblock(Link([v], graph.returnblock))`. let link = Link::new(vec![v_result.clone()], Some(returnblock.clone()), None); diff --git a/majit/majit-translate/src/translator/rtyper/pyre_call_registry.rs b/majit/majit-translate/src/translator/rtyper/pyre_call_registry.rs index 2314e6dfe3b..077e2434754 100644 --- a/majit/majit-translate/src/translator/rtyper/pyre_call_registry.rs +++ b/majit/majit-translate/src/translator/rtyper/pyre_call_registry.rs @@ -60,6 +60,7 @@ //! (`pair_simple_call → FunctionDesc.specialize → cachedgraph → //! FunctionRepr.call`) all see the registry's entries. +use indexmap::IndexMap; use std::cell::RefCell; use std::collections::{HashMap, HashSet}; use std::rc::Rc; @@ -335,14 +336,15 @@ impl PyreCallRegistry { families: &[crate::codewriter::call::TraitFamilyRegistration], ) { for family in families { - let impls: Vec<(String, HashMap)> = family + // The ordered container preserves the host class-dict insertion order. + let impls: Vec<(String, IndexMap)> = family .impl_roots .iter() - .map(|root| (root.clone(), HashMap::new())) + .map(|root| (root.clone(), IndexMap::new())) .collect(); if let Err(e) = self.bookkeeper - .register_trait_family(&family.base_root, HashMap::new(), impls) + .register_trait_family(&family.base_root, IndexMap::new(), impls) { eprintln!( "register_trait_families: base {:?} failed: {e:?}", @@ -415,12 +417,19 @@ impl PyreCallRegistry { // each registry root on its post-crate suffix. let mut full_by_stripped: std::collections::HashMap<&str, &str> = std::collections::HashMap::new(); - for key in reg.fields.keys() { + let mut field_keys: Vec<_> = reg.fields.keys().collect(); + // Pin HashMap order so first-writer-wins suffix resolution is stable. + field_keys.sort(); + for key in field_keys { if let Some((_, rest)) = key.split_once("::") { full_by_stripped.entry(rest).or_insert(key.as_str()); } } - for (key, entry) in self.entries.borrow().iter() { + let entries = self.entries.borrow(); + let mut sorted_entries: Vec<_> = entries.iter().collect(); + // Pin HashMap order because class_set preserves member insertion order. + sorted_entries.sort_by_cached_key(|(key, _)| key.segments().join("::")); + for (key, entry) in sorted_entries { let segs = key.segments(); let [.., owner, method] = segs else { continue; @@ -659,7 +668,11 @@ impl PyreCallRegistry { &self, graph: &crate::flowspace::model::GraphRef, ) -> Option<(FunctionPathKey, Rc)> { - for (key, entry) in self.entries.borrow().iter() { + let entries = self.entries.borrow(); + let mut sorted_entries: Vec<_> = entries.iter().collect(); + // Pin HashMap order so the first matching cached graph is stable. + sorted_entries.sort_by_cached_key(|(key, _)| key.segments().join("::")); + for (key, entry) in sorted_entries { let fd = entry.function_desc.borrow(); let cache = fd.cache.borrow(); if cache.values().any(|pg| Rc::ptr_eq(&pg.graph, graph)) { @@ -746,7 +759,7 @@ impl PyreCallRegistry { .skip(1) .all(|s| !starts_with_uppercase(s)); let entries_borrow = self.entries.borrow(); - let matches: Vec<&Rc> = entries_borrow + let mut matches: Vec<(&FunctionPathKey, &Rc)> = entries_borrow .iter() .filter(|(k, e)| { if !e.host_object.is_user_function() { @@ -784,10 +797,11 @@ impl PyreCallRegistry { } true }) - .map(|(_, e)| e) .collect(); + // Pin HashMap order so a converged leaf match selects a stable entry. + matches.sort_by_cached_key(|(key, _)| key.segments().join("::")); if matches.len() == 1 { - return Some(matches[0].clone()); + return Some(matches[0].1.clone()); } if !matches.is_empty() { // Multi-alias convergence: free-function registration @@ -797,10 +811,12 @@ impl PyreCallRegistry { // (`HostObject`'s `PartialEq` is Arc-pointer equality at // `flowspace/model.rs:208`), the alias cluster is // unambiguous. - let first_host = matches[0].host_object.clone(); - let all_same = matches.iter().all(|e| e.host_object == first_host); + let first_host = matches[0].1.host_object.clone(); + let all_same = matches + .iter() + .all(|(_, entry)| entry.host_object == first_host); if all_same { - return Some(matches[0].clone()); + return Some(matches[0].1.clone()); } } None diff --git a/majit/majit-translate/src/translator/rtyper/rpbc.rs b/majit/majit-translate/src/translator/rtyper/rpbc.rs index 0b31416b337..4ddf936e3ca 100644 --- a/majit/majit-translate/src/translator/rtyper/rpbc.rs +++ b/majit/majit-translate/src/translator/rtyper/rpbc.rs @@ -2733,7 +2733,7 @@ impl SmallFunctionSetPBCRepr { // The dispatcher graph is synthesized — no host-side Python // function backing — so we deliberately do not wrap it in a // `PyGraph` (which expects a real `GraphFunc + HostCode`). - Ok(Rc::new(RefCell::new(graph))) + Ok(graph.into_ref()) } /// RPython `compression_function(r_set)` (rpbc.py:529-545). @@ -3005,7 +3005,7 @@ impl SmallFunctionSetPBCRepr { startblock.closeblock(vec![entry_link]); graph.startblock = startblock; - Ok(Rc::new(RefCell::new(graph))) + Ok(graph.into_ref()) } /// RPython `SmallFunctionSetPBCRepr.convert_desc(self, funcdesc)` diff --git a/majit/majit-translate/src/translator/rtyper/rtyper.rs b/majit/majit-translate/src/translator/rtyper/rtyper.rs index 49e102c7574..633863ac298 100644 --- a/majit/majit-translate/src/translator/rtyper/rtyper.rs +++ b/majit/majit-translate/src/translator/rtyper/rtyper.rs @@ -3208,7 +3208,7 @@ pub(crate) fn helper_pygraph_from_graph( func: GraphFunc, ) -> PyGraph { PyGraph { - graph: Rc::new(RefCell::new(graph)), + graph: graph.into_ref(), func, signature: RefCell::new(Signature::new(argnames, None, None)), defaults: RefCell::new(Some(Vec::new())), diff --git a/majit/majit-translate/src/translator/transform.rs b/majit/majit-translate/src/translator/transform.rs index 3e5d56e5902..179ca76b31f 100644 --- a/majit/majit-translate/src/translator/transform.rs +++ b/majit/majit-translate/src/translator/transform.rs @@ -287,6 +287,80 @@ pub fn transform_dead_code(ann: &RPythonAnnotator, block_subset: &[BlockRef]) { } } +/// Stable, process-comparable identification for dead-code cutoff diagnostics. +pub(crate) struct CutoffBlockTrace { + graph_name: String, + n: usize, + total: usize, + block_position: Option, + operations: usize, +} + +impl std::fmt::Display for CutoffBlockTrace { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "graph={:?} n={} total={} block={} ops={}", + self.graph_name, + self.n, + self.total, + self.block_position + .map(|position| position.to_string()) + .unwrap_or_else(|| "none".to_string()), + self.operations, + ) + } +} + +fn cutoff_block_trace_with_counts( + ann: &RPythonAnnotator, + block: &BlockRef, + n: usize, + total: usize, +) -> CutoffBlockTrace { + let graph = ann + .annotated + .borrow() + .get(&BlockKey::of(block)) + .and_then(|graph| graph.clone()); + let (graph_name, block_position) = match graph { + Some(graph) => { + let graph = graph.borrow(); + let position = graph + .iterblocks() + .iter() + .position(|candidate| Rc::ptr_eq(candidate, block)); + (graph.name.clone(), position) + } + None => ("".to_string(), None), + }; + CutoffBlockTrace { + graph_name, + n, + total, + block_position, + operations: total, + } +} + +/// Capture the same block identification used by `[DTRACE-CUT]` so the +/// per-block panic isolator can name a failed cutoff without using pointers. +pub(crate) fn cutoff_block_trace(ann: &RPythonAnnotator, block: &BlockRef) -> CutoffBlockTrace { + let (n, total) = { + let blk = block.borrow(); + let n = blk + .operations + .iter() + .position(|op| match &op.result { + Hlvalue::Variable(v) => v.annotation.borrow().is_none(), + Hlvalue::Constant(_) => false, + }) + .unwrap_or(blk.operations.len()); + (n, blk.operations.len()) + }; + cutoff_block_trace_with_counts(ann, block, n, total) +} + /// RPython `transform.py:167-198` — `cutoff_alwaysraising_block(self, block)`. /// /// ```python @@ -332,6 +406,10 @@ pub fn cutoff_alwaysraising_block(ann: &RPythonAnnotator, block: &BlockRef) { .unwrap_or(blk.operations.len()); (n, blk.operations.len()) }; + if crate::determinism_trace_enabled() { + let trace = cutoff_block_trace_with_counts(ann, block, n, total); + eprintln!("[DTRACE-CUT] {trace}"); + } // upstream: `assert 0 <= n < len(block.operations)`. assert!( n < total, diff --git a/majit/majit-translate/src/translator/translator.rs b/majit/majit-translate/src/translator/translator.rs index b69e3b9f19d..80e3c25bf6a 100644 --- a/majit/majit-translate/src/translator/translator.rs +++ b/majit/majit-translate/src/translator/translator.rs @@ -396,7 +396,7 @@ impl TranslationContext { // gating. } let pygraph = Rc::new(PyGraph { - graph: Rc::new(RefCell::new(graph)), + graph: graph.into_ref(), func: graph_func.clone(), signature: RefCell::new(code.signature.clone()), defaults: RefCell::new(Some(graph_func.defaults.clone())),