From 147741ac715530981bd82da51d312a85872305c1 Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Sun, 3 Nov 2024 20:57:38 +0100 Subject: [PATCH] rune: Performance improvements --- crates/rune/src/cli/benches.rs | 63 +++++++---- crates/rune/src/cli/out.rs | 6 +- crates/rune/src/compile/meta.rs | 8 +- crates/rune/src/compile/unit_builder.rs | 10 +- crates/rune/src/module/module_meta.rs | 17 ++- crates/rune/src/modules/mem.rs | 72 +++++++++++++ crates/rune/src/runtime/access.rs | 5 + crates/rune/src/runtime/any_obj.rs | 2 + crates/rune/src/runtime/borrow_mut.rs | 4 + crates/rune/src/runtime/mod.rs | 14 +++ crates/rune/src/runtime/object.rs | 16 ++- crates/rune/src/runtime/static_string.rs | 11 +- crates/rune/src/runtime/value.rs | 128 ++++++++++++----------- crates/rune/src/runtime/value/dynamic.rs | 57 +++++++--- crates/rune/src/runtime/value/rtti.rs | 4 +- crates/rune/src/runtime/vm.rs | 121 +++++++++++---------- 16 files changed, 348 insertions(+), 190 deletions(-) diff --git a/crates/rune/src/cli/benches.rs b/crates/rune/src/cli/benches.rs index 131ba4556..7ff0ea206 100644 --- a/crates/rune/src/cli/benches.rs +++ b/crates/rune/src/cli/benches.rs @@ -25,11 +25,11 @@ mod cli { #[command(rename_all = "kebab-case")] pub(crate) struct Flags { /// Rounds of warmup to perform - #[arg(long, default_value = "10")] - pub(super) warmup: u32, + #[arg(long, default_value = "5.0")] + pub(super) warmup: f32, /// Iterations to run of the benchmark - #[arg(long, default_value = "100")] - pub(super) iter: u32, + #[arg(long, default_value = "10.0")] + pub(super) iter: f32, /// Explicit paths to benchmark. pub(super) bench_path: Vec, } @@ -152,35 +152,46 @@ where } fn bench_fn(io: &mut Io<'_>, item: &dyn fmt::Display, args: &Flags, f: &Function) -> Result<()> { - let mut section = io.section("Warming up", Stream::Stdout, Color::Ignore)?; - section.append(format_args!(" {item} ({} iters): ", args.warmup))?; + let mut section = io.section("Warming up", Stream::Stdout, Color::Progress)?; + section.append(format_args!(" {item} for {:.2}s:", args.warmup))?; + section.flush()?; - let step = (args.warmup / 10).max(1); - - for n in 0..args.warmup { - if n % step == 0 { - section.append(".")?; - section.flush()?; - } + let start = Instant::now(); + let mut warmup = 0; + let elapsed = loop { let value = f.call::(()).into_result()?; drop(hint::black_box(value)); - } + warmup += 1; - section.close()?; + let elapsed = start.elapsed().as_secs_f32(); + + if elapsed >= args.warmup { + break elapsed; + } + }; + + section + .append(format_args!(" {warmup} iters in {elapsed:.2}s"))? + .close()?; - let iterations = usize::try_from(args.iter).expect("iterations out of bounds"); + let iterations = (((args.iter * warmup as f32) / args.warmup).round() as usize).max(1); + let step = (iterations / 10).max(1); let mut collected = Vec::try_with_capacity(iterations)?; - let step = (args.iter / 10).max(1); + let mut section = io.section("Running", Stream::Stdout, Color::Progress)?; + section.append(format_args!( + " {item} {} iterations for {:.2}s: ", + iterations, args.iter + ))?; - let mut section = io.section("Running", Stream::Stdout, Color::Highlight)?; - section.append(format_args!(" {item} ({} iters): ", args.iter))?; + let mut added = 0; - for n in 0..args.iter { + for n in 0..=iterations { if n % step == 0 { section.append(".")?; section.flush()?; + added += 1; } let start = Instant::now(); @@ -190,16 +201,25 @@ fn bench_fn(io: &mut Io<'_>, item: &dyn fmt::Display, args: &Flags, f: &Function drop(hint::black_box(value)); } + for _ in added..10 { + section.append(".")?; + section.flush()?; + } + + section.close()?; + collected.sort_unstable(); let len = collected.len() as f64; let average = collected.iter().copied().sum::() as f64 / len; + let variance = collected .iter() .copied() .map(|n| (n as f64 - average).powf(2.0)) .sum::() / len; + let stddev = variance.sqrt(); let format = Format { @@ -208,7 +228,8 @@ fn bench_fn(io: &mut Io<'_>, item: &dyn fmt::Display, args: &Flags, f: &Function iterations, }; - section.passed(format_args!(" {format}"))?.close()?; + let mut section = io.section("Result", Stream::Stdout, Color::Highlight)?; + section.append(format_args!(" {item}: {format}"))?.close()?; Ok(()) } diff --git a/crates/rune/src/cli/out.rs b/crates/rune/src/cli/out.rs index 6b5b57aab..7ff19c285 100644 --- a/crates/rune/src/cli/out.rs +++ b/crates/rune/src/cli/out.rs @@ -27,6 +27,7 @@ pub(super) enum Color { Ignore, Highlight, Important, + Progress, } impl Color { @@ -37,6 +38,7 @@ impl Color { Color::Ignore => &colors.ignored, Color::Highlight => &colors.highlight, Color::Important => &colors.important, + Color::Progress => &colors.progress, } } } @@ -105,6 +107,7 @@ struct Colors { passed: ColorSpec, highlight: ColorSpec, important: ColorSpec, + progress: ColorSpec, ignored: ColorSpec, } @@ -117,13 +120,14 @@ impl Colors { this.highlight.set_bold(true); this.important.set_fg(Some(termcolor::Color::White)); this.important.set_bold(true); + this.progress.set_fg(Some(termcolor::Color::Cyan)); + this.progress.set_bold(true); this.ignored.set_fg(Some(termcolor::Color::Yellow)); this.ignored.set_bold(true); this } } -#[must_use = "Section must be closed"] pub(super) struct Section<'a> { pub(super) io: &'a mut StandardStream, colors: &'a Colors, diff --git a/crates/rune/src/compile/meta.rs b/crates/rune/src/compile/meta.rs index e0b4ea454..719a2e665 100644 --- a/crates/rune/src/compile/meta.rs +++ b/crates/rune/src/compile/meta.rs @@ -6,7 +6,7 @@ use crate as rune; use crate::alloc::borrow::Cow; use crate::alloc::path::Path; use crate::alloc::prelude::*; -use crate::alloc::{self, Box, HashMap, Vec}; +use crate::alloc::{self, Box, Vec}; use crate::ast; use crate::ast::{Span, Spanned}; use crate::compile::attrs::Parser; @@ -15,7 +15,7 @@ use crate::compile::meta; use crate::compile::{self, ItemId, Location, MetaInfo, ModId, Pool, Visibility}; use crate::module::{DocFunction, ModuleItemCommon}; use crate::parse::ResolveContext; -use crate::runtime::{Call, Protocol}; +use crate::runtime::{Call, FieldMap, Protocol}; use crate::{Hash, Item, ItemBuf}; /// A meta reference to an item being compiled. @@ -300,8 +300,8 @@ pub struct FieldsNamed { impl FieldsNamed { /// Coerce into a hashmap of fields. - pub(crate) fn to_fields(&self) -> alloc::Result, usize>> { - let mut fields = HashMap::new(); + pub(crate) fn to_fields(&self) -> alloc::Result, usize>> { + let mut fields = crate::runtime::new_field_hash_map_with_capacity(self.fields.len())?; for f in self.fields.iter() { fields.try_insert(f.name.try_clone()?, f.position)?; diff --git a/crates/rune/src/compile/unit_builder.rs b/crates/rune/src/compile/unit_builder.rs index 5cf95ef7f..db24c78c4 100644 --- a/crates/rune/src/compile/unit_builder.rs +++ b/crates/rune/src/compile/unit_builder.rs @@ -317,7 +317,7 @@ impl UnitBuilder { hash, variant_hash: Hash::EMPTY, item: pool.item(meta.item_meta.item).try_to_owned()?, - fields: HashMap::new(), + fields: HashMap::default(), }); self.constants @@ -350,7 +350,7 @@ impl UnitBuilder { hash: meta.hash, variant_hash: Hash::EMPTY, item: pool.item(meta.item_meta.item).try_to_owned()?, - fields: HashMap::new(), + fields: HashMap::default(), }); if self @@ -409,7 +409,7 @@ impl UnitBuilder { hash: meta.hash, variant_hash: Hash::EMPTY, item: pool.item(meta.item_meta.item).try_to_owned()?, - fields: HashMap::new(), + fields: HashMap::default(), }); if self @@ -487,7 +487,7 @@ impl UnitBuilder { hash: enum_hash, variant_hash: meta.hash, item: pool.item(meta.item_meta.item).try_to_owned()?, - fields: HashMap::new(), + fields: HashMap::default(), }); if self @@ -537,7 +537,7 @@ impl UnitBuilder { hash: enum_hash, variant_hash: meta.hash, item: pool.item(meta.item_meta.item).try_to_owned()?, - fields: HashMap::new(), + fields: HashMap::default(), }); if self diff --git a/crates/rune/src/module/module_meta.rs b/crates/rune/src/module/module_meta.rs index 7220de068..eb2096d55 100644 --- a/crates/rune/src/module/module_meta.rs +++ b/crates/rune/src/module/module_meta.rs @@ -5,11 +5,10 @@ use ::rust_alloc::sync::Arc; use crate as rune; use crate::alloc; use crate::alloc::prelude::*; -use crate::alloc::HashMap; use crate::compile::context::{AttributeMacroHandler, MacroHandler, TraitHandler}; use crate::compile::{meta, Docs}; use crate::function_meta::AssociatedName; -use crate::runtime::{ConstValue, FunctionHandler, TypeInfo}; +use crate::runtime::{ConstValue, FieldMap, FunctionHandler, TypeInfo}; use crate::{Hash, ItemBuf}; #[doc(hidden)] @@ -89,9 +88,19 @@ pub(crate) enum Fields { } impl Fields { + /// Get the number of named fields. + #[inline] + fn size(&self) -> usize { + match self { + Fields::Named(fields) => fields.len(), + _ => 0, + } + } + /// Coerce into fields hash map. - pub(crate) fn to_fields(&self) -> alloc::Result, usize>> { - let mut fields = HashMap::new(); + #[inline] + pub(crate) fn to_fields(&self) -> alloc::Result, usize>> { + let mut fields = crate::runtime::new_field_hash_map_with_capacity(self.size())?; if let Fields::Named(names) = self { for (index, name) in names.iter().copied().enumerate() { diff --git a/crates/rune/src/modules/mem.rs b/crates/rune/src/modules/mem.rs index 43ba4e909..e372571b8 100644 --- a/crates/rune/src/modules/mem.rs +++ b/crates/rune/src/modules/mem.rs @@ -17,6 +17,8 @@ pub fn module() -> Result { m.function_meta(Snapshot::debug)?; m.function_meta(Snapshot::shared)?; m.function_meta(Snapshot::is_exclusive)?; + m.function_meta(Snapshot::is_readable)?; + m.function_meta(Snapshot::is_writable)?; Ok(m) } @@ -67,6 +69,8 @@ impl Snapshot { /// let s = snapshot(v)?; /// assert_eq!(s.shared(), 0); /// assert!(!s.is_exclusive()); + /// assert!(s.is_readable()); + /// assert!(s.is_writable()); /// /// // Assign to a separate variable since the compiler will notice that `v` is moved. /// let u = v; @@ -78,12 +82,80 @@ impl Snapshot { /// /// let s = snapshot(u)?; /// assert!(s.is_exclusive()); + /// assert!(!s.is_readable()); + /// assert!(!s.is_writable()); /// ``` #[rune::function] fn is_exclusive(&self) -> bool { self.inner.is_exclusive() } + /// Test if the snapshot indicates that the value is readable. + /// + /// # Examples + /// + /// ```rune + /// use std::mem::snapshot; + /// + /// let v = [1, 2, 3]; + /// + /// let s = snapshot(v)?; + /// assert_eq!(s.shared(), 0); + /// assert!(!s.is_exclusive()); + /// assert!(s.is_readable()); + /// assert!(s.is_writable()); + /// + /// // Assign to a separate variable since the compiler will notice that `v` is moved. + /// let u = v; + /// + /// // Move the value into a closure, causing the original reference to become exclusively held. + /// let closure = move || { + /// v + /// }; + /// + /// let s = snapshot(u)?; + /// assert!(s.is_exclusive()); + /// assert!(!s.is_readable()); + /// assert!(!s.is_writable()); + /// ``` + #[rune::function] + fn is_readable(&self) -> bool { + self.inner.is_readable() + } + + /// Test if the snapshot indicates that the value is writable. + /// + /// # Examples + /// + /// ```rune + /// use std::mem::snapshot; + /// + /// let v = [1, 2, 3]; + /// + /// let s = snapshot(v)?; + /// assert_eq!(s.shared(), 0); + /// assert!(!s.is_exclusive()); + /// assert!(s.is_readable()); + /// assert!(s.is_writable()); + /// + /// // Assign to a separate variable since the compiler will notice that `v` is moved. + /// let u = v; + /// + /// // Move the value into a closure, causing the original reference to become exclusively held. + /// let closure = move || { + /// v + /// }; + /// + /// let s = snapshot(u)?; + /// assert!(s.is_exclusive()); + /// assert!(!s.is_readable()); + /// assert!(!s.is_writable()); + /// ``` + #[rune::function] + fn is_writable(&self) -> bool { + self.inner.is_writable() + } + #[rune::function(protocol = DISPLAY_FMT)] fn display(&self, f: &mut Formatter) -> VmResult<()> { vm_write!(f, "{}", self.inner) diff --git a/crates/rune/src/runtime/access.rs b/crates/rune/src/runtime/access.rs index f9293649a..1e32a4be1 100644 --- a/crates/rune/src/runtime/access.rs +++ b/crates/rune/src/runtime/access.rs @@ -92,6 +92,11 @@ impl Snapshot { self.0 & MASK == 0 } + /// Test if the snapshot indicates that the value is writable. + pub(crate) fn is_writable(&self) -> bool { + self.0 & MASK == 0 + } + /// Test if access is exclusively held. pub(crate) fn is_exclusive(&self) -> bool { self.0 & MASK != 0 diff --git a/crates/rune/src/runtime/any_obj.rs b/crates/rune/src/runtime/any_obj.rs index 1cc708a3f..5fbc45e59 100644 --- a/crates/rune/src/runtime/any_obj.rs +++ b/crates/rune/src/runtime/any_obj.rs @@ -704,6 +704,7 @@ struct Shared { impl Shared { /// Increment the reference count of the inner value. + #[inline] unsafe fn inc(this: NonNull) { let count_ref = &*addr_of!((*this.as_ptr()).count); let count = count_ref.get(); @@ -726,6 +727,7 @@ impl Shared { /// # Safety /// /// ProtocolCaller needs to ensure that `this` is a valid pointer. + #[inline] unsafe fn dec(this: NonNull) { let count_ref = &*addr_of!((*this.as_ptr()).count); let count = count_ref.get(); diff --git a/crates/rune/src/runtime/borrow_mut.rs b/crates/rune/src/runtime/borrow_mut.rs index 05f5e6f76..6d043b234 100644 --- a/crates/rune/src/runtime/borrow_mut.rs +++ b/crates/rune/src/runtime/borrow_mut.rs @@ -20,6 +20,7 @@ pub struct BorrowMut<'a, T: ?Sized> { impl<'a, T: ?Sized> BorrowMut<'a, T> { /// Construct a borrow mut from static data. + #[inline] pub(crate) fn from_static(data: &mut T) -> Self { Self { data: NonNull::from(data), @@ -36,6 +37,7 @@ impl<'a, T: ?Sized> BorrowMut<'a, T> { /// ensure that access has been acquired correctly using e.g. /// [Access::exclusive]. Otherwise access can be release incorrectly once /// this guard is dropped. + #[inline] pub(crate) unsafe fn new(data: NonNull, guard: RawAccessGuard) -> Self { Self { data, @@ -60,6 +62,7 @@ impl<'a, T: ?Sized> BorrowMut<'a, T> { /// assert_eq!(&mut bytes[..], &mut [1u8, 2u8][..]); /// # Ok::<_, rune::support::Error>(()) /// ``` + #[inline] pub fn map(mut this: Self, m: impl FnOnce(&mut T) -> &mut U) -> BorrowMut<'a, U> { // SAFETY: This is safe per construction. unsafe { @@ -89,6 +92,7 @@ impl<'a, T: ?Sized> BorrowMut<'a, T> { /// assert_eq!(&mut bytes[..], &mut [1u8, 2u8][..]); /// # Ok::<_, rune::support::Error>(()) /// ``` + #[inline] pub fn try_map( mut this: Self, m: impl FnOnce(&mut T) -> Option<&mut U>, diff --git a/crates/rune/src/runtime/mod.rs b/crates/rune/src/runtime/mod.rs index 4dbce9e37..c98a00bf0 100644 --- a/crates/rune/src/runtime/mod.rs +++ b/crates/rune/src/runtime/mod.rs @@ -208,3 +208,17 @@ pub use self::control_flow::ControlFlow; mod hasher; #[cfg(feature = "alloc")] pub use self::hasher::Hasher; + +pub(crate) type FieldMap = crate::alloc::HashMap; + +#[inline(always)] +pub(crate) fn new_field_map() -> FieldMap { + FieldMap::new() +} + +#[inline(always)] +pub(crate) fn new_field_hash_map_with_capacity( + cap: usize, +) -> crate::alloc::Result> { + FieldMap::try_with_capacity(cap) +} diff --git a/crates/rune/src/runtime/object.rs b/crates/rune/src/runtime/object.rs index 5972981c5..6a1bbf8b3 100644 --- a/crates/rune/src/runtime/object.rs +++ b/crates/rune/src/runtime/object.rs @@ -5,12 +5,12 @@ use core::hash; use core::iter; use crate as rune; +use crate::alloc::hash_map; use crate::alloc::hashbrown::raw::RawIter; use crate::alloc::prelude::*; use crate::alloc::{self, String}; -use crate::alloc::{hash_map, HashMap}; use crate::runtime::{ - FromValue, ProtocolCaller, RawAnyGuard, Ref, ToValue, Value, VmError, VmResult, + FieldMap, FromValue, ProtocolCaller, RawAnyGuard, Ref, ToValue, Value, VmError, VmResult, }; use crate::Any; @@ -82,7 +82,7 @@ pub type Values<'a> = hash_map::Values<'a, String, Value>; #[repr(transparent)] #[rune(item = ::std::object)] pub struct Object { - inner: HashMap, + inner: FieldMap, } impl Object { @@ -98,7 +98,7 @@ impl Object { #[rune::function(keep, path = Self::new)] pub fn new() -> Self { Self { - inner: HashMap::new(), + inner: crate::runtime::new_field_map(), } } @@ -121,7 +121,7 @@ impl Object { // BTreeMap doesn't support setting capacity on creation but we keep // this here in case we want to switch store later. Ok(Self { - inner: HashMap::try_with_capacity(capacity)?, + inner: crate::runtime::new_field_hash_map_with_capacity(capacity)?, }) } @@ -247,6 +247,7 @@ impl Object { /// Inserts a key-value pair into the map. /// /// If the map did not have this key present, `None` is returned. + #[inline] pub fn insert(&mut self, k: String, v: Value) -> alloc::Result> { self.inner.try_insert(k, v) } @@ -259,11 +260,6 @@ impl Object { self.inner.clear(); } - /// Convert into inner. - pub fn into_inner(self) -> HashMap { - self.inner - } - /// An iterator visiting all key-value pairs in arbitrary order. /// The iterator element type is `(&'a String, &'a Value)`. pub fn iter(&self) -> Iter<'_> { diff --git a/crates/rune/src/runtime/static_string.rs b/crates/rune/src/runtime/static_string.rs index f3057fd04..0e35e853d 100644 --- a/crates/rune/src/runtime/static_string.rs +++ b/crates/rune/src/runtime/static_string.rs @@ -17,6 +17,7 @@ pub struct StaticString { } impl cmp::PartialEq for StaticString { + #[inline] fn eq(&self, other: &Self) -> bool { self.hash == other.hash && self.inner == other.inner } @@ -25,18 +26,21 @@ impl cmp::PartialEq for StaticString { impl cmp::Eq for StaticString {} impl cmp::PartialOrd for StaticString { + #[inline] fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } impl cmp::Ord for StaticString { + #[inline] fn cmp(&self, other: &Self) -> cmp::Ordering { self.inner.cmp(&other.inner) } } impl hash::Hash for StaticString { + #[inline] fn hash(&self, state: &mut H) { self.hash.hash(state) } @@ -44,6 +48,7 @@ impl hash::Hash for StaticString { impl StaticString { /// Construct a new static string. + #[inline] pub fn new(s: S) -> alloc::Result where S: AsRef, @@ -68,21 +73,23 @@ impl AsRef for StaticString { } impl fmt::Debug for StaticString { + #[inline] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}", &self.inner)?; - Ok(()) + write!(f, "{:?}", &self.inner) } } impl ops::Deref for StaticString { type Target = String; + #[inline] fn deref(&self) -> &Self::Target { &self.inner } } impl From for StaticString { + #[inline] fn from(inner: String) -> Self { let hash = inner.as_str().into_hash(); Self { inner, hash } diff --git a/crates/rune/src/runtime/value.rs b/crates/rune/src/runtime/value.rs index 82823cbbc..af1065457 100644 --- a/crates/rune/src/runtime/value.rs +++ b/crates/rune/src/runtime/value.rs @@ -447,42 +447,43 @@ impl Value { match &self.repr { Repr::Empty => { vm_try!(vm_write!(f, "")); - return VmResult::Ok(()); } Repr::Inline(value) => { vm_try!(vm_write!(f, "{value:?}")); - return VmResult::Ok(()); } Repr::Dynamic(ref value) => { vm_try!(value.debug_fmt_with(f, caller)); - return VmResult::Ok(()); } - Repr::Any(..) => {} - } - - // reborrow f to avoid moving it - let mut args = DynGuardedArgs::new((&mut *f,)); - - match vm_try!(caller.try_call_protocol_fn(Protocol::DEBUG_FMT, self.clone(), &mut args)) { - CallResultOnly::Ok(value) => { - vm_try!(<()>::from_value(value)); - } - CallResultOnly::Unsupported(value) => match &value.repr { - Repr::Empty => { - vm_try!(vm_write!(f, "")); - } - Repr::Inline(value) => { - vm_try!(vm_write!(f, "{value:?}")); - } - Repr::Dynamic(value) => { - let ty = value.type_info(); - vm_try!(vm_write!(f, "<{ty} object at {value:p}>")); - } - Repr::Any(value) => { - let ty = value.type_info(); - vm_try!(vm_write!(f, "<{ty} object at {value:p}>")); + Repr::Any(..) => { + // reborrow f to avoid moving it + let mut args = DynGuardedArgs::new((&mut *f,)); + + match vm_try!(caller.try_call_protocol_fn( + Protocol::DEBUG_FMT, + self.clone(), + &mut args + )) { + CallResultOnly::Ok(value) => { + vm_try!(<()>::from_value(value)); + } + CallResultOnly::Unsupported(value) => match &value.repr { + Repr::Empty => { + vm_try!(vm_write!(f, "")); + } + Repr::Inline(value) => { + vm_try!(vm_write!(f, "{value:?}")); + } + Repr::Dynamic(value) => { + let ty = value.type_info(); + vm_try!(vm_write!(f, "<{ty} object at {value:p}>")); + } + Repr::Any(value) => { + let ty = value.type_info(); + vm_try!(vm_write!(f, "<{ty} object at {value:p}>")); + } + }, } - }, + } } VmResult::Ok(()) @@ -1369,6 +1370,7 @@ impl Value { /// Coerce into a checked [`Inline`] object. /// /// Any empty value will cause an access error. + #[inline] pub(crate) fn as_inline(&self) -> Result, AccessError> { match &self.repr { Repr::Empty => Err(AccessError::empty()), @@ -1378,6 +1380,7 @@ impl Value { } } + #[inline] pub(crate) fn as_inline_mut(&mut self) -> Result, AccessError> { match &mut self.repr { Repr::Empty => Err(AccessError::empty()), @@ -1390,6 +1393,7 @@ impl Value { /// Coerce into a checked [`AnyObj`] object. /// /// Any empty value will cause an access error. + #[inline] pub(crate) fn as_any(&self) -> Result, AccessError> { match &self.repr { Repr::Empty => Err(AccessError::empty()), @@ -1399,6 +1403,7 @@ impl Value { } } + #[inline] pub(crate) fn take_repr(self) -> Result { match self.repr { Repr::Empty => Err(AccessError::empty()), @@ -1408,15 +1413,17 @@ impl Value { } } + #[inline] pub(crate) fn as_ref(&self) -> Result, AccessError> { match &self.repr { + Repr::Empty => Err(AccessError::empty()), Repr::Inline(value) => Ok(ReprRef::Inline(value)), Repr::Dynamic(value) => Ok(ReprRef::Dynamic(value)), Repr::Any(value) => Ok(ReprRef::Any(value)), - Repr::Empty => Err(AccessError::empty()), } } + #[inline] pub(crate) fn as_mut(&mut self) -> Result, AccessError> { match &mut self.repr { Repr::Empty => Err(AccessError::empty()), @@ -1426,27 +1433,29 @@ impl Value { } } + #[inline] pub(crate) fn try_borrow_ref(&self) -> Result>, AccessError> where T: Any, { match &self.repr { + Repr::Empty => Err(AccessError::empty()), Repr::Inline(..) => Ok(None), Repr::Dynamic(..) => Ok(None), Repr::Any(value) => value.try_borrow_ref(), - Repr::Empty => Err(AccessError::empty()), } } + #[inline] pub(crate) fn try_borrow_mut(&self) -> Result>, AccessError> where T: Any, { match &self.repr { + Repr::Empty => Err(AccessError::empty()), Repr::Inline(..) => Ok(None), Repr::Dynamic(..) => Ok(None), Repr::Any(value) => value.try_borrow_mut(), - Repr::Empty => Err(AccessError::empty()), } } @@ -1495,49 +1504,42 @@ impl Value { impl fmt::Debug for Value { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let snapshot = match &self.repr { + match &self.repr { Repr::Empty => { write!(f, "")?; - return Ok(()); } Repr::Inline(value) => { write!(f, "{value:?}")?; - return Ok(()); } - Repr::Dynamic(value) => value.snapshot(), - Repr::Any(value) => value.snapshot(), - }; - - if !snapshot.is_readable() { - write!(f, "<{snapshot}>")?; - return Ok(()); - } - - let mut s = String::new(); - let result = Formatter::format_with(&mut s, |f| self.debug_fmt(f)); + _ => { + let mut s = String::new(); + let result = Formatter::format_with(&mut s, |f| self.debug_fmt(f)); + + if let Err(e) = result.into_result() { + match &self.repr { + Repr::Empty => { + write!(f, "")?; + } + Repr::Inline(value) => { + write!(f, "<{value:?}: {e}>")?; + } + Repr::Dynamic(value) => { + let ty = value.type_info(); + write!(f, "<{ty} object at {value:p}: {e}>")?; + } + Repr::Any(value) => { + let ty = value.type_info(); + write!(f, "<{ty} object at {value:p}: {e}>")?; + } + } - if let Err(e) = result.into_result() { - match &self.repr { - Repr::Empty => { - write!(f, "")?; - } - Repr::Inline(value) => { - write!(f, "<{value:?}: {e}>")?; + return Ok(()); } - Repr::Dynamic(value) => { - let ty = value.type_info(); - write!(f, "<{ty} object at {value:p}: {e}>")?; - } - Repr::Any(value) => { - let ty = value.type_info(); - write!(f, "<{ty} object at {value:p}: {e}>")?; - } - } - return Ok(()); + f.write_str(s.as_str())?; + } } - f.write_str(s.as_str())?; Ok(()) } } diff --git a/crates/rune/src/runtime/value/dynamic.rs b/crates/rune/src/runtime/value/dynamic.rs index c9df704fa..9a99dcd2d 100644 --- a/crates/rune/src/runtime/value/dynamic.rs +++ b/crates/rune/src/runtime/value/dynamic.rs @@ -97,6 +97,7 @@ impl Dynamic { } /// Test if the value is sharable. + #[inline] pub(crate) fn is_readable(&self) -> bool { // Safety: Since we have a reference to this shared, we know that the // inner is available. @@ -104,29 +105,34 @@ impl Dynamic { } /// Test if the value is exclusively accessible. + #[inline] pub(crate) fn is_writable(&self) -> bool { unsafe { self.shared.as_ref().access.is_exclusive() } } /// Get access snapshot of shared value. + #[inline] pub(crate) fn snapshot(&self) -> Snapshot { // SAFETY: We know that the shared pointer is valid. unsafe { self.shared.as_ref().access.snapshot() } } /// Get the size of the dynamic collection of values. + #[inline] pub(crate) fn len(&self) -> usize { // SAFETY: We know that the shared pointer is valid. unsafe { self.shared.as_ref().len } } /// Get runtime type information of the dynamic value. + #[inline] pub(crate) fn rtti(&self) -> &H { // SAFETY: We know that the shared pointer is valid. unsafe { &self.shared.as_ref().rtti } } /// Borrow the interior data array by reference. + #[inline] pub(crate) fn borrow_ref(&self) -> Result, AccessError> { // SAFETY: We know the layout is valid since it is reference counted. unsafe { @@ -138,6 +144,7 @@ impl Dynamic { } /// Borrow the interior data array by mutable reference. + #[inline] pub(crate) fn borrow_mut(&self) -> Result, AccessError> { // SAFETY: We know the layout is valid since it is reference counted. unsafe { @@ -149,6 +156,7 @@ impl Dynamic { } /// Take the interior value and drop it if necessary. + #[inline] pub(crate) fn drop(self) -> Result<(), AccessError> { // SAFETY: We've checked for the appropriate type just above. unsafe { @@ -218,6 +226,7 @@ impl Clone for Dynamic { } } +#[repr(C)] struct Shared { /// Run time type information of the shared value. rtti: H, @@ -232,6 +241,7 @@ struct Shared { } impl Shared { + #[inline] fn layout(len: usize) -> Result { let array = Layout::array::(len)?; Layout::from_size_align( @@ -241,16 +251,19 @@ impl Shared { } /// Get the rtti pointer in the shared container. + #[inline] unsafe fn as_rtti_ptr(this: NonNull) -> NonNull { NonNull::new_unchecked(addr_of_mut!((*this.as_ptr()).rtti)) } /// Get the data pointer in the shared container. + #[inline] unsafe fn as_data_ptr(this: NonNull) -> NonNull { NonNull::new_unchecked(addr_of_mut!((*this.as_ptr()).data)).cast::() } /// Increment the reference count of the inner value. + #[inline] unsafe fn inc(this: NonNull) { let count_ref = &*addr_of!((*this.as_ptr()).count); let count = count_ref.get(); @@ -273,6 +286,7 @@ impl Shared { /// # Safety /// /// ProtocolCaller needs to ensure that `this` is a valid pointer. + #[inline] unsafe fn dec(this: NonNull) { let count_ref = &*addr_of!((*this.as_ptr()).count); let access = &*addr_of!((*this.as_ptr()).access); @@ -307,6 +321,7 @@ impl Shared { Global.deallocate(this.cast(), layout); } + #[inline] unsafe fn drop_values(this: NonNull, len: usize) { if needs_drop::() { let data = Self::as_data_ptr(this); @@ -335,11 +350,7 @@ impl Dynamic, T> { return Ok(None); }; - let values = self.borrow_ref()?; - - let index = *index; - let value = BorrowRef::try_map(values, |value| value.get(index)); - Ok(value.ok()) + self.get_ref(*index) } /// Access a field mutably by name. @@ -349,27 +360,41 @@ impl Dynamic, T> { return Ok(None); }; - let values = self.borrow_mut()?; - - let index = *index; - let value = BorrowMut::try_map(values, |value| value.get_mut(index)); - Ok(value.ok()) + self.get_mut(*index) } /// Access a field by index. #[inline] pub(crate) fn get_ref(&self, index: usize) -> Result>, AccessError> { - let values = self.borrow_ref()?; - let value = BorrowRef::try_map(values, |value| value.get(index)); - Ok(value.ok()) + // SAFETY: We know the layout is valid since it is reference counted. + unsafe { + let shared = self.shared.as_ref(); + + if index >= shared.len { + return Ok(None); + } + + let guard = shared.access.shared()?; + let data = Shared::as_data_ptr(self.shared).add(index); + Ok(Some(BorrowRef::new(data, guard.into_raw()))) + } } /// Access a field mutably by index. #[inline] pub(crate) fn get_mut(&self, index: usize) -> Result>, AccessError> { - let values = self.borrow_mut()?; - let value = BorrowMut::try_map(values, |value| value.get_mut(index)); - Ok(value.ok()) + // SAFETY: We know the layout is valid since it is reference counted. + unsafe { + let shared = self.shared.as_ref(); + + if index >= shared.len { + return Ok(None); + } + + let guard = shared.access.exclusive()?; + let data = Shared::as_data_ptr(self.shared).add(index); + Ok(Some(BorrowMut::new(data, guard.into_raw()))) + } } } diff --git a/crates/rune/src/runtime/value/rtti.rs b/crates/rune/src/runtime/value/rtti.rs index 4b45e413d..479f479eb 100644 --- a/crates/rune/src/runtime/value/rtti.rs +++ b/crates/rune/src/runtime/value/rtti.rs @@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize}; use crate::alloc::prelude::*; use crate::alloc::HashMap; use crate::item::Item; -use crate::runtime::{TypeInfo, Value}; +use crate::runtime::{FieldMap, TypeInfo, Value}; use crate::{Hash, ItemBuf}; /// Field accessor for a variant struct. @@ -56,7 +56,7 @@ pub struct Rtti { /// The item of the type. pub(crate) item: ItemBuf, /// Mapping from field names to their corresponding indexes. - pub(crate) fields: HashMap, usize>, + pub(crate) fields: FieldMap, usize>, } impl Rtti { diff --git a/crates/rune/src/runtime/vm.rs b/crates/rune/src/runtime/vm.rs index a737b92ed..1cbd1e715 100644 --- a/crates/rune/src/runtime/vm.rs +++ b/crates/rune/src/runtime/vm.rs @@ -1128,72 +1128,41 @@ impl Vm { } } - /// Implementation of getting a string index on an object-like type. - fn try_object_slot_index_get( - &mut self, - target: Value, - slot: usize, - out: Output, - ) -> VmResult> { - let index = vm_try!(self.unit.lookup_string(slot)); - - 'fallback: { - match vm_try!(target.as_ref()) { - ReprRef::Dynamic(data) if matches!(data.rtti().kind, RttiKind::Struct) => { - if let Some(value) = vm_try!(data.get_field_ref(index.as_str())) { - vm_try!(out.store(&mut self.stack, || value.clone())); - return VmResult::Ok(CallResult::Ok(())); - } - } - ReprRef::Any(value) => match value.type_hash() { - Object::HASH => { - let object = vm_try!(value.borrow_ref::()); - - if let Some(value) = object.get(index.as_str()) { - vm_try!(out.store(&mut self.stack, || value.clone())); - return VmResult::Ok(CallResult::Ok(())); - } - } - _ => break 'fallback, - }, - _ => { - return VmResult::Ok(CallResult::Unsupported(target.clone())); - } - } - - return err(VmErrorKind::ObjectIndexMissing { slot }); - }; - - let hash = index.hash(); - let result = vm_try!(self.call_field_fn(Protocol::GET, target, hash, &mut (), out)); - VmResult::Ok(result) - } - - fn try_object_slot_index_set(target: &Value, field: &str, value: &Value) -> VmResult { - match vm_try!(target.as_ref()) { + fn try_object_slot_index_set( + target: &Value, + field: &str, + value: &Value, + ) -> Result { + match target.as_ref()? { ReprRef::Dynamic(data) if matches!(data.rtti().kind, RttiKind::Struct) => { - if let Some(mut v) = vm_try!(data.get_field_mut(field)) { + if let Some(mut v) = data.get_field_mut(field)? { v.clone_from(value); - return VmResult::Ok(true); + return Ok(true); } - err(VmErrorKind::MissingField { + Err(VmErrorKind::MissingField { target: data.type_info(), - field: vm_try!(field.try_to_owned()), + field: field.try_to_owned()?, }) } ReprRef::Any(target) => match target.type_hash() { Object::HASH => { - let key = vm_try!(field.try_to_owned()); - let mut object = vm_try!(target.borrow_mut::()); - vm_try!(object.insert(key, value.clone())); - VmResult::Ok(true) + let mut target = target.borrow_mut::()?; + + if let Some(target) = target.get_mut(field) { + target.clone_from(value); + } else { + let key = field.try_to_owned()?; + target.insert(key, value.clone())?; + } + + Ok(true) } - _ => VmResult::Ok(false), + _ => Ok(false), }, - target => err(VmErrorKind::MissingField { + target => Err(VmErrorKind::MissingField { target: target.type_info(), - field: vm_try!(field.try_to_owned()), + field: field.try_to_owned()?, }), } } @@ -2522,15 +2491,11 @@ impl Vm { let index = vm_try!(self.stack.at(index)); let value = vm_try!(self.stack.at(value)); - 'fallback: { - let Some(field) = vm_try!(index.try_borrow_ref::()) else { - break 'fallback; - }; - + if let Some(field) = vm_try!(index.try_borrow_ref::()) { if vm_try!(Self::try_object_slot_index_set(target, &field, value)) { return VmResult::Ok(()); } - }; + } let target = target.clone(); let index = index.clone(); @@ -2794,10 +2759,42 @@ impl Vm { slot: usize, out: Output, ) -> VmResult<()> { - let target = vm_try!(self.stack.at(addr)).clone(); + let target = vm_try!(self.stack.at(addr)); + let index = vm_try!(self.unit.lookup_string(slot)); + + match vm_try!(target.as_ref()) { + ReprRef::Dynamic(data) if matches!(data.rtti().kind, RttiKind::Struct) => { + let Some(value) = vm_try!(data.get_field_ref(index.as_str())) else { + return err(VmErrorKind::ObjectIndexMissing { slot }); + }; + + let value = value.clone(); + vm_try!(out.store(&mut self.stack, value)); + return VmResult::Ok(()); + } + ReprRef::Any(value) if value.type_hash() == Object::HASH => { + let object = vm_try!(value.borrow_ref::()); + + let Some(value) = object.get(index.as_str()) else { + return err(VmErrorKind::ObjectIndexMissing { slot }); + }; + + let value = value.clone(); + vm_try!(out.store(&mut self.stack, value)); + return VmResult::Ok(()); + } + ReprRef::Any(..) => {} + target => { + return err(VmErrorKind::UnsupportedObjectSlotIndexGet { + target: target.type_info(), + }); + } + } + + let target = target.clone(); if let CallResult::Unsupported(target) = - vm_try!(self.try_object_slot_index_get(target, slot, out)) + vm_try!(self.call_field_fn(Protocol::GET, target, index.hash(), &mut (), out)) { return err(VmErrorKind::UnsupportedObjectSlotIndexGet { target: vm_try!(target.type_info()),