From 9680d34abfa102f68f30897d9cbeb58f0b960e6f Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Wed, 5 Aug 2026 22:17:50 +0000 Subject: [PATCH] Carry record elements by the drop-glue rule and unify the lift and extract adapters --- .../interpreted-executor/src/node_registry.rs | 126 +++++------- node-graph/libraries/core-types/src/record.rs | 190 +++++++++--------- node-graph/nodes/gcore/src/record.rs | 4 +- 3 files changed, 144 insertions(+), 176 deletions(-) diff --git a/node-graph/interpreted-executor/src/node_registry.rs b/node-graph/interpreted-executor/src/node_registry.rs index 402c82b6e6..e7523fc6ea 100644 --- a/node-graph/interpreted-executor/src/node_registry.rs +++ b/node-graph/interpreted-executor/src/node_registry.rs @@ -385,62 +385,62 @@ fn node_registry() -> HashMap> { record_extract_node!(graphene_std::vector::style::GradientType), record_lift_node!(graphene_std::vector::style::GradientSpreadMethod), record_extract_node!(graphene_std::vector::style::GradientSpreadMethod), - record_lift_node!(ref String), - record_extract_node!(clone String), - record_lift_node!(ref List), - record_extract_node!(clone List), - record_lift_node!(ref List), - record_extract_node!(clone List), - record_lift_node!(ref List), - record_extract_node!(clone List), - record_lift_node!(ref List), - record_extract_node!(clone List), - record_lift_node!(ref List), - record_extract_node!(clone List), - record_lift_node!(ref List), - record_extract_node!(clone List), - record_lift_node!(ref List>), - record_extract_node!(clone List>), + record_lift_node!(String), + record_extract_node!(String), + record_lift_node!(List), + record_extract_node!(List), + record_lift_node!(List), + record_extract_node!(List), + record_lift_node!(List), + record_extract_node!(List), + record_lift_node!(List), + record_extract_node!(List), + record_lift_node!(List), + record_extract_node!(List), + record_lift_node!(List), + record_extract_node!(List), + record_lift_node!(List>), + record_extract_node!(List>), #[cfg(feature = "gpu")] - record_lift_node!(ref List>), + record_lift_node!(List>), #[cfg(feature = "gpu")] - record_extract_node!(clone List>), - record_lift_node!(ref List), - record_extract_node!(clone List), - record_lift_node!(ref List), - record_extract_node!(clone List), - record_lift_node!(ref List), - record_extract_node!(clone List), - record_lift_node!(ref AttributeDyn), - record_extract_node!(clone AttributeDyn), - record_lift_node!(ref AttributeValueDyn), - record_extract_node!(clone AttributeValueDyn), - record_lift_node!(ref ListDyn), - record_extract_node!(clone ListDyn), - record_lift_node!(ref std::sync::Arc), - record_extract_node!(clone std::sync::Arc), - record_lift_node!(ref RuntimeHandle), - record_extract_node!(clone RuntimeHandle), - record_lift_node!(ref RenderIntermediate), - record_extract_node!(clone RenderIntermediate), - record_lift_node!(ref RenderOutput), - record_extract_node!(clone RenderOutput), + record_extract_node!(List>), + record_lift_node!(List), + record_extract_node!(List), + record_lift_node!(List), + record_extract_node!(List), + record_lift_node!(List), + record_extract_node!(List), + record_lift_node!(AttributeDyn), + record_extract_node!(AttributeDyn), + record_lift_node!(AttributeValueDyn), + record_extract_node!(AttributeValueDyn), + record_lift_node!(ListDyn), + record_extract_node!(ListDyn), + record_lift_node!(std::sync::Arc), + record_extract_node!(std::sync::Arc), + record_lift_node!(RuntimeHandle), + record_extract_node!(RuntimeHandle), + record_lift_node!(RenderIntermediate), + record_extract_node!(RenderIntermediate), + record_lift_node!(RenderOutput), + record_extract_node!(RenderOutput), #[cfg(target_family = "wasm")] - record_lift_node!(ref CanvasHandle), + record_lift_node!(CanvasHandle), #[cfg(target_family = "wasm")] - record_extract_node!(clone CanvasHandle), + record_extract_node!(CanvasHandle), #[cfg(feature = "gpu")] - record_lift_node!(ref WgpuExecutorHandle), + record_lift_node!(WgpuExecutorHandle), #[cfg(feature = "gpu")] - record_extract_node!(clone WgpuExecutorHandle), + record_extract_node!(WgpuExecutorHandle), #[cfg(feature = "gpu")] - record_lift_node!(ref Option), + record_lift_node!(Option), #[cfg(feature = "gpu")] - record_extract_node!(clone Option), + record_extract_node!(Option), #[cfg(feature = "gpu")] - record_lift_node!(ref wgpu_executor::WgpuPipelineCache), + record_lift_node!(wgpu_executor::WgpuPipelineCache), #[cfg(feature = "gpu")] - record_extract_node!(clone wgpu_executor::WgpuPipelineCache), + record_extract_node!(wgpu_executor::WgpuPipelineCache), ( ProtoNodeIdentifier::new("graphene_core::memo::MonitorNode"), RegistryEntry { @@ -877,22 +877,6 @@ mod node_registry_macros { }, ) }; - (ref $type:ty) => { - ( - ProtoNodeIdentifier::new("core_types::record::RecordLiftNode"), - RegistryEntry { - io: NodeIOTypes::new(concrete!(Context), core_types::registry::record_type::<$type>(), vec![fn_type!(Context, $type)]), - constructor: |inputs| { - if inputs.len() != 1 { - return Err(ConstructionError::Arity { expected: 1, got: inputs.len() }); - } - let mut inputs = inputs.into_iter(); - let node = core_types::record::RecordLiftRef::<$type, _>::new(inputs.next().unwrap().downcast::<$type>()?); - Ok(EdgeHandle::new_record::<$type>(std::sync::Arc::new(node) as std::sync::Arc)) - }, - }, - ) - }; } macro_rules! record_extract_node { @@ -914,24 +898,6 @@ mod node_registry_macros { }, ) }; - (clone $type:ty) => { - ( - ProtoNodeIdentifier::new("core_types::record::RecordExtractNode"), - RegistryEntry { - io: NodeIOTypes::new(concrete!(Context), concrete!($type), vec![core_types::registry::record_edge_type::<$type>()]), - constructor: |inputs| { - if inputs.len() != 1 { - return Err(ConstructionError::Arity { expected: 1, got: inputs.len() }); - } - let mut inputs = inputs.into_iter(); - let edge = inputs.next().unwrap(); - let layout = edge.layout().ok_or(ConstructionError::MissingLayout)?.clone(); - let node = core_types::record::RecordExtractClone::<$type, _>::new(edge.downcast_record::<$type>()?, &layout); - Ok(EdgeHandle::new(std::sync::Arc::new(node) as std::sync::Arc>)) - }, - }, - ) - }; } macro_rules! clone_node { diff --git a/node-graph/libraries/core-types/src/record.rs b/node-graph/libraries/core-types/src/record.rs index 7a4bdaac8b..4118d43315 100644 --- a/node-graph/libraries/core-types/src/record.rs +++ b/node-graph/libraries/core-types/src/record.rs @@ -422,6 +422,54 @@ pub unsafe fn write_field(dst: *mut u8, offset: usize, value: T) { unsafe { dst.add(offset).cast::().write(value) } } +/// Whether elements of `T` move once into the arena and ride as references: +/// records byte-copy their contents and never run drop glue, so a type is +/// byte-carried exactly when it has none. +pub const fn element_parked() -> bool { + std::mem::needs_drop::() +} + +/// The element (size, align) a record wire of `T` carries. +pub fn element_dims() -> (usize, usize) { + match element_parked::() { + true => (size_of::<*const u8>(), align_of::<*const u8>()), + false => (size_of::(), align_of::()), + } +} + +/// # Safety +/// The record's element must be a `T` in the form [`element_parked`] picks, +/// and the borrow is only valid while the record is. +pub unsafe fn borrow_element<'e, T>(rec: Rec) -> &'e T { + match element_parked::() { + true => unsafe { rec.element::<&T>() }, + false => unsafe { &*rec.ptr().cast::() }, + } +} + +/// # Safety +/// The record's element must be a `T` in the form [`element_parked`] picks. +pub unsafe fn read_element(rec: Rec) -> T { + unsafe { borrow_element::(rec) }.clone() +} + +/// # Safety +/// `dst` must be fresh element storage of a record whose element is `T`. +/// `None` reports arena exhaustion for a parked element. +pub unsafe fn write_element(dst: *mut u8, value: T, arena: &crate::arena::Arena) -> Option<()> { + match element_parked::() { + true => { + let (parked, _) = arena.alloc(value)?; + unsafe { dst.cast::<&T>().write(parked) }; + Some(()) + } + false => { + unsafe { dst.cast::().write(value) }; + Some(()) + } + } +} + /// # Safety /// `src` must be a record of the plan's source layout and `dst` a buffer of /// the plan's target layout; both are proven at wiring. @@ -584,74 +632,24 @@ where } /// Lifts a plain producer onto a record wire: the element lands at offset 0 -/// of a fresh element-only record. `Copy` elements only until droppable -/// elements ride records. +/// of a fresh element-only record, parked when it carries drop glue. pub struct RecordLift { edge: N, layout: Layout, _marker: std::marker::PhantomData El>, } -impl RecordLift { +impl RecordLift { pub fn new(edge: N) -> Self { Self { edge, - layout: Layout::default().with_writes(0, (size_of::(), align_of::()), &[]), + layout: Layout::default().with_writes(0, element_dims::(), &[]), _marker: std::marker::PhantomData, } } } impl<'e, C, El, N> Node for RecordLift -where - C: crate::context::ExtractArena, - El: Copy + 'static, - N: Node, -{ - type Output = RecordValue<'e>; - - fn eval(&self, input: &C) -> GPoll> { - if self.layout.is_inline() { - return self.edge.eval(input).map(|element| { - let mut value = RecordValue::zeroed(); - unsafe { write_field(value.as_mut_ptr(), 0, element) }; - value - }); - } - let dst = stack::push(self.layout.frame_bytes()); - let value = self.edge.eval(input).map(|element| { - unsafe { write_field(dst, 0, element) }; - RecordValue::spilled(unsafe { Rec::new(dst.cast_const()) }) - }); - stack::pop(dst); - value - } - - fn layout(&self) -> Option<&Layout> { - Some(&self.layout) - } -} - -/// Lifts a droppable producer onto a record wire: the owned element parks in -/// the arena and the record's element is the parked reference, the same rule -/// reference-valued attributes follow. -pub struct RecordLiftRef { - edge: N, - layout: Layout, - _marker: std::marker::PhantomData El>, -} - -impl RecordLiftRef { - pub fn new(edge: N) -> Self { - Self { - edge, - layout: Layout::default().with_writes(0, (size_of::<&El>(), align_of::<&El>()), &[]), - _marker: std::marker::PhantomData, - } - } -} - -impl<'e, C, El, N> Node for RecordLiftRef where C: crate::context::ExtractArena, El: Send + Sync + 'static, @@ -660,11 +658,17 @@ where type Output = RecordValue<'e>; fn eval(&self, input: &C) -> GPoll> { - let park = |element: El| { - let (parked, _) = input.arena().alloc(element)?; - let mut value = RecordValue::zeroed(); - unsafe { write_field::<&El>(value.as_mut_ptr(), 0, parked) }; - Some(value) + let build = |element: El| { + if self.layout.is_inline() { + let mut value = RecordValue::zeroed(); + unsafe { write_element(value.as_mut_ptr(), element, input.arena())? }; + Some(value) + } else { + let dst = stack::push(self.layout.frame_bytes()); + let written = unsafe { write_element(dst, element, input.arena()) }; + stack::pop(dst); + written.map(|()| RecordValue::spilled(unsafe { Rec::new(dst.cast_const()) })) + } }; let exhausted = || { GPoll::Error(Box::new(crate::gpoll::GraphError { @@ -673,11 +677,11 @@ where })) }; match self.edge.eval(input) { - GPoll::Final(element) => park(element).map_or_else(exhausted, GPoll::Final), - GPoll::Partial(element) => park(element).map_or_else(exhausted, GPoll::Partial), + GPoll::Final(element) => build(element).map_or_else(exhausted, GPoll::Final), + GPoll::Partial(element) => build(element).map_or_else(exhausted, GPoll::Partial), GPoll::Fallback(boxed) => { let (element, error) = *boxed; - park(element).map_or_else(exhausted, |value| GPoll::Fallback(Box::new((value, error)))) + build(element).map_or_else(exhausted, |value| GPoll::Fallback(Box::new((value, error)))) } GPoll::Pending => GPoll::Pending, GPoll::Error(error) => GPoll::Error(error), @@ -689,37 +693,8 @@ where } } -/// Extracts a parked-reference element from a record wire by cloning it out -/// for a plain consumer. -pub struct RecordExtractClone { - edge: N, - layout: Layout, - _marker: std::marker::PhantomData El>, -} - -impl RecordExtractClone { - pub fn new(edge: N, layout: &Layout) -> Self { - Self { - edge, - layout: layout.clone(), - _marker: std::marker::PhantomData, - } - } -} - -impl<'e, C, El, N> Node for RecordExtractClone -where - El: Clone + 'static, - N: Node>, -{ - type Output = El; - - fn eval(&self, input: &C) -> GPoll { - self.edge.eval(input).map(|value| unsafe { self.layout.rec(&value).element::<&El>() }.clone()) - } -} - -/// Extracts the element from a record wire for a plain consumer. +/// Extracts the element from a record wire for a plain consumer, cloning out +/// of the parked reference when the element carries drop glue. pub struct RecordExtract { edge: N, layout: Layout, @@ -738,13 +713,13 @@ impl RecordExtract { impl<'e, C, El, N> Node for RecordExtract where - El: Copy + 'static, + El: Clone + 'static, N: Node>, { type Output = El; fn eval(&self, input: &C) -> GPoll { - self.edge.eval(input).map(|value| unsafe { self.layout.rec(&value).element::() }) + self.edge.eval(input).map(|value| unsafe { read_element::(self.layout.rec(&value)) }) } } @@ -832,6 +807,33 @@ mod tests { assert!(SourcePlan::new(&layout, &layout.clone()).is_none()); } + #[test] + fn elements_ride_as_bytes_exactly_without_drop_glue() { + assert!(!element_parked::()); + assert!(!element_parked::<[f64; 4]>()); + assert!(element_parked::()); + assert!(element_parked::>()); + assert_eq!(element_dims::<[f64; 4]>(), (32, 8)); + assert_eq!(element_dims::(), (8, 8)); + } + + #[test] + fn elements_write_and_read_in_their_picked_form() { + let arena = crate::arena::Arena::new(256).unwrap(); + + let mut inline = [0u64; 2]; + unsafe { write_element(inline.as_mut_ptr().cast(), 4.5f64, &arena) }.unwrap(); + let rec = unsafe { Rec::new(inline.as_ptr().cast()) }; + assert_eq!(unsafe { *borrow_element::(rec) }, 4.5); + assert_eq!(unsafe { read_element::(rec) }, 4.5); + + let mut parked = [0u64; 2]; + unsafe { write_element(parked.as_mut_ptr().cast(), String::from("moved once"), &arena) }.unwrap(); + let rec = unsafe { Rec::new(parked.as_ptr().cast()) }; + assert_eq!(unsafe { borrow_element::(rec) }.as_str(), "moved once"); + assert_eq!(unsafe { read_element::(rec) }, "moved once"); + } + #[test] fn record_values_are_two_words() { assert_eq!(size_of::(), 16); diff --git a/node-graph/nodes/gcore/src/record.rs b/node-graph/nodes/gcore/src/record.rs index a9606c5a8d..3fdbd9cc55 100644 --- a/node-graph/nodes/gcore/src/record.rs +++ b/node-graph/nodes/gcore/src/record.rs @@ -636,9 +636,9 @@ mod tests { let scope = scope_fixture(&generations, &arena); let ctx = ContextImpl::root(&scope); - let lift = core_types::record::RecordLiftRef::::new(ValueNode(String::from("parked"))); + let lift = core_types::record::RecordLift::::new(ValueNode(String::from("parked"))); let layout = Node::::layout(&lift).unwrap().clone(); - let chain = core_types::record::RecordExtractClone::::new(lift, &layout); + let chain = core_types::record::RecordExtract::::new(lift, &layout); let GPoll::Final(text) = chain.eval(&ctx) else { panic!("expected a final value");