Skip to content

Commit

Permalink
vm: Optimize value cloning
Browse files Browse the repository at this point in the history
  • Loading branch information
udoprog committed Jul 29, 2024
1 parent 1ec7ae5 commit 01fca91
Show file tree
Hide file tree
Showing 7 changed files with 330 additions and 161 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- run: cargo run --bin rune -- fmt --recursive --verbose --check
# - run: cargo run --bin rune -- fmt --verbose --check

clippy:
runs-on: ubuntu-latest
Expand Down
11 changes: 1 addition & 10 deletions crates/rune/src/runtime/inst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};

use crate as rune;
use crate::alloc::prelude::*;
use crate::runtime::{Call, FormatSpec, Memory, Mutable, Type, Value, VmError, VmResult};
use crate::runtime::{Call, FormatSpec, Memory, Type, Value, VmError, VmResult};
use crate::Hash;

/// Pre-canned panic reasons.
Expand Down Expand Up @@ -1307,15 +1307,6 @@ impl IntoOutput for Value {
}
}

impl IntoOutput for Mutable {
type Output = Mutable;

#[inline]
fn into_output(self) -> VmResult<Self::Output> {
VmResult::Ok(self)
}
}

/// How an instruction addresses a value.
#[derive(
Default,
Expand Down
2 changes: 1 addition & 1 deletion crates/rune/src/runtime/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ impl Object {
return VmResult::Ok(false);
};

if !vm_try!(Value::partial_eq_with(v1, &v2, caller)) {
if !vm_try!(Value::partial_eq_with(v1, v2, caller)) {
return VmResult::Ok(false);
}
}
Expand Down
31 changes: 27 additions & 4 deletions crates/rune/src/runtime/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,13 +220,30 @@ impl<T: ?Sized> TryClone for Shared<T> {
}

impl<T: ?Sized> Clone for Shared<T> {
#[inline]
fn clone(&self) -> Self {
// SAFETY: We know that the inner value is live in this instance.
unsafe {
SharedBox::inc(self.inner);
}

Self { inner: self.inner }
}

#[inline]
fn clone_from(&mut self, source: &Self) {
if ptr::eq(self.inner.as_ptr(), source.inner.as_ptr()) {
return;
}

// SAFETY: We know that the inner value is live in both instances.
unsafe {
SharedBox::dec(self.inner);
SharedBox::inc(source.inner);
}

self.inner = source.inner;
}
}

impl<T: ?Sized> Drop for Shared<T> {
Expand Down Expand Up @@ -274,7 +291,12 @@ impl<T: ?Sized> SharedBox<T> {
let count_ref = &*ptr::addr_of!((*this.as_ptr()).count);
let count = count_ref.get();

if count == 0 || count == usize::MAX {
debug_assert_ne!(
count, 0,
"Reference count of zero should only happen if Shared is incorrectly implemented"
);

if count == usize::MAX {
crate::alloc::abort();
}

Expand All @@ -291,9 +313,10 @@ impl<T: ?Sized> SharedBox<T> {
let count_ref = &*ptr::addr_of!((*this.as_ptr()).count);
let count = count_ref.get();

if count == 0 {
crate::alloc::abort();
}
debug_assert_ne!(
count, 0,
"Reference count of zero should only happen if Shared is incorrectly implemented"
);

let count = count - 1;
count_ref.set(count);
Expand Down
31 changes: 30 additions & 1 deletion crates/rune/src/runtime/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use core::slice;
use crate::alloc::alloc::Global;
use crate::alloc::prelude::*;
use crate::alloc::{self, Vec};
use crate::runtime::{InstAddress, Value, VmErrorKind};
use crate::runtime::{InstAddress, Output, Value, VmErrorKind};

/// An error raised when accessing an address on the stack.
#[derive(Debug, PartialEq)]
Expand Down Expand Up @@ -487,6 +487,35 @@ impl Stack {
Ok(())
}

/// Copy the value at the given address to the output.
pub(crate) fn copy(&mut self, from: InstAddress, out: Output) -> Result<(), StackError> {
let Some(to) = out.as_addr() else {
return Ok(());
};

if from == to {
return Ok(());
}

let from = self.top.wrapping_add(from.offset());
let to = self.top.wrapping_add(to.offset());

if from.max(to) >= self.stack.len() {
return Err(StackError {
addr: InstAddress::new(from.max(to).wrapping_sub(self.top)),
});
}

// SAFETY: We've checked that both addresses are in-bound and different
// just above.
unsafe {
let ptr = self.stack.as_mut_ptr();
(*ptr.add(to)).clone_from(&*ptr.add(from).cast_const());
}

Ok(())
}

/// Get a pair of addresses.
pub(crate) fn pair(&mut self, a: InstAddress, b: InstAddress) -> Result<Pair<'_>, StackError> {
if a == b {
Expand Down
52 changes: 51 additions & 1 deletion crates/rune/src/runtime/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ pub(crate) enum ValueShared {
}

/// An entry on the stack.
#[derive(Clone)]
pub struct Value {
repr: Repr,
}
Expand Down Expand Up @@ -2001,6 +2000,15 @@ impl From<Inline> for Value {
}
}

impl IntoOutput for Inline {
type Output = Inline;

#[inline]
fn into_output(self) -> VmResult<Self::Output> {
VmResult::Ok(self)
}
}

impl TryFrom<Mutable> for Value {
type Error = alloc::Error;

Expand All @@ -2012,6 +2020,15 @@ impl TryFrom<Mutable> for Value {
}
}

impl IntoOutput for Mutable {
type Output = Mutable;

#[inline]
fn into_output(self) -> VmResult<Self::Output> {
VmResult::Ok(self)
}
}

impl ToValue for Value {
#[inline]
fn to_value(self) -> VmResult<Value> {
Expand Down Expand Up @@ -2067,6 +2084,35 @@ impl MaybeTypeOf for Value {
}
}

impl Clone for Value {
#[inline]
fn clone(&self) -> Self {
let repr = match &self.repr {
Repr::Empty => Repr::Empty,
Repr::Inline(inline) => Repr::Inline(*inline),
Repr::Mutable(mutable) => Repr::Mutable(mutable.clone()),
};

Self { repr }
}

#[inline]
fn clone_from(&mut self, source: &Self) {
match (&mut self.repr, &source.repr) {
(Repr::Empty, Repr::Empty) => {}
(Repr::Inline(lhs), Repr::Inline(rhs)) => {
*lhs = *rhs;
}
(Repr::Mutable(lhs), Repr::Mutable(rhs)) => {
lhs.clone_from(rhs);
}
(lhs, rhs) => {
*lhs = rhs.clone();
}
}
}
}

impl TryClone for Value {
fn try_clone(&self) -> alloc::Result<Self> {
// NB: value cloning is a shallow clone of the underlying data.
Expand Down Expand Up @@ -2402,10 +2448,14 @@ impl Mutable {
}
}

/// Ensures that `Value` and `Repr` is niche-filled when used in common
/// combinations.
#[test]
fn size_of_value() {
use core::mem::size_of;

assert_eq!(size_of::<Repr>(), size_of::<Inline>());
assert_eq!(size_of::<Repr>(), size_of::<Value>());
assert_eq!(size_of::<Option<Value>>(), size_of::<Value>());
assert_eq!(size_of::<Option<Repr>>(), size_of::<Repr>());
}
Loading

0 comments on commit 01fca91

Please sign in to comment.