diff --git a/node-graph/libraries/core-types/src/record.rs b/node-graph/libraries/core-types/src/record.rs index 6ae3fc693b..fecf404359 100644 --- a/node-graph/libraries/core-types/src/record.rs +++ b/node-graph/libraries/core-types/src/record.rs @@ -116,18 +116,15 @@ impl Layout { self.fields.iter().find(|field| field.name == name && field.level == level).map(|field| field.offset) } - pub fn is_inline(&self) -> bool { - self.size <= 16 && self.align <= 8 - } - pub fn frame_bytes(&self) -> usize { - if self.is_inline() { 0 } else { self.size.next_multiple_of(8) } + self.size.next_multiple_of(8) } /// Resolves a value of this layout, which must be its wiring-proven one, - /// to its record bytes. + /// to its record bytes. An empty record carries nothing and resolves to the + /// value's own storage; every other record spills and rides the pointer. pub fn rec(&self, value: &RecordValue<'_>) -> Rec { - match self.is_inline() { + match self.size == 0 { true => Rec((&raw const *value).cast()), false => Rec(value.ptr), } @@ -268,16 +265,12 @@ impl Rec { } } -/// An opaque, tagless record value: inline layouts live in the value's own -/// two words, spilled layouts ride the pointer, and only [`Layout::rec`] -/// tells them apart. Two scalar fields are load-bearing: a union or an -/// over-aligned repr demotes the return to memory. Both constructors -/// initialize all 16 bytes, and a raw pointer field accepts any bit -/// pattern, so inline bytes overlay the fields soundly. +/// An opaque record value: every non-empty record spills to the record stack +/// and the value carries its pointer, while an empty record carries nothing. +/// Only [`Layout::rec`] reads it, against the wiring-proven layout. #[derive(Clone, Copy)] pub struct RecordValue<'e> { ptr: *const u8, - _extra: usize, _lifetime: std::marker::PhantomData<&'e ()>, } @@ -292,7 +285,6 @@ impl<'e> RecordValue<'e> { pub fn zeroed() -> Self { RecordValue { ptr: std::ptr::null(), - _extra: 0, _lifetime: std::marker::PhantomData, } } @@ -307,7 +299,6 @@ impl<'e> RecordValue<'e> { pub fn spilled(rec: Rec) -> Self { RecordValue { ptr: rec.ptr(), - _extra: 0, _lifetime: std::marker::PhantomData, } } @@ -317,7 +308,6 @@ impl<'e> RecordValue<'e> { fn rebind<'a>(self) -> RecordValue<'a> { RecordValue { ptr: self.ptr, - _extra: self._extra, _lifetime: std::marker::PhantomData, } } @@ -354,7 +344,7 @@ impl<'e, C, N: Node>> RecordEdge<'e, C> for N {} /// a parked element reports as an error poll. pub fn lift_poll<'e, T: Send + Sync + 'static>(poll: GPoll, layout: &Layout, arena: &'e crate::arena::Arena) -> GPoll> { let build = |element: T| { - if layout.is_inline() { + if layout.frame_bytes() == 0 { let mut value = RecordValue::zeroed(); unsafe { write_element(value.as_mut_ptr(), element, arena)? }; Some(value) @@ -447,7 +437,14 @@ impl<'a, Out, N> ElementEdge<'a, Out, N> { where N: Node>, { - self.node.eval(ctx).map(|value| unsafe { (self.read)(self.layout.rec(&value), self.reads) }) + let mark = stack::sp(); + self.node.eval(ctx).map(|value| { + let out = unsafe { (self.read)(self.layout.rec(&value), self.reads) }; + // SAFETY: the read copied out by value, so no record above `mark` (the + // edge's own frame) is live. + unsafe { stack::rewind(mark) }; + out + }) } } @@ -502,8 +499,13 @@ impl<'a, Out, N> ElementLazyInput<'a, Out, N> { where N: Node>, { + let mark = stack::sp(); let value = self.cell.eval_input(self.input_index, self.node, ctx)?; - Ok(unsafe { (self.read)(self.layout.rec(&value), self.reads) }) + let out = unsafe { (self.read)(self.layout.rec(&value), self.reads) }; + // SAFETY: the read copied the element and declared attributes out by value, + // so no record above `mark` (the edge's own frame) is live. + unsafe { stack::rewind(mark) }; + Ok(out) } } @@ -934,7 +936,7 @@ pub unsafe fn copy_record_bytes(layout: &Layout, rec: Rec) -> Box<[u8]> { /// `bytes` must hold a record of `layout` whose parked references are still /// live; both hold for a copy taken in the same evaluation frame. pub unsafe fn record_from_bytes<'e>(layout: &Layout, bytes: &'e [u8]) -> RecordValue<'e> { - if layout.is_inline() { + if layout.frame_bytes() == 0 { let mut value = RecordValue::zeroed(); unsafe { std::ptr::copy_nonoverlapping(bytes.as_ptr(), value.as_mut_ptr(), bytes.len()) }; value @@ -1124,7 +1126,7 @@ where fn eval(&self, input: &C) -> GPoll> { match &self.plan { None => self.edge.eval(input), - Some(plan) if plan.union.is_inline() => self.edge.eval(input).map(|value| { + Some(plan) if plan.union.frame_bytes() == 0 => self.edge.eval(input).map(|value| { let mut out = RecordValue::zeroed(); unsafe { plan.translate(plan.source.rec(&value), out.as_mut_ptr()) }; out @@ -1261,27 +1263,21 @@ mod tests { } #[test] - fn record_values_are_two_words() { - assert_eq!(size_of::(), 16); + fn record_values_are_one_word() { + assert_eq!(size_of::(), 8); assert_eq!(align_of::(), 8); } #[test] - fn layouts_resolve_inline_and_spilled_values() { - let inline = Layout::default().with_writes(0, element_write::(), &[f64_field("opacity")]); - assert!(inline.is_inline()); - assert_eq!(inline.frame_bytes(), 0); - let mut value = RecordValue::zeroed(); - unsafe { - write_field(value.as_mut_ptr(), 0, 4f64); - write_field(value.as_mut_ptr(), inline.offset_of("opacity", 0).unwrap(), 0.5f64); - } - let rec = inline.rec(&value); - assert_eq!(unsafe { rec.element::() }, 4.); - assert_eq!(unsafe { rec.read::(inline.offset_of("opacity", 0).unwrap()) }, 0.5); + fn layouts_resolve_spilled_values() { + let small = Layout::default().with_writes(0, element_write::(), &[f64_field("opacity")]); + assert_eq!(small.frame_bytes(), 16); + let backing = [4f64, 0.5]; + let value = RecordValue::spilled(unsafe { Rec::new(backing.as_ptr().cast()) }); + assert_eq!(unsafe { small.rec(&value).element::() }, 4.); + assert_eq!(unsafe { small.rec(&value).read::(small.offset_of("opacity", 0).unwrap()) }, 0.5); let spilled = Layout::default().with_writes(0, element_write::(), &[f64_field("opacity"), f64_field("length")]); - assert!(!spilled.is_inline()); assert_eq!(spilled.frame_bytes(), 24); let record = [1f64, 2., 3.]; let value = RecordValue::spilled(unsafe { Rec::new(record.as_ptr().cast()) }); diff --git a/node-graph/libraries/core-types/src/runtime.rs b/node-graph/libraries/core-types/src/runtime.rs index 9b75b568eb..6390fb1afd 100644 --- a/node-graph/libraries/core-types/src/runtime.rs +++ b/node-graph/libraries/core-types/src/runtime.rs @@ -171,7 +171,7 @@ mod tests { use crate::context::{ContextImpl, Ctx, CtxSnapshot, EvalScope, ExtractFootprint, ExtractVarArgs, VarArgLink, VarArgSlots}; use crate::gpoll::GPoll; use crate::node::Node; - use crate::record::{Layout, RecordExtract, RecordLift, element_write}; + use crate::record::{Layout, RecordExtract, RecordLift, element_write, stack}; use crate::transform::Footprint; use std::sync::Mutex; use std::sync::atomic::{AtomicU32, Ordering}; @@ -267,6 +267,7 @@ mod tests { } fn extract(graph: N) -> RecordExtract { + stack::reserve(1 << 12); RecordExtract::new(graph, &element_layout::()) } diff --git a/node-graph/node-macro/src/codegen.rs b/node-graph/node-macro/src/codegen.rs index 51e8793829..2561957208 100644 --- a/node-graph/node-macro/src/codegen.rs +++ b/node-graph/node-macro/src/codegen.rs @@ -1197,8 +1197,10 @@ pub(crate) fn generate_node_impl(crate_ident: &CrateIdent, parsed: &ParsedNodeFn ParsedFieldType::Regular(RegularParsedField { ty, .. }) if record.is_some() && !field.attribute_reads.is_empty() => { let slot = format_ident!("__in_{index}"); let rec_local = format_ident!("__rec_{index}"); + let mark = format_ident!("__mark_{index}"); let bindings: Vec = reads_of(index).into_iter().map(|(slot, read)| read_binding(slot, read, quote!(#rec_local))).collect(); quote! { + let #mark = #core_types::record::stack::sp(); let #name = match __cell.eval_input(#index, &self.#name, __input) { Ok(value) => value, Err(interrupt) => return interrupt.into(), @@ -1206,6 +1208,8 @@ pub(crate) fn generate_node_impl(crate_ident: &CrateIdent, parsed: &ParsedNodeFn let #rec_local = self.#slot.rec(&#name); #(#bindings)* let #name: #ty = unsafe { #core_types::record::read_element(#rec_local) }; + // SAFETY: the element and declared attribute reads copied out by value, so no record above the mark is live. + unsafe { #core_types::record::stack::rewind(#mark) }; } } // The lend input's frame survives on the record stack until this @@ -1223,12 +1227,16 @@ pub(crate) fn generate_node_impl(crate_ident: &CrateIdent, parsed: &ParsedNodeFn } ParsedFieldType::Regular(RegularParsedField { ty, .. }) if flip => { let slot = format_ident!("__in_{index}"); + let mark = format_ident!("__mark_{index}"); quote! { + let #mark = #core_types::record::stack::sp(); let #name = match __cell.eval_input(#index, &self.#name, __input) { Ok(value) => value, Err(interrupt) => return interrupt.into(), }; let #name: #ty = unsafe { #core_types::record::read_element(self.#slot.rec(&#name)) }; + // SAFETY: the element copied out by value, so no record above the mark is live. + unsafe { #core_types::record::stack::rewind(#mark) }; } } // A routing node's value input rides a record edge; the element @@ -1236,12 +1244,16 @@ pub(crate) fn generate_node_impl(crate_ident: &CrateIdent, parsed: &ParsedNodeFn // can reuse the record stack. ParsedFieldType::Regular(RegularParsedField { ty, .. }) if routing.is_some() && !routing_source(ty) => { let slot = format_ident!("__in_{index}"); + let mark = format_ident!("__mark_{index}"); quote! { + let #mark = #core_types::record::stack::sp(); let #name = match __cell.eval_input(#index, &self.#name, __input) { Ok(value) => value, Err(interrupt) => return interrupt.into(), }; let #name: #ty = unsafe { #core_types::record::read_element(self.#slot.rec(&#name)) }; + // SAFETY: the element copied out by value, so no record above the mark is live. + unsafe { #core_types::record::stack::rewind(#mark) }; } } ParsedFieldType::Regular(_) => quote! { diff --git a/node-graph/nodes/gcore/src/memo.rs b/node-graph/nodes/gcore/src/memo.rs index 590a1fd992..ce2ef37b08 100644 --- a/node-graph/nodes/gcore/src/memo.rs +++ b/node-graph/nodes/gcore/src/memo.rs @@ -164,6 +164,7 @@ mod tests { } fn scope_fixture<'a>(generations: &'a [(SourceId, u64)], arena: &'a Arena) -> EvalScope<'a> { + core_types::record::stack::reserve(1 << 16); EvalScope::new(Some(0.5), None, None, generations, arena) } diff --git a/node-graph/nodes/gcore/src/record.rs b/node-graph/nodes/gcore/src/record.rs index dc7912ab31..0f27b09644 100644 --- a/node-graph/nodes/gcore/src/record.rs +++ b/node-graph/nodes/gcore/src/record.rs @@ -183,6 +183,7 @@ mod tests { } fn scope_fixture<'a>(generations: &'a [(SourceId, u64)], arena: &'a Arena) -> EvalScope<'a> { + stack::reserve(1 << 16); EvalScope::new(Some(0.5), None, None, generations, arena) } @@ -202,7 +203,7 @@ mod tests { } fn reserve_for(layouts: &[&Layout]) { - stack::reserve(layouts.iter().map(|layout| layout.frame_bytes()).sum()); + stack::reserve(layouts.iter().map(|layout| layout.frame_bytes()).sum::().max(1 << 12)); } fn lifted_value(value: T) -> (core_types::record::RecordLift>, Layout) { @@ -544,7 +545,7 @@ mod tests { let carrier_layout = f64_layout(&["opacity"]); let by_layout = f64_layout(&["opacity", "length"]); - assert!(!by_layout.is_inline(), "the borrow must point into a spilled frame to exercise the park"); + assert!(by_layout.frame_bytes() != 0, "the borrow must point into a spilled frame to exercise the park"); reserve_for(&[&carrier_layout, &by_layout]); let node = OffsetNode::new( @@ -858,13 +859,20 @@ mod tests { type Output = RecordValue<'e>; fn eval(&self, input: &ContextImpl<'e>) -> GPoll> { - assert!(self.layout.is_inline()); - let mut value = RecordValue::zeroed(); let element: f64 = match core_types::context::ExtractRealTime::try_real_time(input) { Some(_) => 1., None => 0., }; - unsafe { value.as_mut_ptr().cast::().write(element) }; + let mut value = RecordValue::zeroed(); + let dst = match self.layout.frame_bytes() { + 0 => value.as_mut_ptr(), + bytes => stack::push(bytes), + }; + unsafe { dst.cast::().write(element) }; + if self.layout.frame_bytes() != 0 { + stack::pop(dst); + value = RecordValue::spilled(unsafe { Rec::new(dst.cast_const()) }); + } GPoll::Final(value) } } @@ -945,36 +953,6 @@ mod tests { assert_eq!(text, "parked"); } - #[test] - fn inline_records_survive_sibling_evaluations_by_value() { - let arena = Arena::new(1024).unwrap(); - let generations = []; - let scope = scope_fixture(&generations, &arena); - let ctx = ContextImpl::root(&scope); - - let layout_a = f64_layout(&["opacity"]); - let layout_b = f64_layout(&[]); - let union = Layout::union(&[&layout_a, &layout_b]); - assert!(union.is_inline()); - reserve_for(&[&layout_a, &layout_b, &union, &union]); - - let (condition, condition_layout) = lifted_value(false); - let chain = HoldFirstNode::new( - condition, - RecordSource::new(f64_record_source(&layout_a, 1., vec![(layout_a.offset_of("opacity", 0).unwrap(), 0.5)]), &layout_a, &union), - RecordSource::new(f64_record_source(&layout_b, 3., vec![]), &layout_b, &union), - &union, - &condition_layout, - ); - - let GPoll::Final(value) = chain.eval(&ctx) else { - panic!("expected a final record"); - }; - let rec = union.rec(&value); - assert_eq!(unsafe { rec.element::() }, 1.); - assert_eq!(unsafe { rec.read::(union.offset_of("opacity", 0).unwrap()) }, 0.5); - } - #[test] fn identity_layouts_forward_the_record_pointer() { let arena = Arena::new(1024).unwrap();