Skip to content

Commit

Permalink
Keep entity lists alive during iteration
Browse files Browse the repository at this point in the history
Entity lists are now copy-on-write. This lets the interpreter keep
iterators pointing into them while (potentially) mutating them, and
preserves GML `with` semantics in the presence of such mutation.

In particular, this makes `instance_create` and `instance_destroy` memory
safe when called from GML, as described by #40.
  • Loading branch information
rpjohnst committed May 23, 2020
1 parent 89294b1 commit 104da50
Show file tree
Hide file tree
Showing 12 changed files with 355 additions and 82 deletions.
1 change: 1 addition & 0 deletions gml/src/back/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ impl From<ssa::Opcode> for code::Op {
ssa::Opcode::Return => code::Op::Ret,

ssa::Opcode::With => code::Op::With,
ssa::Opcode::ReleaseWith => code::Op::ReleaseWith,
ssa::Opcode::ScopeError => code::Op::ScopeError,
ssa::Opcode::LoadPointer => code::Op::LoadPointer,
ssa::Opcode::NextPointer => code::Op::NextPointer,
Expand Down
8 changes: 8 additions & 0 deletions gml/src/back/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ pub enum Instruction {
/// instructions.
Parameter,

Nullary { op: Opcode },
Unary { op: Opcode, arg: Value },
UnaryReal { op: Opcode, real: f64 },
UnarySymbol { op: Opcode, symbol: Symbol },
Expand Down Expand Up @@ -101,6 +102,8 @@ pub enum Opcode {

/// Build an iterator over a scope, producing a tuple of start and end.
With,
/// Release the iterator created by the most recent `With`.
ReleaseWith,
/// Error that a scope does not exist.
ScopeError,
LoadPointer,
Expand Down Expand Up @@ -264,6 +267,7 @@ impl Function {
match self.values[value] {
Alias { .. } | Project { .. } | Parameter => panic!("finding op of non-instruction"),

Nullary { op, .. } |
Unary { op, .. } |
UnaryReal { op, .. } |
UnarySymbol { op, .. } |
Expand All @@ -286,6 +290,7 @@ impl Function {
Alias { .. } | Project { .. } | Parameter => panic!("finding defs of non-instruction"),

// Zero-valued instructions:
Nullary { op: Opcode::ReleaseWith } |
Unary { op: Opcode::Release, .. } |
Unary { op: Opcode::Return, .. } |
Unary { op: Opcode::ScopeError, .. } |
Expand All @@ -302,6 +307,7 @@ impl Function {
Unary { op: Opcode::With, .. } => ValueRange { range: start + 1..start + 3 },

// The common case: single-valued instructions:
Nullary { .. } |
Unary { .. } |
UnaryReal { .. } |
UnarySymbol { .. } |
Expand All @@ -328,6 +334,7 @@ impl Function {
match self.values[value] {
Alias { .. } | Project { .. } | Parameter => panic!("finding uses of non-instruction"),

Nullary { .. } => &[],
Unary { ref arg, .. } => slice::from_ref(arg),
UnaryReal { .. } => &[],
UnarySymbol { .. } => &[],
Expand All @@ -350,6 +357,7 @@ impl Function {
match self.values[value] {
Alias { .. } | Project { .. } | Parameter => panic!("finding uses of non-instruction"),

Nullary { .. } => &mut [],
Unary { ref mut arg, .. } => slice::from_mut(arg),
UnaryReal { .. } => &mut [],
UnarySymbol { .. } => &mut [],
Expand Down
8 changes: 8 additions & 0 deletions gml/src/front/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,9 @@ impl<'p, 'e> Codegen<'p, 'e> {
let exists = self.emit_unary(ssa::Opcode::ExistsEntity, entity, location);
self.emit_branch(exists, body_block, cond_block, location);

self.current_block = exit_block;
self.emit_nullary(ssa::Opcode::ReleaseWith, location);

With { cond_block, body_block, exit_block, entity }
}

Expand Down Expand Up @@ -1251,6 +1254,11 @@ impl<'p, 'e> Codegen<'p, 'e> {
self.emit_unary_symbol(ssa::Opcode::Constant, string, location)
}

fn emit_nullary(&mut self, op: ssa::Opcode, location: usize) -> ssa::Value {
let instruction = ssa::Instruction::Nullary { op };
self.function.emit_instruction(self.current_block, instruction, location)
}

fn emit_unary(&mut self, op: ssa::Opcode, arg: ssa::Value, location: usize) -> ssa::Value {
let instruction = ssa::Instruction::Unary { op, arg };
self.function.emit_instruction(self.current_block, instruction, location)
Expand Down
57 changes: 0 additions & 57 deletions gml/src/index_map.rs

This file was deleted.

3 changes: 2 additions & 1 deletion gml/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![feature(maybe_uninit_extra)]
#![feature(box_patterns)]
#![feature(box_syntax)]
#![feature(extern_types)]
Expand All @@ -18,7 +19,7 @@ pub use gml_meta::bind;

#[macro_use]
mod handle_map;
mod index_map;
mod rc_vec;
mod bit_vec;
pub mod symbol;

Expand Down
164 changes: 164 additions & 0 deletions gml/src/rc_vec.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
use std::{ops, cmp, iter, mem, ptr, slice};
use std::marker::PhantomData;
use std::mem::MaybeUninit;
use std::rc::Rc;

/// A copy-on-write `Vec`.
///
/// Invoking `clone` on an `RcVec` produces a new `RcVec` using the same array.
///
/// Attempts to mutate an `RcVec` will first ensure it is unique, invoking `clone` on its elements
/// if there are other `RcVec` objects sharing its array.
pub struct RcVec<T> {
buf: Rc<[MaybeUninit<T>]>,
len: usize,

_marker: PhantomData<[T]>,
}

impl<T> Default for RcVec<T> {
/// Create a new, empty, unshared `RcVec`.
fn default() -> Self {
RcVec { buf: Rc::new([]), len: 0, _marker: PhantomData }
}
}

impl<T> Clone for RcVec<T> {
/// Create a new `RcVec` with the same array, increasing its reference count.
fn clone(&self) -> RcVec<T> {
RcVec { buf: Rc::clone(&self.buf), len: self.len, _marker: PhantomData }
}
}

impl<T> Drop for RcVec<T> {
fn drop(&mut self) {
// If this is the last reference to the array, drop its elements.
if Rc::strong_count(&self.buf) == 1 {
unsafe {
let buf = slice::from_raw_parts_mut(self.as_mut_ptr(), self.len);
ptr::drop_in_place(buf);
}
}
}
}

impl<T> ops::Deref for RcVec<T> {
type Target = [T];

fn deref(&self) -> &[T] {
// Safety: `self.len` represents the number of initialized elements.
unsafe { slice::from_raw_parts(self.as_ptr(), self.len) }
}
}

impl<T> RcVec<T> {
/// Construct a new `RcVec` with elements initialized to those of `iter` and capacity `cap`.
///
/// If `cap` is less than the number of elements yielded by `iter`, the rest are discarded.
pub fn from_iter_with_capacity<I>(iter: I, cap: usize) -> Self where
I: IntoIterator<Item = T>
{
let mut len = 0;
let buf = iter.into_iter()
.inspect(|_| len += 1)
.map(MaybeUninit::new)
.chain(iter::repeat_with(MaybeUninit::uninit))
.take(cap)
.collect();

// Safety: The soundness of `Deref` depends on the value of `len` here.
RcVec { buf, len, _marker: PhantomData }
}

pub fn len(&self) -> usize { self.len }

pub fn as_ptr(&self) -> *const T {
// Safety: This immediately "forgets" the duplicate `Rc`.
let ptr = unsafe { Rc::into_raw(ptr::read(&self.buf)) };
ptr as *const T
}

pub fn as_mut_ptr(&mut self) -> *mut T {
// Safety: This immediately "forgets" the duplicate `Rc`.
let ptr = unsafe { Rc::into_raw(ptr::read(&self.buf)) };
ptr as *mut T
}
}

impl<T> RcVec<T> where T: Clone {
/// Reserve capacity for at least `extra` more elements to be inserted.
///
/// If the array must be reallocated and there are other `RcVec`s using the same allocation,
/// existing elements will be cloned.
pub fn reserve(&mut self, extra: usize) {
if self.buf.len().wrapping_sub(self.len) >= extra {
return;
}

// Mimic `Vec`'s behavior: limiting `size` to `isize::MAX` ensures that subsequent
// evaluations of `2 * self.buf.len()` cannot overflow.
let buf_len = self.len.checked_add(extra).expect("capacity overflow");
let buf_len = cmp::max(buf_len, 2 * self.buf.len());
let size = buf_len.checked_mul(mem::size_of::<T>()).expect("capacity overflow");
if mem::size_of::<usize>() < 8 && size > isize::MAX as usize {
panic!("capacity overflow");
}

// If this is the only reference to the array, move its elements. Otherwise, clone them.
if Rc::strong_count(&self.buf) == 1 {
// Safety: The moved from values are immediately freed without being dropped.
let iter = self.buf.iter().take(self.len).map(|value| unsafe { value.read() });
*self = Self::from_iter_with_capacity(iter, buf_len);
} else {
*self = Self::from_iter_with_capacity(self.iter().cloned(), buf_len);
}
}

/// Make a mutable reference into the array.
///
/// If there are other `RcVec`s sharing the array, `clone` the elements to ensure uniqueness.
pub fn make_mut(&mut self) -> &mut [T] {
// If this is not the only reference to the array, clone it.
if Rc::strong_count(&self.buf) != 1 {
*self = Self::from_iter_with_capacity(self.iter().cloned(), self.buf.len());
}

// Safety: The strong count of `self.buf` is now 1.
// `self.len` represents the number of initialized elements.
unsafe { slice::from_raw_parts_mut(self.as_mut_ptr(), self.len) }
}

/// Append `value` to the end of the array.
pub fn push(&mut self, value: T) {
if self.len == self.buf.len() {
self.reserve(1);
}

// Safety: If there was excess capacity, then no references have been formed to that memory.
// Otherwise, the strong count of `self.buf` is now 1 and there are no references at all.
unsafe {
let end = self.as_mut_ptr().add(self.len);
ptr::write(end, value);
self.len += 1;
}
}

/// Remove and return the element at position `index`, shifting later elements down.
pub fn remove(&mut self, index: usize) -> T {
let len = self.len();
assert!(index < len);

let ret;
self.make_mut();

// Safety: The strong count of `self.buf` is now 1, and `index` is in bounds.
unsafe {
let ptr = self.as_mut_ptr().add(index);
ret = ptr::read(ptr);
ptr::copy(ptr.offset(1), ptr, len - index - 1);
}

self.len -= 1;
ret
}
}
1 change: 1 addition & 0 deletions gml/src/vm/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ pub enum Op {
StoreScope,

With,
ReleaseWith,
LoadPointer,
NextPointer,
NePointer,
Expand Down
62 changes: 62 additions & 0 deletions gml/src/vm/instance_map.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
use std::mem;
use std::collections::HashMap;
use std::collections::hash_map::Entry;
use std::ops::Index;
use std::hash::Hash;

use crate::rc_vec::RcVec;

/// A hash map that preserves insertion order in a copy-on-write array of values.
///
/// This is designed to map instance IDs to entity handles. Using a copy-on-write `RcVec` for
/// the entity handles allows `with` loops to iterate over them without being invalidated by
/// reallocations or mutations.
pub struct InstanceMap<K, V> where K: Eq + Hash {
keys: HashMap<K, usize>,
values: RcVec<V>,
}

impl<K, V> Default for InstanceMap<K, V> where K: Eq + Hash {
fn default() -> Self {
InstanceMap {
keys: HashMap::default(),
values: RcVec::default(),
}
}
}

impl<K, V> InstanceMap<K, V> where K: Eq + Hash {
pub fn values(&self) -> &RcVec<V> { &self.values }

pub fn len(&self) -> usize { self.values.len() }

pub fn is_empty(&self) -> bool { self.len() == 0 }

pub fn contains_key(&self, key: K) -> bool { self.keys.contains_key(&key) }
}

impl<K, V> InstanceMap<K, V> where K: Eq + Hash, V: Clone {
pub fn insert(&mut self, key: K, value: V) -> Option<V> {
match self.keys.entry(key) {
Entry::Occupied(entry) => {
let values = self.values.make_mut();
let old = mem::replace(&mut values[*entry.get()], value);
Some(old)
}

Entry::Vacant(entry) => {
entry.insert(self.values.len());
self.values.push(value);
None
}
}
}
}

impl<K, V> Index<K> for InstanceMap<K, V> where K: Eq + Hash {
type Output = V;

fn index(&self, key: K) -> &V {
&self.values[self.keys[&key]]
}
}
Loading

0 comments on commit 104da50

Please sign in to comment.