Skip to content

Commit

Permalink
perf(vm): Copy instead of clone unrooted gc values
Browse files Browse the repository at this point in the history
  • Loading branch information
Marwes committed Aug 3, 2019
1 parent 08767e3 commit a0396f4
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 60 deletions.
2 changes: 1 addition & 1 deletion vm/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::base::{
};
use crate::{
forget_lifetime,
gc::{DataDef, GcRef, Move, Trace},
gc::{CloneUnrooted, DataDef, GcRef, Move, Trace},
thread::{RootedThread, ThreadInternal, VmRoot, VmRootInternal},
types::{VmIndex, VmInt, VmTag},
value::{ArrayDef, ArrayRepr, ClosureData, DataStruct, Value, ValueArray, ValueRepr},
Expand Down
2 changes: 1 addition & 1 deletion vm/src/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
primitive, Function, FunctionRef, Generic, Getable, OpaqueRef, OpaqueValue, OwnedFunction,
Pushable, Pushed, RuntimeResult, Unrooted, VmType, WithVM, IO,
},
gc::{self, GcPtr, Trace},
gc::{self, CloneUnrooted, GcPtr, Trace},
stack::{ClosureState, ExternState, State},
thread::{ActiveThread, ThreadInternal},
types::VmInt,
Expand Down
14 changes: 14 additions & 0 deletions vm/src/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,13 +385,23 @@ impl<T: ?Sized> From<OwnedPtr<T>> for GcPtr<T> {
}
}

/// SAFETY The only unsafety from copying the type is the creation of an unrooted value
pub unsafe trait CopyUnrooted: CloneUnrooted<Value = Self> + Sized {
unsafe fn copy_unrooted(&self) -> Self {
ptr::read(self)
}
}

pub trait CloneUnrooted {
type Value;
/// Creates a clone of the value that is not rooted. To ensure safety the value must be
/// forgotten or rooted before the next garbage collection
unsafe fn clone_unrooted(&self) -> Self::Value;
}

impl<T: ?Sized + CloneUnrooted> CloneUnrooted for &'_ T {
type Value = T::Value;
#[inline]
unsafe fn clone_unrooted(&self) -> Self::Value {
(**self).clone_unrooted()
}
Expand Down Expand Up @@ -421,6 +431,7 @@ where
T: CloneUnrooted,
{
type Value = T::Value;
#[inline]
unsafe fn clone_unrooted(&self) -> Self::Value {
self.0.clone_unrooted()
}
Expand Down Expand Up @@ -494,6 +505,7 @@ impl<'gc, T: ?Sized> From<OwnedGcRef<'gc, T>> for GcRef<'gc, T> {
}

impl<'a, T: ?Sized> Clone for GcRef<'a, T> {
#[inline]
fn clone(&self) -> Self {
// SAFETY The lifetime of the new value is the same which just means that both values need
// to be dropped before any gc collection
Expand Down Expand Up @@ -577,8 +589,10 @@ impl<T: ?Sized + fmt::Display> fmt::Display for GcPtr<T> {
}
}

unsafe impl<T: ?Sized> CopyUnrooted for GcPtr<T> {}
impl<T: ?Sized> CloneUnrooted for GcPtr<T> {
type Value = Self;
#[inline]
unsafe fn clone_unrooted(&self) -> Self {
GcPtr(self.0)
}
Expand Down
2 changes: 1 addition & 1 deletion vm/src/lazy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
generic::A, FunctionRef, Getable, OpaqueValue, Pushable, Pushed, Userdata, VmType, WithVM,
},
base::types::{self, ArcType},
gc::{GcPtr, GcRef, Move, Trace},
gc::{CloneUnrooted, GcPtr, GcRef, Move, Trace},
thread::{RootedThread, ThreadInternal},
value::{Cloner, Value},
vm::Thread,
Expand Down
17 changes: 9 additions & 8 deletions vm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,15 @@ mod value;

use std::{self as real_std, fmt, marker::PhantomData};

use crate::api::{ValueRef, VmType};
use crate::base::metadata::Metadata;
use crate::base::symbol::Symbol;
use crate::base::types::ArcType;
use crate::stack::Stacktrace;
use crate::thread::{RootedThread, RootedValue, Thread};
use crate::types::{VmIndex, VmInt};
use crate::value::{Value, ValueRepr};
use crate::{
api::{ValueRef, VmType},
gc::CloneUnrooted,
stack::Stacktrace,
thread::{RootedThread, RootedValue, Thread},
types::{VmIndex, VmInt},
value::{Value, ValueRepr},
};
use crate::{base::metadata::Metadata, base::symbol::Symbol, base::types::ArcType};

unsafe fn forget_lifetime<'a, 'b, T: ?Sized>(x: &'a T) -> &'b T {
::std::mem::transmute(x)
Expand Down
2 changes: 1 addition & 1 deletion vm/src/reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::real_std::{any::Any, fmt, marker::PhantomData, sync::Mutex};

use crate::{
api::{generic::A, Generic, RuntimeResult, Unrooted, Userdata, WithVM},
gc::{GcPtr, GcRef, Move, Trace},
gc::{CloneUnrooted, GcPtr, GcRef, Move, Trace},
thread::ThreadInternal,
value::{Cloner, Value},
vm::Thread,
Expand Down
37 changes: 15 additions & 22 deletions vm/src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{
use crate::base::{pos::Line, symbol::Symbol};

use crate::{
gc::{self, CloneUnrooted, GcPtr, Trace},
gc::{self, CloneUnrooted, CopyUnrooted, GcPtr, Trace},
types::VmIndex,
value::{ClosureData, DataStruct, ExternFunction, Value, ValueRepr},
Variants,
Expand Down Expand Up @@ -116,13 +116,12 @@ pub struct ClosureState {
pub(crate) instruction_index: usize,
}

unsafe impl CopyUnrooted for ClosureState {}
impl CloneUnrooted for ClosureState {
type Value = Self;
#[inline]
unsafe fn clone_unrooted(&self) -> Self {
ClosureState {
closure: self.closure.clone_unrooted(),
instruction_index: self.instruction_index,
}
self.copy_unrooted()
}
}

Expand Down Expand Up @@ -155,14 +154,12 @@ pub struct ExternState {
locked: Option<VmIndex>,
}

unsafe impl CopyUnrooted for ExternState {}
impl CloneUnrooted for ExternState {
type Value = Self;
#[inline]
unsafe fn clone_unrooted(&self) -> Self {
ExternState {
function: self.function.clone_unrooted(),
call_state: self.call_state.clone(),
locked: self.locked.clone(),
}
self.copy_unrooted()
}
}

Expand All @@ -182,7 +179,7 @@ impl ExternState {
}
}

pub trait StackState: CloneUnrooted<Value = Self> + Sized {
pub trait StackState: CopyUnrooted + Sized {
fn from_state(state: &State) -> &Self;
fn from_state_mut(state: &mut State) -> &mut Self;
fn to_state(&self) -> gc::Borrow<State>;
Expand Down Expand Up @@ -255,14 +252,12 @@ pub enum State {
Extern(#[cfg_attr(feature = "serde_derive", serde(state))] ExternState),
}

unsafe impl CopyUnrooted for State {}
impl CloneUnrooted for State {
type Value = Self;
#[inline]
unsafe fn clone_unrooted(&self) -> Self {
match self {
State::Unknown => State::Unknown,
State::Closure(x) => State::Closure(x.clone_unrooted()),
State::Extern(x) => State::Extern(x.clone_unrooted()),
}
self.copy_unrooted()
}
}

Expand Down Expand Up @@ -292,17 +287,15 @@ pub struct Frame<S = State> {
pub excess: bool,
}

unsafe impl<S> CopyUnrooted for Frame<S> where S: CopyUnrooted {}
impl<S> CloneUnrooted for Frame<S>
where
S: CloneUnrooted<Value = S>,
S: CopyUnrooted,
{
type Value = Self;
#[inline]
unsafe fn clone_unrooted(&self) -> Self {
Frame {
offset: self.offset,
state: self.state.clone_unrooted(),
excess: self.excess,
}
self.copy_unrooted()
}
}

Expand Down
50 changes: 25 additions & 25 deletions vm/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ use crate::base::{
use crate::{
array::Array,
compiler::DebugInfo,
gc::{self, CloneUnrooted, DataDef, Gc, GcPtr, GcRef, Generation, Move, Trace, WriteOnly},
gc::{
self, CloneUnrooted, CopyUnrooted, DataDef, Gc, GcPtr, GcRef, Generation, Move, Trace,
WriteOnly,
},
interner::InternedStr,
thread::{Status, Thread},
types::*,
Expand Down Expand Up @@ -462,6 +465,16 @@ impl From<ValueRepr> for Value {
}
}

unsafe impl CopyUnrooted for Value {}

impl CloneUnrooted for Value {
type Value = Self;
#[inline]
unsafe fn clone_unrooted(&self) -> Self {
self.copy_unrooted()
}
}

impl Value {
pub(crate) fn from_ref(v: &ValueRepr) -> &Value {
unsafe { mem::transmute(v) }
Expand All @@ -483,10 +496,6 @@ impl Value {
self.clone_unrooted()
}

pub unsafe fn clone_unrooted(&self) -> Value {
Value(self.0.clone_unrooted())
}

pub fn get_variants(&self) -> Variants {
Variants::new(self)
}
Expand Down Expand Up @@ -537,23 +546,15 @@ impl ValueRepr {
pub fn get_variants(&self) -> Variants {
Variants::new(Value::from_ref(self))
}
}

pub(crate) unsafe fn clone_unrooted(&self) -> Self {
use self::ValueRepr::*;
match self {
String(p) => String(p.clone_unrooted()),
Data(p) => Data(p.clone_unrooted()),
Function(p) => Function(p.clone_unrooted()),
Closure(p) => Closure(p.clone_unrooted()),
Array(p) => Array(p.clone_unrooted()),
PartialApplication(p) => PartialApplication(p.clone_unrooted()),
Userdata(p) => Userdata(p.clone_unrooted()),
Thread(p) => Thread(p.clone_unrooted()),
Tag(p) => Tag(*p),
Byte(p) => Byte(*p),
Int(p) => Int(*p),
Float(p) => Float(*p),
}
unsafe impl CopyUnrooted for ValueRepr {}

impl CloneUnrooted for ValueRepr {
type Value = Self;
#[inline]
unsafe fn clone_unrooted(&self) -> Self {
self.copy_unrooted()
}
}

Expand Down Expand Up @@ -890,13 +891,12 @@ pub(crate) enum Callable {
Extern(#[cfg_attr(feature = "serde_derive", serde(state))] GcPtr<ExternFunction>),
}

unsafe impl CopyUnrooted for Callable {}
impl CloneUnrooted for Callable {
type Value = Self;
#[inline]
unsafe fn clone_unrooted(&self) -> Self {
match self {
Callable::Closure(closure) => Callable::Closure(closure.clone_unrooted()),
Callable::Extern(ext) => Callable::Extern(ext.clone_unrooted()),
}
self.copy_unrooted()
}
}

Expand Down
2 changes: 1 addition & 1 deletion vm/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::base::{
use crate::{
api::{ValueRef, IO},
compiler::{CompiledFunction, CompiledModule, CompilerEnv, Variable},
gc::{Gc, GcPtr, GcRef, Generation, Move, Trace},
gc::{CloneUnrooted, Gc, GcPtr, GcRef, Generation, Move, Trace},
interner::{InternedStr, Interner},
lazy::Lazy,
macros::MacroEnv,
Expand Down

0 comments on commit a0396f4

Please sign in to comment.