From 70c50583f41b357dc19aa7056af5050007340777 Mon Sep 17 00:00:00 2001 From: jasisz Date: Fri, 25 Sep 2026 09:06:44 +0200 Subject: [PATCH 1/2] Release a consumed tuple or Option box on the VM The generated loop hands an answer module's state out of the run in an Option, inside a tuple, and takes the answer back in another tuple. On the VM the Option box and the tuples kept no holder count, so after a match had taken them apart they still counted as holders of the state, and every answer copied the module's Maps. #1424 fixed this on the Rust side only. Tuples and the boxes of Option.Some, Result.Ok and Result.Err now count their off-stack holders the way maps, vectors and records do: every entry, global and constant that stores one registers itself. A match whose subject nothing reads afterwards (a temporary, or a local at its last use) now releases what it takes apart: POP_CONSUMED after a tuple arm empties the tuple, and MATCH_UNWRAP with the consume bit empties the box, each only when nothing off the stack and no stack cell holds the tuple or box, and only when a Map or Vector inside makes it worth the walk. `?` does the same for an Ok box through PROPAGATE_ERR_CONSUMED. aver run --profile now also prints how many map entries the writes that were not in place copied. Release build without LTO, run_owned_answer_state scaled to 2000 requests against a 100k-entry Map: 1.84 s -> 0.13 s. The fixture as checked in (200 requests, 1k entries) is 0.06 s either way. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + aver-memory/src/arena.rs | 94 ++++--- aver-memory/src/lib.rs | 58 ++++- aver-memory/src/memory.rs | 20 +- src/main/commands.rs | 4 + src/vm/compiler/mir.rs | 52 +++- src/vm/execute/dispatch.rs | 22 +- src/vm/execute/slots.rs | 69 +++++ src/vm/execute/tests.rs | 4 +- src/vm/opcode.rs | 21 +- tests/mir_vm_codegen.rs | 9 + tests/vm_consumed_destructure.rs | 417 +++++++++++++++++++++++++++++++ 12 files changed, 716 insertions(+), 55 deletions(-) create mode 100644 tests/vm_consumed_destructure.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 07568de6a..60e53fde0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,7 @@ The generated loop is now written from the program's source alone, and the manif - **A `main` that answers `Err` exits non-zero on wasm-gc and wasip2**, with the error on stderr on wasm-gc, as it already did on the VM and in generated Rust. Both wasm targets used to exit zero. - **`aver replay` of a run whose `main` answered `Err` matches.** The VM replay compared a runtime error against the recorded `Err` value and always reported a mismatch. - **The VM updates a Map in a field of a record in place when the record is consumed.** `State.update(state, counts = Map.set(state.counts, k, v), served = state.served + 1)` used to copy the whole Map on every call, even with `state` held by nothing else: the base of the update and the local, still needed for `served`, both held the record while the Map was set. A field that a record update or a record literal reads once, while every other read of the local in it is of another field and the local is not read afterwards, is now taken out of the record first, and the runtime does it only when exactly the holders the compiler accounted for hold the record. An update whose base nothing else holds moves the fields it keeps into the new record, so the dead base no longer keeps the next update of another field copying. In a release build, two thousand requests against a hundred-thousand-key Map now run in 0.06 s instead of 2.3 s, and ten thousand in 0.06 s instead of 9.6 s. A field two levels down (`setting.window.created`) is still copied; take the inner record out first. +- **A generated loop updates an answer module's state in place on the VM too.** The loop hands the state out of the run in an `Option` and takes the answer back in a tuple, and on the VM the `Option` box and the tuple went on counting as holders of the state after the match had taken them apart, so every answer copied the module's Maps. A tuple, or the box of an `Option.Some`, `Result.Ok` or `Result.Err`, now counts its holders as maps, vectors and records do, and a `match` or `?` whose subject nothing reads afterwards releases what it holds when nothing else holds the tuple or box. The same goes for any program that carries a record through `Option`, `Result` or a tuple from one step to the next. `aver run --profile` also reports how many map entries the writes that were not in place copied. In a release build, 2000 requests against a 100,000-entry Map in the answer module's state run in 0.13 s instead of 1.8 s. - **A wait over sockets and jobs keeps watching its sockets after a job outside its set settles.** The wake from that job used to end the socket poll, and the wait then slept out the rest of its timeout on the job engine alone, missing sockets that became ready meanwhile and never reporting them. The VM, generated Rust and the wasm-gc native host now share one wait loop that polls the whole set again. - **A handle whose slot the engine has forgotten answers `work: unknown job` on the VM**, as it already did elsewhere, instead of claiming another job kind started it. Which kind began a job is now kept in the job's own slot, so nothing a job kind keeps grows with the number of jobs it starts. - **The VM runs a function whose bytecode is larger than 32 KiB.** Jump offsets were sixteen bits and a longer forward jump wrapped into a backward one, which crashed `aver verify` on large generated trace laws. diff --git a/aver-memory/src/arena.rs b/aver-memory/src/arena.rs index b9cb59f7e..1bbdf4d17 100644 --- a/aver-memory/src/arena.rs +++ b/aver-memory/src/arena.rs @@ -195,7 +195,7 @@ impl Arena { let idx = self.push(ArenaEntry::String(s)); NanValue::new_string(idx) } - ArenaEntry::Tuple(items) => { + ArenaEntry::Tuple { items, .. } => { let imported: Vec = items.iter().map(|v| self.deep_import(*v, source)).collect(); let idx = self.push_tuple(imported); @@ -286,9 +286,9 @@ impl Arena { }); NanValue::new_variant(idx) } - ArenaEntry::Boxed(inner) => { + ArenaEntry::Boxed { value: inner, .. } => { let imported = self.deep_import(inner, source); - let idx = self.push(ArenaEntry::Boxed(imported)); + let idx = self.push_boxed(imported); NanValue::encode(value.tag(), ARENA_REF_BIT | (idx as u64)) } // Fn/Builtin/Namespace — should not appear in independent product results @@ -307,10 +307,7 @@ impl Arena { /// of marking sites for maps, vectors, and records. #[inline(always)] pub fn note_held_elsewhere(&mut self, value: NanValue) { - if value.is_heap_map() - || (value.is_vector() && !value.is_empty_vector_immediate()) - || value.is_record() - { + if value.counts_holders() { self.mark_held_elsewhere(value.arena_index()); } } @@ -326,7 +323,9 @@ impl Arena { match self.get_mut(index) { ArenaEntry::Map { holder_count, .. } | ArenaEntry::Vector { holder_count, .. } - | ArenaEntry::Record { holder_count, .. } => { + | ArenaEntry::Record { holder_count, .. } + | ArenaEntry::Tuple { holder_count, .. } + | ArenaEntry::Boxed { holder_count, .. } => { *holder_count = holder_count.saturating_add(1); } _ => {} @@ -337,10 +336,7 @@ impl Arena { /// physically stopped holding `value`. #[inline(always)] fn release_held_elsewhere(&mut self, value: NanValue) { - if value.is_heap_map() - || (value.is_vector() && !value.is_empty_vector_immediate()) - || value.is_record() - { + if value.counts_holders() { self.release_holder(value.arena_index()); } } @@ -350,7 +346,9 @@ impl Arena { match self.get_mut(index) { ArenaEntry::Map { holder_count, .. } | ArenaEntry::Vector { holder_count, .. } - | ArenaEntry::Record { holder_count, .. } => { + | ArenaEntry::Record { holder_count, .. } + | ArenaEntry::Tuple { holder_count, .. } + | ArenaEntry::Boxed { holder_count, .. } => { // Saturation is sticky. Once exact cardinality is lost, the // safe answer is "held" forever rather than a future false 0. if *holder_count == u32::MAX { @@ -384,8 +382,8 @@ impl Arena { #[inline(never)] fn note_entry_holds_takeable(&mut self, entry: &ArenaEntry) { match entry { - ArenaEntry::Boxed(value) => self.note_held_elsewhere(*value), - ArenaEntry::Tuple(items) => { + ArenaEntry::Boxed { value, .. } => self.note_held_elsewhere(*value), + ArenaEntry::Tuple { items, .. } => { for value in items { self.note_held_elsewhere(*value); } @@ -492,6 +490,9 @@ impl Arena { if self.holds_any_map || self.holds_any_vector || self.holds_any_record { self.note_entry_holds_takeable(&entry); } + if matches!(entry, ArenaEntry::Tuple { .. } | ArenaEntry::Boxed { .. }) { + self.holds_any_record = true; + } return self.push_heap(entry); } } @@ -977,7 +978,10 @@ impl Arena { self.push(ArenaEntry::String(Rc::from(s))) } pub fn push_boxed(&mut self, val: NanValue) -> u32 { - self.push(ArenaEntry::Boxed(val)) + self.push(ArenaEntry::Boxed { + value: val, + holder_count: 0, + }) } pub fn push_record(&mut self, type_id: u32, fields: Vec) -> u32 { self.push(ArenaEntry::Record { @@ -1017,10 +1021,7 @@ impl Arena { for (key, value) in map.values() { all_immediate &= key.is_immediate() && value.is_immediate(); for child in [*key, *value] { - if child.is_heap_map() - || (child.is_vector() && !child.is_empty_vector_immediate()) - || child.is_record() - { + if child.counts_holders() { held.get_or_insert_default().push(child); } } @@ -1038,7 +1039,10 @@ impl Arena { }) } pub fn push_tuple(&mut self, items: Vec) -> u32 { - self.push(ArenaEntry::Tuple(items)) + self.push(ArenaEntry::Tuple { + items, + holder_count: 0, + }) } /// Store a vector, marking every map or vector it holds as held by this /// entry — the vector spelling of [`Arena::push_map`]'s marking pass, made @@ -1059,11 +1063,7 @@ impl Arena { if child.heap_index().is_some() { all_immediate = false; } - if marking - && (child.is_heap_map() - || (child.is_vector() && !child.is_empty_vector_immediate()) - || child.is_record()) - { + if marking && child.counts_holders() { held.get_or_insert_default().push(*child); } } @@ -1131,7 +1131,7 @@ impl Arena { } pub fn get_boxed(&self, index: u32) -> NanValue { match self.get(index) { - ArenaEntry::Boxed(v) => *v, + ArenaEntry::Boxed { value, .. } => *value, _ => panic!("Arena: expected Boxed at {}", index), } } @@ -1144,6 +1144,44 @@ impl Arena { } } + /// Whether a root or another arena entry has registered a reference to + /// this tuple or boxed wrapper. `false` for anything else. + pub fn wrapper_or_tuple_is_held_elsewhere(&self, value: NanValue) -> bool { + match self.get(value.arena_index()) { + ArenaEntry::Tuple { holder_count, .. } | ArenaEntry::Boxed { holder_count, .. } => { + *holder_count != 0 + } + _ => true, + } + } + + /// Empty a boxed wrapper nothing else holds and hand back its value. The + /// box stops holding the value, so the value loses that registered holder; + /// the caller has established that no root, entry or stack cell can reach + /// the box again. + pub fn take_boxed_value(&mut self, wrapper: NanValue) -> NanValue { + let value = match self.get_mut(wrapper.arena_index()) { + ArenaEntry::Boxed { value, .. } => std::mem::replace(value, NanValue::UNIT), + _ => panic!("Arena: expected Boxed at {}", wrapper.arena_index()), + }; + self.release_held_elsewhere(value); + value + } + + /// Empty a tuple nothing else holds, releasing the registered holder it + /// was of each item. The caller has established that no root, entry or + /// stack cell can reach the tuple again, and has already copied out the + /// items it needs. + pub fn release_tuple_items(&mut self, tuple: NanValue) { + let items = match self.get_mut(tuple.arena_index()) { + ArenaEntry::Tuple { items, .. } => std::mem::take(items), + _ => panic!("Arena: expected Tuple at {}", tuple.arena_index()), + }; + for item in items { + self.release_held_elsewhere(item); + } + } + /// Whether a root or another arena entry has registered a reference to /// this record. Operand-stack aliases are deliberately not represented /// here; the VM can inspect those directly after popping its operand. @@ -1188,7 +1226,7 @@ impl Arena { } pub fn get_tuple(&self, index: u32) -> &[NanValue] { match self.get(index) { - ArenaEntry::Tuple(items) => items, + ArenaEntry::Tuple { items, .. } => items, _ => panic!("Arena: expected Tuple at {}", index), } } diff --git a/aver-memory/src/lib.rs b/aver-memory/src/lib.rs index 43529a924..4ec43c6af 100644 --- a/aver-memory/src/lib.rs +++ b/aver-memory/src/lib.rs @@ -179,8 +179,10 @@ pub struct VectorSlot { pub fn entry_holds_slot(entry: &ArenaEntry, index: u32) -> bool { let holds = |value: &NanValue| value.heap_index() == Some(index); match entry { - ArenaEntry::Boxed(value) => holds(value), - ArenaEntry::Tuple(items) | ArenaEntry::Vector { items, .. } => items.iter().any(holds), + ArenaEntry::Boxed { value, .. } => holds(value), + ArenaEntry::Tuple { items, .. } | ArenaEntry::Vector { items, .. } => { + items.iter().any(holds) + } ArenaEntry::Record { fields, .. } | ArenaEntry::Variant { fields, .. } => { fields.iter().any(holds) } @@ -1088,6 +1090,29 @@ impl NanValue { self.is_nan_boxed() && self.tag() == TAG_TUPLE } + /// An `Option.Some`, `Result.Ok` or `Result.Err` whose value lives in an + /// arena box rather than inline in the wrapper. + #[inline] + pub fn is_boxed_wrapper(self) -> bool { + self.is_nan_boxed() + && matches!(self.tag(), TAG_SOME | TAG_OK | TAG_ERR) + && self.payload() & ARENA_REF_BIT != 0 + } + + /// Whether this value is an arena entry that counts its off-stack holders: + /// a heap map, a heap vector, a record, a tuple, or a boxed wrapper. These + /// are the entries the runtime may empty in place once nothing else holds + /// them, so every entry, root and table that stores one has to register + /// itself as a holder. + #[inline] + pub fn counts_holders(self) -> bool { + self.is_heap_map() + || (self.is_vector() && !self.is_empty_vector_immediate()) + || self.is_record() + || self.is_tuple() + || self.is_boxed_wrapper() + } + #[inline] pub fn is_builtin(self) -> bool { self.is_nan_boxed() && self.tag() == TAG_SYMBOL && self.symbol_kind() == SYMBOL_BUILTIN @@ -1525,9 +1550,11 @@ pub struct Arena { /// job: no vector entry means no value here can carry a vector's index, /// so the per-push marking pass has nothing to find. holds_any_vector: bool, - /// Whether this arena has ever stored a record. Records can be consumed by - /// the VM's last-use field projection, so an aggregate that stores one has - /// to register itself as an off-stack holder just like it does for maps and + /// Whether this arena has ever stored a record, a tuple or a boxed + /// wrapper. Records can be consumed by the VM's last-use field projection, + /// and tuples and boxes give up what they hold when they are destructured + /// with nothing else holding them, so an aggregate that stores one has to + /// register itself as an off-stack holder just like it does for maps and /// vectors. holds_any_record: bool, /// Which out-of-region slots the descent above has already rewritten, one @@ -1577,7 +1604,13 @@ pub enum ArenaEntry { BigInt(Box), String(Rc), List(ArenaList), - Tuple(Vec), + Tuple { + items: Vec, + /// Registered off-stack holders of this tuple. A tuple destructured + /// where nothing else holds it gives its items up; see + /// [`Arena::release_tuple_items`]. + holder_count: u32, + }, /// A map, plus the same claim [`ListBody::all_immediate`] makes about a /// list body: `all_immediate` is `true` only when every key and every value /// in `map` is [`NanValue::is_immediate`], which makes relocating the table @@ -1716,7 +1749,15 @@ pub enum ArenaEntry { name: Rc, members: Vec<(Rc, NanValue)>, }, - Boxed(NanValue), + /// The value inside an `Option.Some`, `Result.Ok` or `Result.Err` that + /// is not stored inline in the wrapper itself. + Boxed { + value: NanValue, + /// Registered off-stack holders of this box. A box unwrapped where + /// nothing else holds it gives its value up; see + /// [`Arena::take_boxed_value`]. + holder_count: u32, + }, } /// A borrowed view of an arena-stored integer, discriminating the @@ -1767,8 +1808,7 @@ impl ListBody { let mut holds_takeable = false; for value in &items { all_immediate &= value.is_immediate(); - holds_takeable |= - (value.is_map() || value.is_vector() || value.is_record()) && !value.is_immediate(); + holds_takeable |= value.counts_holders(); } Self { items, diff --git a/aver-memory/src/memory.rs b/aver-memory/src/memory.rs index 802124d9d..982822d64 100644 --- a/aver-memory/src/memory.rs +++ b/aver-memory/src/memory.rs @@ -371,13 +371,25 @@ impl Arena { ArenaEntry::String(s) => ArenaEntry::String(s), ArenaEntry::Builtin(name) => ArenaEntry::Builtin(name), ArenaEntry::Fn(f) => ArenaEntry::Fn(f), - ArenaEntry::Boxed(inner) => ArenaEntry::Boxed(rewrite(self, inner)), + ArenaEntry::Boxed { + value, + holder_count, + } => ArenaEntry::Boxed { + value: rewrite(self, value), + holder_count, + }, ArenaEntry::List(list) => ArenaEntry::List(self.rewrite_list_with(list, rewrite)), - ArenaEntry::Tuple(mut items) => { + ArenaEntry::Tuple { + mut items, + holder_count, + } => { for value in &mut items { *value = rewrite(self, *value); } - ArenaEntry::Tuple(items) + ArenaEntry::Tuple { + items, + holder_count, + } } ArenaEntry::Vector { mut items, @@ -2281,7 +2293,7 @@ impl Arena { } => { return entry; } - ArenaEntry::Vector { items, .. } | ArenaEntry::Tuple(items) + ArenaEntry::Vector { items, .. } | ArenaEntry::Tuple { items, .. } if !items.is_empty() && !items .iter() diff --git a/src/main/commands.rs b/src/main/commands.rs index 08057806f..a0eef737f 100644 --- a/src/main/commands.rs +++ b/src/main/commands.rs @@ -1579,6 +1579,10 @@ pub(super) fn cmd_run_vm( " refused, something off the stack holds it:{} not examined, walk dearer than the copy:{}", owned.refused_off_stack_holder, owned.unexamined_walk_too_costly ); + eprintln!( + " map entries copied by the writes that were not in place:{}", + machine.arena.map_entries_copied() + ); let fence = &report.vector_ownership; eprintln!("\nVector writes the compiler granted, confirmed at run time:"); eprintln!( diff --git a/src/vm/compiler/mir.rs b/src/vm/compiler/mir.rs index da047ceae..543bd2c2d 100644 --- a/src/vm/compiler/mir.rs +++ b/src/vm/compiler/mir.rs @@ -512,7 +512,15 @@ pub(super) fn compile_mir_expr( // ── Phase 4d: `?` propagation ─────────────────────────── MirExpr::Try(inner) => { compile_mir_expr(fc, inner)?; - fc.emit_op(PROPAGATE_ERR); + // A `Result` nothing reads afterwards (a temporary, or a local at + // its last use) gives up the value it unwraps when nothing else + // holds its box, as a consumed match subject does. + let consumes = !matches!(&inner.node, MirExpr::Local(local) if !local.node.last_use); + fc.emit_op(if consumes { + PROPAGATE_ERR_CONSUMED + } else { + PROPAGATE_ERR + }); Ok(()) } @@ -837,6 +845,13 @@ pub(super) fn compile_mir_expr( return Ok(()); } compile_mir_expr(fc, &m.subject)?; + // A subject nothing reads after the match (a temporary, or a + // local at its last use) is consumed by the arm that matches it: + // a tuple or box the arm takes apart, if nothing else holds it, + // gives up what it holds, so the values the arm binds are not + // still held by it. The runtime decides whether nothing else does. + let consumes = + !matches!(&m.subject.node, MirExpr::Local(local) if !local.node.last_use); let mut end_jumps: Vec = Vec::new(); let last_idx = m.arms.len() - 1; @@ -847,12 +862,16 @@ pub(super) fn compile_mir_expr( // Bindings still need extracting (value on // stack, shape known from preceding arm // failures). - emit_last_arm_bindings(fc, &arm.pattern)?; + emit_last_arm_bindings(fc, &arm.pattern, consumes)?; Vec::new() } else { - emit_pattern_check(fc, &arm.pattern)? + emit_pattern_check(fc, &arm.pattern, consumes)? }; - fc.emit_op(POP); + fc.emit_op(if consumes && matches!(arm.pattern, MirPattern::Tuple(_)) { + POP_CONSUMED + } else { + POP + }); compile_mir_expr(fc, &arm.body)?; if !is_last { end_jumps.push(fc.emit_jump(JUMP)); @@ -1224,9 +1243,14 @@ fn pattern_supported(p: &MirPattern) -> bool { /// list of `fail_offset` patch positions the caller will fill /// in to point at the next arm's start. Empty `Vec` = pattern /// always matches (Wildcard / Bind). +/// +/// `consumes` marks the match's own subject when the match consumes it; see +/// `MATCH_UNWRAP_CONSUMES`. Nested subpatterns never consume: the value they +/// look at is still held by the one around it. fn emit_pattern_check( fc: &mut FnCompiler<'_>, pattern: &MirPattern, + consumes: bool, ) -> Result, MirVmUnsupported> { match pattern { MirPattern::Wildcard => Ok(Vec::new()), @@ -1386,7 +1410,7 @@ fn emit_pattern_check( BuiltinCtor::OptionNone => unreachable!(), }; fc.emit_op(MATCH_UNWRAP); - fc.emit_u8(kind); + fc.emit_u8(unwrap_kind(kind, consumes)); let patch = fc.offset(); fc.emit_i32(0); // MATCH_UNWRAP replaces TOS with the inner @@ -1575,14 +1599,24 @@ fn literal_dispatch_bits(fc: &mut FnCompiler<'_>, lit: &Literal) -> u64 { nv.bits() } +/// `MATCH_UNWRAP`'s kind byte, marked when the match consumes its subject. +fn unwrap_kind(kind: u8, consumes: bool) -> u8 { + if consumes { + kind | MATCH_UNWRAP_CONSUMES + } else { + kind + } +} + /// Last-arm exhaustive binding extraction. The pattern is /// guaranteed to match (preceding arms exhausted everything /// else) so we skip the match-check opcode and just bind /// whatever the pattern names. Mirror of HIR's last-arm logic -/// in `compile_match`. +/// in `compile_match`. `consumes` as for [`emit_pattern_check`]. fn emit_last_arm_bindings( fc: &mut FnCompiler<'_>, pattern: &MirPattern, + consumes: bool, ) -> Result<(), MirVmUnsupported> { match pattern { MirPattern::Wildcard | MirPattern::Literal(_) | MirPattern::EmptyList => Ok(()), @@ -1629,7 +1663,7 @@ fn emit_last_arm_bindings( } }; fc.emit_op(MATCH_UNWRAP); - fc.emit_u8(kind); + fc.emit_u8(unwrap_kind(kind, consumes)); fc.emit_i32(0); // no-fail (shape known) emit_dup_and_bind(fc, *b)?; } @@ -1647,7 +1681,7 @@ fn emit_last_arm_bindings( i, &format!("tuple pattern uses item index {i}"), )?); - emit_last_arm_bindings(fc, sub)?; + emit_last_arm_bindings(fc, sub, false)?; fc.emit_op(POP); } Ok(()) @@ -1683,7 +1717,7 @@ where F: FnOnce(&mut FnCompiler<'_>), { emit_subject(fc); - let inner_fail_patches = emit_pattern_check(fc, pattern)?; + let inner_fail_patches = emit_pattern_check(fc, pattern, false)?; fc.emit_op(POP); if inner_fail_patches.is_empty() { diff --git a/src/vm/execute/dispatch.rs b/src/vm/execute/dispatch.rs index c2e8c1407..54550cfaf 100644 --- a/src/vm/execute/dispatch.rs +++ b/src/vm/execute/dispatch.rs @@ -309,6 +309,13 @@ impl VM { self.stack.pop().ok_or(VmError::StackUnderflow)?; } + POP_CONSUMED => { + let subject = self.stack.pop().ok_or(VmError::StackUnderflow)?; + if subject.is_tuple() { + self.release_consumed_tuple(subject); + } + } + DUP => { let val = *self.stack.last().ok_or(VmError::StackUnderflow)?; self.stack.push(val); @@ -1628,11 +1635,14 @@ impl VM { } } - PROPAGATE_ERR => { + PROPAGATE_ERR | PROPAGATE_ERR_CONSUMED => { let value = *self.stack.last().ok_or(VmError::StackUnderflow)?; if value.is_ok() { let inner = value.wrapper_inner(&self.arena); *self.stack.last_mut().ok_or(VmError::StackUnderflow)? = inner; + if op == PROPAGATE_ERR_CONSUMED && value.is_boxed_wrapper() { + self.release_consumed_box(value, inner); + } continue; } if value.is_err() { @@ -1916,8 +1926,10 @@ impl VM { } MATCH_UNWRAP => { - let kind = read_u8!(code, ip); + let raw_kind = read_u8!(code, ip); let offset = read_jump!(code, ip); + let consumes = raw_kind & MATCH_UNWRAP_CONSUMES != 0; + let kind = raw_kind & !MATCH_UNWRAP_CONSUMES; let top = *self.stack.last().ok_or(VmError::StackUnderflow)?; let matches = match kind { 0 => top.is_ok(), @@ -1928,6 +1940,12 @@ impl VM { if matches { let inner = top.wrapper_inner(&self.arena); *self.stack.last_mut().unwrap() = inner; + // The subject is consumed here: a box nothing else + // holds is gone after this, so it stops holding its + // value and a later write to that value need not copy. + if consumes && top.is_boxed_wrapper() { + self.release_consumed_box(top, inner); + } } else { ip = (ip as isize + offset as isize) as usize; } diff --git a/src/vm/execute/slots.rs b/src/vm/execute/slots.rs index 85876ecc4..c73d50d97 100644 --- a/src/vm/execute/slots.rs +++ b/src/vm/execute/slots.rs @@ -595,6 +595,75 @@ impl VM { true } + /// What giving up `value` can save a later write: the entries of a map, the + /// length of a vector, and those of the maps and vectors directly inside a + /// record, tuple or box, one level further down at most. Zero for anything + /// else. + pub(super) fn release_worth(&self, value: NanValue, depth: u8) -> usize { + if !value.counts_holders() { + return 0; + } + if let Some(map) = self.arena.map_slot(value) { + return map.entries; + } + if let Some(vector) = self.arena.vector_slot(value) { + return vector.len; + } + if depth > 1 { + return 0; + } + let children: &[NanValue] = if value.is_record() { + self.arena.get_record(value.arena_index()).1 + } else if value.is_tuple() { + self.arena.get_tuple(value.arena_index()) + } else if value.is_boxed_wrapper() { + return self.release_worth(value.wrapper_inner(&self.arena), depth + 1); + } else { + return 0; + }; + children + .iter() + .map(|child| self.release_worth(*child, depth + 1)) + .sum() + } + + /// Whether a tuple or boxed wrapper just destructured, and no longer on + /// the operand stack as the match subject, is held by nothing at all, so + /// its parts may be released. Asked only when that can save a copy worth + /// the walk: `worth` is [`VM::release_worth`] of the parts, and the walk + /// is bounded by it the way a map write's is. + pub(super) fn destructured_is_unheld(&self, value: NanValue, worth: usize) -> bool { + worth != 0 + && self.stack.len() <= worth + WALK_SLACK + && !self.arena.wrapper_or_tuple_is_held_elsewhere(value) + && self.slot_is_unheld(value) + } + + /// A boxed wrapper a match or `?` consumed, with `inner` already on the + /// stack in its place: when nothing else holds the box, it stops holding + /// `inner`. Kept out of the dispatch loop, which it would otherwise grow. + #[inline(never)] + pub(super) fn release_consumed_box(&mut self, wrapper: NanValue, inner: NanValue) { + if self.destructured_is_unheld(wrapper, self.release_worth(inner, 0)) { + self.arena.take_boxed_value(wrapper); + } + } + + /// A tuple a match consumed, already popped: when nothing else holds it, + /// it stops holding its items. + #[inline(never)] + pub(super) fn release_consumed_tuple(&mut self, tuple: NanValue) { + let worth = self + .arena + .get_tuple(tuple.arena_index()) + .iter() + .map(|item| self.release_worth(*item, 0)) + .sum(); + if self.destructured_is_unheld(tuple, worth) { + self.arena.release_tuple_items(tuple); + } + } + /// Whether a record update may move the fields of `base`, whose operand /// has just been popped, into the record it builds. /// diff --git a/src/vm/execute/tests.rs b/src/vm/execute/tests.rs index a1211e01f..3a3692795 100644 --- a/src/vm/execute/tests.rs +++ b/src/vm/execute/tests.rs @@ -51,7 +51,7 @@ fn assert_no_young_refs(value: NanValue, arena: &Arena, context: &str) { } } }, - ArenaEntry::Tuple(items) | ArenaEntry::Vector { items, .. } => { + ArenaEntry::Tuple { items, .. } | ArenaEntry::Vector { items, .. } => { for item in items.iter().copied() { assert_no_young_refs(item, arena, context); } @@ -77,7 +77,7 @@ fn assert_no_young_refs(value: NanValue, arena: &Arena, context: &str) { assert_no_young_refs(*member, arena, context); } } - ArenaEntry::Boxed(inner) => assert_no_young_refs(*inner, arena, context), + ArenaEntry::Boxed { value, .. } => assert_no_young_refs(*value, arena, context), ArenaEntry::Int(_) | ArenaEntry::BigInt(_) | ArenaEntry::String(_) diff --git a/src/vm/opcode.rs b/src/vm/opcode.rs index 6a2374836..f76103ff6 100644 --- a/src/vm/opcode.rs +++ b/src/vm/opcode.rs @@ -228,6 +228,21 @@ pub const RECORD_GET_NAMED: u8 = 0x67; // field_symbol_id:u32 /// `RECORD_GET_NAMED`. pub const RECORD_TAKE_NAMED: u8 = 0x6C; // field_symbol_id:u32, holders:u8 +/// Pop a match subject the match consumes: a tuple destructured by the arm +/// that just matched it. When nothing else holds the tuple, its items are +/// released, so the values the arm bound are no longer held by it. Otherwise +/// behaves exactly like `POP`. +pub const POP_CONSUMED: u8 = 0xB1; + +/// `PROPAGATE_ERR` on a `Result` nothing reads afterwards: an `Ok` box nothing +/// else holds gives up the value it unwraps, as `MATCH_UNWRAP` does when its +/// kind carries `MATCH_UNWRAP_CONSUMES`. +pub const PROPAGATE_ERR_CONSUMED: u8 = 0xB2; + +/// Set in `MATCH_UNWRAP`'s kind byte when the match consumes its subject: +/// a box nothing else holds then gives up the value it unwraps. +pub const MATCH_UNWRAP_CONSUMES: u8 = 0x80; + /// Pop `count` field values, push a new variant. pub const VARIANT_NEW: u8 = 0x65; // type_id:u16, variant_id:u16, count:u8 @@ -602,6 +617,7 @@ pub fn opcode_name(op: u8) -> &'static str { LOAD_CONST => "LOAD_CONST", LOAD_GLOBAL => "LOAD_GLOBAL", POP => "POP", + POP_CONSUMED => "POP_CONSUMED", DUP => "DUP", LOAD_UNIT => "LOAD_UNIT", LOAD_TRUE => "LOAD_TRUE", @@ -653,6 +669,7 @@ pub fn opcode_name(op: u8) -> &'static str { TUPLE_NEW => "TUPLE_NEW", RECORD_UPDATE => "RECORD_UPDATE", PROPAGATE_ERR => "PROPAGATE_ERR", + PROPAGATE_ERR_CONSUMED => "PROPAGATE_ERR_CONSUMED", LIST_LEN => "LIST_LEN", LIST_PREPEND => "LIST_PREPEND", MATCH_VARIANT => "MATCH_VARIANT", @@ -748,6 +765,7 @@ pub fn opcode_operand_width(op: u8, code: &[u8], ip: usize) -> usize { | GT_FLOAT | RETURN | PROPAGATE_ERR + | PROPAGATE_ERR_CONSUMED | LIST_HEAD_TAIL | LIST_NIL | LIST_CONS @@ -789,7 +807,8 @@ pub fn opcode_operand_width(op: u8, code: &[u8], ip: usize) -> usize { | VECTOR_NEW_LITERAL | BRANCH_PATH_CHILD_LITERAL | BRANCH_PATH_PARSE_LITERAL - | RESULT_PROVEN => 0, + | RESULT_PROVEN + | POP_CONSUMED => 0, // 1-byte LOAD_LOCAL | MOVE_LOCAL | STORE_LOCAL | CALL_VALUE | EXTRACT_FIELD | EXTRACT_TUPLE_ITEM diff --git a/tests/mir_vm_codegen.rs b/tests/mir_vm_codegen.rs index 597d172c4..8361c535c 100644 --- a/tests/mir_vm_codegen.rs +++ b/tests/mir_vm_codegen.rs @@ -394,9 +394,18 @@ fn nonfinal_record_field_access_keeps_record_get_named() { #[test] fn try_propagation_emits_propagate_err() { + // `?` on a temporary consumes the Result: an `Ok` box nothing else holds + // gives up its value. assert_emits( "fn fetch(x: Int) -> Result\n Result.Ok(x)\n\nfn relay(x: Int) -> Result\n Result.Ok(fetch(x)?)\n", "relay", + opcode::PROPAGATE_ERR_CONSUMED, + "PROPAGATE_ERR_CONSUMED", + ); + // A local read again afterwards is not consumed. + assert_emits( + "fn relay(r: Result) -> Result\n x = r?\n Result.Ok(x + Result.withDefault(r, 0))\n", + "relay", opcode::PROPAGATE_ERR, "PROPAGATE_ERR", ); diff --git a/tests/vm_consumed_destructure.rs b/tests/vm_consumed_destructure.rs new file mode 100644 index 000000000..12d7c4da9 --- /dev/null +++ b/tests/vm_consumed_destructure.rs @@ -0,0 +1,417 @@ +//! A tuple or `Option`/`Result` box that a match (or `?`) takes apart where +//! nothing reads it afterwards gives up what it holds. +//! +//! A record carried in `Option.Some(state)` or in `(state, reply)` used to stay +//! held by that box or tuple after the match had bound it: the arena counted +//! the box as a holder of `state`, and nothing ever took the count back, so +//! every later `Map.set` on a field of `state` copied the whole Map. The +//! generated loop hands an answer module's state out of the run exactly this +//! way, so every answer copied the module's Maps on the VM. +//! +//! Tuples and boxes now count their holders the way maps, vectors and records +//! do, and a match whose subject is a temporary or a local at its last use +//! releases what the tuple or box holds when nothing else holds the tuple or +//! box itself. +//! +//! Two halves: +//! +//! - the WINS: the per-step update copies nothing, measured as map entries +//! copied; +//! - the REFUSALS: every way the tuple or box can still be reached keeps it +//! holding its value, and the program's answer is the one immutable values +//! give. A wrong release would show up as the other holder reading back a +//! changed Map. + +use std::process::Command; + +use aver::ir::pipeline::{PipelineConfig, TypecheckMode}; +use aver::nan_value::Arena; +use aver::vm::{self, VM}; + +fn compiled_vm(src: &str) -> VM { + let mut items = aver::source::parse_source(src).expect("parse failed"); + let result = aver::ir::pipeline::run( + &mut items, + PipelineConfig { + typecheck: Some(TypecheckMode::Full { base_dir: None }), + ..Default::default() + }, + ); + let tc = result.typecheck.as_ref().expect("typecheck requested"); + assert!(tc.errors.is_empty(), "typecheck failed: {:?}", tc.errors); + + let mut arena = Arena::new(); + let (code, globals) = vm::compile_program( + &result.resolved_items, + &result.symbol_table, + &mut arena, + None, + ) + .expect("compile failed"); + VM::new(code, globals, arena) +} + +/// What the program answered and how many map entries it copied. +fn run(src: &str) -> (i64, u64) { + let mut machine = compiled_vm(src); + let result = machine.run().expect("program should run"); + ( + result.as_int(&machine.arena), + machine.arena.map_entries_copied(), + ) +} + +const PRELUDE: &str = r#"module Consumed + intent = "destructured tuples and boxes" + effects [] + +record State + counts: Map + +record Holder + held: Option + +fn fill(counts: Map, left: Int) -> Map + match left <= 0 + true -> counts + false -> fill(Map.set(counts, left, 1), left - 1) + +fn fresh() -> State + State(counts = fill({}, 200)) + +fn get(m: Map, k: Int) -> Int + Option.withDefault(Map.get(m, k), 0 - 1) + +fn countOf(held: Option, k: Int) -> Int + match held + Option.Some(s) -> get(s.counts, k) + Option.None -> 0 - 2 + +fn step(held: Option, key: Int) -> Option + match held + Option.Some(s) -> Option.Some(State(counts = Map.set(s.counts, key, 5))) + Option.None -> Option.None + +fn pair(both: Tuple, key: Int) -> Tuple + match both + (s, n) -> (State(counts = Map.set(s.counts, key, 5)), n + 1) + +fn checked(s: State, key: Int) -> Result + Result.Ok(State(counts = Map.set(s.counts, key, 5))) +"#; + +fn program(body: &str) -> String { + format!("{PRELUDE}\n{body}") +} + +const STEPS: i64 = 50; + +#[test] +fn an_option_unwrapped_at_its_last_use_copies_nothing() { + let src = program(&format!( + r#" +fn serve(held: Option, left: Int) -> Option + match left <= 0 + true -> held + false -> serve(step(held, left), left - 1) + +fn main() -> Int + done = serve(Option.Some(fresh()), {STEPS}) + countOf(done, 7) * 1000 + countOf(done, 199) +"# + )); + let (answer, copied) = run(&src); + assert_eq!(answer, 5 * 1000 + 1); + assert_eq!(copied, 0, "a step copied the Map inside the Option"); +} + +#[test] +fn a_tuple_destructured_at_its_last_use_copies_nothing() { + let src = program(&format!( + r#" +fn serve(both: Tuple, left: Int) -> Tuple + match left <= 0 + true -> both + false -> serve(pair(both, left), left - 1) + +fn main() -> Int + match serve((fresh(), 0), {STEPS}) + (s, n) -> n * 1000 + get(s.counts, 7) +"# + )); + let (answer, copied) = run(&src); + assert_eq!(answer, STEPS * 1000 + 5); + assert_eq!(copied, 0, "a step copied the Map inside the tuple"); +} + +#[test] +fn a_result_unwrapped_by_try_copies_nothing() { + let src = program(&format!( + r#" +fn serve(s: State, left: Int) -> Result + match left <= 0 + true -> Result.Ok(s) + false -> serve(checked(s, left)?, left - 1) + +fn main() -> Int + match serve(fresh(), {STEPS}) + Result.Ok(s) -> get(s.counts, 7) + Result.Err(_) -> 0 - 3 +"# + )); + let (answer, copied) = run(&src); + assert_eq!(answer, 5); + assert_eq!(copied, 0, "a step copied the Map inside the Result"); +} + +/// The shape the generated loop has: a box inside a tuple, taken apart in two +/// matches. +#[test] +fn a_box_inside_a_tuple_taken_apart_in_turn_copies_nothing() { + let src = program(&format!( + r#" +fn stepBoth(both: Tuple, Int>, key: Int) -> Tuple, Int> + match both + (held, n) -> match held + Option.Some(s) -> (Option.Some(State(counts = Map.set(s.counts, key, 5))), n + 1) + Option.None -> (Option.None, n) + +fn serve(both: Tuple, Int>, left: Int) -> Tuple, Int> + match left <= 0 + true -> both + false -> serve(stepBoth(both, left), left - 1) + +fn main() -> Int + match serve((Option.Some(fresh()), 0), {STEPS}) + (held, n) -> n * 1000 + countOf(held, 7) +"# + )); + let (answer, copied) = run(&src); + assert_eq!(answer, STEPS * 1000 + 5); + assert_eq!( + copied, 0, + "a step copied the Map inside the box inside the tuple" + ); +} + +/// The caller keeps the Option it passed. +#[test] +fn an_option_the_caller_still_holds_keeps_its_value() { + let src = program( + r#" +fn main() -> Int + before = Option.Some(fresh()) + after = step(before, 7) + countOf(before, 7) * 100 + countOf(after, 7) +"#, + ); + let (answer, copied) = run(&src); + assert_eq!(answer, 100 + 5); + assert!(copied > 0, "the shared Map was written in place"); +} + +/// A record holds the box. +#[test] +fn an_option_a_record_holds_keeps_its_value() { + let src = program( + r#" +fn main() -> Int + holder = Holder(held = Option.Some(fresh())) + after = step(holder.held, 7) + countOf(holder.held, 7) * 100 + countOf(after, 7) +"#, + ); + let (answer, copied) = run(&src); + assert_eq!(answer, 100 + 5); + assert!(copied > 0, "the shared Map was written in place"); +} + +/// A list holds the box, and the only other reference is the match subject. +#[test] +fn an_option_a_list_holds_keeps_its_value() { + let src = program( + r#" +fn firstStepped(xs: List>) -> Option + match xs + [] -> Option.None + [h, .._] -> step(h, 7) + +fn main() -> Int + xs = [Option.Some(fresh())] + after = firstStepped(xs) + match xs + [] -> 0 - 4 + [h, .._] -> countOf(h, 7) * 100 + countOf(after, 7) +"#, + ); + let (answer, copied) = run(&src); + assert_eq!(answer, 100 + 5); + assert!(copied > 0, "the shared Map was written in place"); +} + +/// One box in two tuples: taking the first apart releases its hold on the +/// box, and the second still holds it. +#[test] +fn a_box_two_tuples_share_keeps_its_value() { + let src = program( + r#" +fn stepFirst(both: Tuple, Int>) -> Option + match both + (held, _) -> step(held, 7) + +fn main() -> Int + shared = Option.Some(fresh()) + second = (shared, 2) + first = (shared, 1) + after = stepFirst(first) + match second + (held, n) -> countOf(held, 7) * 100 + countOf(after, 7) * 10 + n +"#, + ); + let (answer, copied) = run(&src); + assert_eq!(answer, 100 + 5 * 10 + 2); + assert!(copied > 0, "the shared Map was written in place"); +} + +/// A Map holds the box as a value. +#[test] +fn an_option_a_map_holds_keeps_its_value() { + let src = program( + r#" +fn main() -> Int + table = Map.set({}, 1, Option.Some(fresh())) + after = match Map.get(table, 1) + Option.Some(held) -> step(held, 7) + Option.None -> Option.None + match Map.get(table, 1) + Option.Some(held) -> countOf(held, 7) * 100 + countOf(after, 7) + Option.None -> 0 - 4 +"#, + ); + let (answer, copied) = run(&src); + assert_eq!(answer, 100 + 5); + assert!(copied > 0, "the shared Map was written in place"); +} + +/// The caller keeps the tuple it passed. +#[test] +fn a_tuple_the_caller_still_holds_keeps_its_items() { + let src = program( + r#" +fn main() -> Int + before = (fresh(), 0) + after = pair(before, 7) + match before + (s, _) -> match after + (t, n) -> get(s.counts, 7) * 100 + get(t.counts, 7) * 10 + n +"#, + ); + let (answer, copied) = run(&src); + assert_eq!(answer, 100 + 5 * 10 + 1); + assert!(copied > 0, "the shared Map was written in place"); +} + +/// A tuple inside another tuple: the outer one still holds the inner one. +#[test] +fn a_tuple_another_tuple_holds_keeps_its_items() { + let src = program( + r#" +fn main() -> Int + inner = (fresh(), 0) + outer = (inner, 1) + after = pair(inner, 7) + match outer + (kept, _) -> match kept + (s, _) -> match after + (t, _) -> get(s.counts, 7) * 10 + get(t.counts, 7) +"#, + ); + let (answer, copied) = run(&src); + assert_eq!(answer, 10 + 5); + assert!(copied > 0, "the shared Map was written in place"); +} + +/// The Result a `?` unwraps is kept by the caller. +#[test] +fn a_result_the_caller_still_holds_keeps_its_value() { + let src = program( + r#" +fn relay(r: Result) -> Result + s = r? + checked(s, 7) + +fn main() -> Int + before: Result = Result.Ok(fresh()) + after = relay(before) + match before + Result.Ok(s) -> match after + Result.Ok(t) -> get(s.counts, 7) * 10 + get(t.counts, 7) + Result.Err(_) -> 0 - 5 + Result.Err(_) -> 0 - 6 +"#, + ); + let (answer, copied) = run(&src); + assert_eq!(answer, 10 + 5); + assert!(copied > 0, "the shared Map was written in place"); +} + +/// A local read again after the match is not consumed by it. +#[test] +fn an_option_read_after_its_match_keeps_its_value() { + let src = program( + r#" +fn twice(held: Option) -> Int + after = match held + Option.Some(s) -> Option.Some(State(counts = Map.set(s.counts, 7, 5))) + Option.None -> Option.None + countOf(held, 7) * 10 + countOf(after, 7) + +fn main() -> Int + twice(Option.Some(fresh())) +"#, + ); + let (answer, _) = run(&src); + assert_eq!(answer, 10 + 5); +} + +fn repo_root() -> std::path::PathBuf { + std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")) +} + +/// The generated loop hands an answer module's state out of the run in an +/// `Option`, inside a tuple, and takes the answer back in another tuple. Every +/// request updates a thousand-entry Map in that state; only the first may copy +/// it (the state the run starts from is still shared with the seating). +#[test] +fn the_generated_loop_updates_an_answer_modules_state_in_place() { + let dir = repo_root().join("tests/fixtures/run_owned_answer_state"); + let out = Command::new(env!("CARGO_BIN_EXE_aver")) + .current_dir(repo_root()) + .arg("run") + .arg(dir.join("main.av")) + .arg("--module-root") + .arg(&dir) + .arg("--profile") + .output() + .expect("run aver"); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("total 400"), "{stdout}"); + let report = String::from_utf8_lossy(&out.stderr); + let copied: u64 = report + .lines() + .find_map(|line| { + line.trim() + .strip_prefix("map entries copied by the writes that were not in place:") + }) + .and_then(|n| n.trim().parse().ok()) + .unwrap_or_else(|| panic!("no copied-entries line in:\n{report}")); + assert!( + copied < 2 * 1000, + "the answers copied {copied} map entries; each request copied the state's Map" + ); +} From 9369e95a1411b19fd10431601f557d8635c201cb Mon Sep 17 00:00:00 2001 From: jasisz Date: Fri, 25 Sep 2026 09:27:00 +0200 Subject: [PATCH 2/2] Close the release helper the merge left open Co-Authored-By: Claude Opus 5.5 (1M context) --- src/vm/execute/slots.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/vm/execute/slots.rs b/src/vm/execute/slots.rs index a6042d947..6e4ecc693 100644 --- a/src/vm/execute/slots.rs +++ b/src/vm/execute/slots.rs @@ -663,6 +663,8 @@ impl VM { if self.destructured_is_unheld(tuple, worth) { self.arena.release_tuple_items(tuple); } + } + /// Whether exactly `holders` operand-stack cells hold `record`, which has /// just been popped or read out of another record. Zero asks the cheaper /// question [`VM::slot_is_unheld`] answers.