From a0396f4039641ad228b3cce2b0bfa0f443742c57 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Fri, 2 Aug 2019 16:47:11 +0200 Subject: [PATCH] perf(vm): Copy instead of clone unrooted gc values --- vm/src/api/mod.rs | 2 +- vm/src/channel.rs | 2 +- vm/src/gc.rs | 14 +++++++++++++ vm/src/lazy.rs | 2 +- vm/src/lib.rs | 17 +++++++-------- vm/src/reference.rs | 2 +- vm/src/stack.rs | 37 ++++++++++++++------------------- vm/src/value.rs | 50 ++++++++++++++++++++++----------------------- vm/src/vm.rs | 2 +- 9 files changed, 68 insertions(+), 60 deletions(-) diff --git a/vm/src/api/mod.rs b/vm/src/api/mod.rs index 07b7e2c09e..2ba2d97e53 100644 --- a/vm/src/api/mod.rs +++ b/vm/src/api/mod.rs @@ -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}, diff --git a/vm/src/channel.rs b/vm/src/channel.rs index a6a8236f7f..2ed847c391 100644 --- a/vm/src/channel.rs +++ b/vm/src/channel.rs @@ -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, diff --git a/vm/src/gc.rs b/vm/src/gc.rs index c96cb10d28..17087e9a8e 100644 --- a/vm/src/gc.rs +++ b/vm/src/gc.rs @@ -385,13 +385,23 @@ impl From> for GcPtr { } } +/// SAFETY The only unsafety from copying the type is the creation of an unrooted value +pub unsafe trait CopyUnrooted: CloneUnrooted + 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 CloneUnrooted for &'_ T { type Value = T::Value; + #[inline] unsafe fn clone_unrooted(&self) -> Self::Value { (**self).clone_unrooted() } @@ -421,6 +431,7 @@ where T: CloneUnrooted, { type Value = T::Value; + #[inline] unsafe fn clone_unrooted(&self) -> Self::Value { self.0.clone_unrooted() } @@ -494,6 +505,7 @@ impl<'gc, T: ?Sized> From> 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 @@ -577,8 +589,10 @@ impl fmt::Display for GcPtr { } } +unsafe impl CopyUnrooted for GcPtr {} impl CloneUnrooted for GcPtr { type Value = Self; + #[inline] unsafe fn clone_unrooted(&self) -> Self { GcPtr(self.0) } diff --git a/vm/src/lazy.rs b/vm/src/lazy.rs index 0b1cbfb725..4494fc7496 100644 --- a/vm/src/lazy.rs +++ b/vm/src/lazy.rs @@ -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, diff --git a/vm/src/lib.rs b/vm/src/lib.rs index 8083e2fbb7..598f2b862b 100644 --- a/vm/src/lib.rs +++ b/vm/src/lib.rs @@ -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) diff --git a/vm/src/reference.rs b/vm/src/reference.rs index 3bb46e15f9..64a05b076e 100644 --- a/vm/src/reference.rs +++ b/vm/src/reference.rs @@ -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, diff --git a/vm/src/stack.rs b/vm/src/stack.rs index 1b0d83df1a..0101680895 100644 --- a/vm/src/stack.rs +++ b/vm/src/stack.rs @@ -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, @@ -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() } } @@ -155,14 +154,12 @@ pub struct ExternState { locked: Option, } +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() } } @@ -182,7 +179,7 @@ impl ExternState { } } -pub trait StackState: CloneUnrooted + 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; @@ -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() } } @@ -292,17 +287,15 @@ pub struct Frame { pub excess: bool, } +unsafe impl CopyUnrooted for Frame where S: CopyUnrooted {} impl CloneUnrooted for Frame where - S: CloneUnrooted, + 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() } } diff --git a/vm/src/value.rs b/vm/src/value.rs index 3abe4f4564..046ab6e4f4 100644 --- a/vm/src/value.rs +++ b/vm/src/value.rs @@ -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::*, @@ -462,6 +465,16 @@ impl From 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) } @@ -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) } @@ -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() } } @@ -890,13 +891,12 @@ pub(crate) enum Callable { Extern(#[cfg_attr(feature = "serde_derive", serde(state))] GcPtr), } +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() } } diff --git a/vm/src/vm.rs b/vm/src/vm.rs index 422a933af7..f89f529cbb 100644 --- a/vm/src/vm.rs +++ b/vm/src/vm.rs @@ -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,