Yielding to the collector

The Vm now has a setting for how many steps to count per each budget charge, and each budget charge is when we also yield to the garbage collector. This yielding slowed execution down. One area where extra collection was happening that didn't need to was how the registered code was being stored as a weak reference. By upgrading it to a root, we can avoid constantly dropping the root count to 0 when calling Function::call(). Roots dropping to 0 will notify the collector to run, which is the overhead we really are avoiding. This ended up being a net swap for performance in the fibonacci vm example.
khonsulabs · Mar 27, 2024 · 41ec5a7 · 41ec5a7
1 parent 6a1a533
commit 41ec5a7
Show file tree

Hide file tree

Showing 10 changed files with 131 additions and 97 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -38,6 +38,7 @@ serde = { version = "1.0.195", features = ["derive", "rc"] }
 unicode-ident = "1.0.12"
 refuse = { git = "https://github.com/khonsulabs/refuse" }
 refuse-pool = { git = "https://github.com/khonsulabs/refuse" }
+parking_lot = "0.12.1"
 
 [dev-dependencies]
 rsn = "0.1"

diff --git a/src/list.rs b/src/list.rs
@@ -1,5 +1,4 @@
-use std::sync::Mutex;
-
+use parking_lot::Mutex;
 use refuse::Trace;
 
 use crate::symbol::Symbol;
@@ -28,7 +27,7 @@ pub static LIST_TYPE: RustType<List> = RustType::new("List", |t| {
                             Symbol::len_symbol(),
                             0,
                             |_vm: &mut VmContext<'_, '_>, this: &Rooted<List>| {
-                                Value::try_from(this.0.lock().expect("poisoned").len())
+                                Value::try_from(this.0.lock().len())
                             },
                         )
                         .with_fn(Symbol::set_symbol(), 2, |vm, this| {
@@ -64,7 +63,7 @@ impl List {
         let Some(index) = index.as_usize() else {
             return Err(Fault::OutOfBounds);
         };
-        let contents = self.0.lock().expect("poisoned");
+        let contents = self.0.lock();
 
         contents.get(index).copied().ok_or(Fault::OutOfBounds)
     }
@@ -73,13 +72,13 @@ impl List {
         let Some(index) = index.as_usize() else {
             return Err(Fault::OutOfBounds);
         };
-        let mut contents = self.0.lock().expect("poisoned");
+        let mut contents = self.0.lock();
         contents.insert(index, value);
         Ok(())
     }
 
     pub fn push(&self, value: Value) -> Result<(), Fault> {
-        let mut contents = self.0.lock().expect("poisoned");
+        let mut contents = self.0.lock();
         contents.push(value);
         Ok(())
     }
@@ -88,7 +87,7 @@ impl List {
         let Some(index) = index.as_usize() else {
             return Err(Fault::OutOfBounds);
         };
-        let mut contents = self.0.lock().expect("poisoned");
+        let mut contents = self.0.lock();
 
         if let Some(entry) = contents.get_mut(index) {
             Ok(Some(std::mem::replace(entry, value)))
@@ -98,7 +97,7 @@ impl List {
     }
 
     pub fn to_vec(&self) -> Vec<Value> {
-        self.0.lock().expect("poisoned").clone()
+        self.0.lock().clone()
     }
 }
 
@@ -124,6 +123,6 @@ impl Trace for List {
     const MAY_CONTAIN_REFERENCES: bool = true;
 
     fn trace(&self, tracer: &mut refuse::Tracer) {
-        self.0.lock().expect("poisoned").trace(tracer);
+        self.0.lock().trace(tracer);
     }
 }
diff --git a/src/map.rs b/src/map.rs
@@ -1,6 +1,6 @@
 use std::cmp::Ordering;
-use std::sync::Mutex;
 
+use parking_lot::Mutex;
 use refuse::{CollectionGuard, Trace};
 
 use crate::list::List;
@@ -50,7 +50,7 @@ pub static MAP_TYPE: RustType<Map> = RustType::new("Map", |t| {
                             this.nth(&index, vm.as_ref())
                         })
                         .with_fn(Symbol::len_symbol(), 0, |_vm, this| {
-                            let contents = this.0.lock().expect("poisoned");
+                            let contents = this.0.lock();
                             Value::try_from(contents.len())
                         })
                 });
@@ -63,7 +63,7 @@ impl Trace for Map {
     const MAY_CONTAIN_REFERENCES: bool = true;
 
     fn trace(&self, tracer: &mut refuse::Tracer) {
-        for field in &*self.0.lock().expect("poisoned") {
+        for field in &*self.0.lock() {
             field.key.value.trace(tracer);
             field.value.trace(tracer);
         }
@@ -93,7 +93,7 @@ impl Map {
 
     pub fn get(&self, vm: &mut VmContext<'_, '_>, key: &Value) -> Result<Option<Value>, Fault> {
         let hash = key.hash(vm);
-        let contents = self.0.lock().expect("poisoned");
+        let contents = self.0.lock();
         for field in &*contents {
             match hash.cmp(&field.key.hash) {
                 Ordering::Less => continue,
@@ -113,7 +113,7 @@ impl Map {
         let Some(index) = index.as_usize() else {
             return Err(Fault::OutOfBounds);
         };
-        let contents = self.0.lock().expect("poisoned");
+        let contents = self.0.lock();
         contents
             .get(index)
             .map(|field| Value::dynamic(List::from_iter([field.key.value, field.value]), guard))
@@ -127,7 +127,7 @@ impl Map {
         value: Value,
     ) -> Result<Option<Value>, Fault> {
         let key = MapKey::new(vm, key);
-        let mut contents = self.0.lock().expect("poisoned");
+        let mut contents = self.0.lock();
         let mut insert_at = contents.len();
         for (index, field) in contents.iter_mut().enumerate() {
             match key.hash.cmp(&field.key.hash) {
@@ -153,7 +153,7 @@ impl Map {
     }
 
     pub fn to_vec(&self) -> Vec<Field> {
-        self.0.lock().expect("poisoned").clone()
+        self.0.lock().clone()
     }
 }
 

diff --git a/src/string.rs b/src/string.rs
@@ -1,7 +1,7 @@
 use std::fmt::Display;
 use std::hash::Hash;
-use std::sync::{Mutex, MutexGuard};
 
+use parking_lot::{Mutex, MutexGuard};
 use refuse::ContainsNoRefs;
 
 use crate::list::List;
@@ -17,7 +17,7 @@ pub struct MuseString(Mutex<String>);
 
 impl MuseString {
     pub(crate) fn lock(&self) -> MutexGuard<'_, String> {
-        self.0.lock().expect("poisoned")
+        self.0.lock()
     }
 }
 
@@ -75,15 +75,15 @@ pub static STRING_TYPE: RustType<MuseString> = RustType::new("String", |t| {
     })
     .with_hash(|_| {
         |this, _vm, hasher| {
-            this.0.lock().expect("poisoned").hash(hasher);
+            this.0.lock().hash(hasher);
         }
     })
     .with_eq(|_| {
         |this, vm, rhs| {
             if let Some(rhs) = rhs.as_downcast_ref::<MuseString>(vm.as_ref()) {
-                Ok(*this.0.lock().expect("poisoned") == *rhs.0.lock().expect("poisoned"))
+                Ok(*this.0.lock() == *rhs.0.lock())
             } else if let Some(rhs) = rhs.as_symbol(vm.as_ref()) {
-                Ok(*this.0.lock().expect("poisoned") == *rhs)
+                Ok(*this.0.lock() == *rhs)
             } else {
                 Ok(false)
             }
@@ -92,11 +92,7 @@ pub static STRING_TYPE: RustType<MuseString> = RustType::new("String", |t| {
     .with_total_cmp(|_| {
         |this, vm, rhs| {
             if let Some(rhs) = rhs.as_downcast_ref::<MuseString>(vm.as_ref()) {
-                Ok(this
-                    .0
-                    .lock()
-                    .expect("poisoned")
-                    .cmp(&rhs.0.lock().expect("poisoned")))
+                Ok(this.0.lock().cmp(&rhs.0.lock()))
             } else if rhs.as_any_dynamic().is_none() {
                 // Dynamics sort after primitive values
                 Ok(std::cmp::Ordering::Greater)
@@ -114,7 +110,7 @@ pub static STRING_TYPE: RustType<MuseString> = RustType::new("String", |t| {
                             Symbol::len_symbol(),
                             0,
                             |_vm: &mut VmContext<'_, '_>, this: &Rooted<MuseString>| {
-                                Value::try_from(this.0.lock().expect("poisoned").len())
+                                Value::try_from(this.0.lock().len())
                             },
                         )
                         .with_fn(
@@ -169,7 +165,7 @@ pub static STRING_TYPE: RustType<MuseString> = RustType::new("String", |t| {
     })
     .with_add(|_| {
         |this, vm, rhs| {
-            let lhs = this.0.lock().expect("poisoned");
+            let lhs = this.0.lock();
             if let Some(rhs) = rhs.as_downcast_ref::<MuseString>(vm.as_ref()) {
                 if std::ptr::eq(&this.0, &rhs.0) {
                     let mut repeated =
@@ -178,7 +174,7 @@ pub static STRING_TYPE: RustType<MuseString> = RustType::new("String", |t| {
                     repeated.push_str(&lhs);
                     Ok(Value::dynamic(MuseString::from(repeated), vm))
                 } else {
-                    let rhs = rhs.0.lock().expect("poisoned");
+                    let rhs = rhs.0.lock();
                     let mut combined = String::with_capacity(lhs.len() + rhs.len());
                     combined.push_str(&lhs);
                     combined.push_str(&rhs);
@@ -196,7 +192,7 @@ pub static STRING_TYPE: RustType<MuseString> = RustType::new("String", |t| {
     .with_add_right(|_| {
         |this, vm, lhs| {
             let lhs = lhs.to_string(vm)?.try_upgrade(vm.guard())?;
-            let rhs = this.0.lock().expect("poisoned");
+            let rhs = this.0.lock();
             let mut combined = String::with_capacity(rhs.len() + lhs.len());
             combined.push_str(&lhs);
             combined.push_str(&rhs);
@@ -208,7 +204,7 @@ pub static STRING_TYPE: RustType<MuseString> = RustType::new("String", |t| {
             let Some(times) = rhs.as_usize() else {
                 return Err(Fault::ExpectedInteger);
             };
-            let this = this.0.lock().expect("poisoned");
+            let this = this.0.lock();
 
             Ok(Value::dynamic(MuseString::from(this.repeat(times)), vm))
         }
@@ -218,17 +214,17 @@ pub static STRING_TYPE: RustType<MuseString> = RustType::new("String", |t| {
             let Some(times) = lhs.as_usize() else {
                 return Err(Fault::ExpectedInteger);
             };
-            let this = this.0.lock().expect("poisoned");
+            let this = this.0.lock();
 
             Ok(Value::dynamic(MuseString::from(this.repeat(times)), vm))
         }
     })
-    .with_truthy(|_| |this, _vm| !this.0.lock().expect("poisoned").is_empty())
-    .with_to_string(|_| |this, _vm| Ok(SymbolRef::from(&*this.0.lock().expect("poisoned"))))
+    .with_truthy(|_| |this, _vm| !this.0.lock().is_empty())
+    .with_to_string(|_| |this, _vm| Ok(SymbolRef::from(&*this.0.lock())))
     .with_deep_clone(|_| {
         |this, guard| {
             Some(AnyDynamic::new(
-                MuseString(Mutex::new(this.0.lock().expect("poisoned").clone())),
+                MuseString(Mutex::new(this.0.lock().clone())),
                 guard,
             ))
         }
@@ -244,6 +240,6 @@ impl CustomType for MuseString {
 
 impl Display for MuseString {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.0.lock().expect("poisioned").fmt(f)
+        self.0.lock().fmt(f)
     }
 }
diff --git a/src/symbol.rs b/src/symbol.rs
@@ -36,7 +36,7 @@ impl SymbolRef {
 
 impl kempt::Sort<SymbolRef> for Symbol {
     fn compare(&self, other: &SymbolRef) -> std::cmp::Ordering {
-        self.0.as_any().cmp(&other.0.as_any())
+        self.0.downgrade_any().cmp(&other.0.as_any())
     }
 }
 

diff --git a/src/tests.rs b/src/tests.rs
@@ -21,8 +21,9 @@ fn budgeting() {
         code.copy(value, Register(0));
     }
     let code = code.to_code(&guard);
-    let mut vm = Vm::new(&guard);
+    let vm = Vm::new(&guard);
     // Turn on budgeting, but don't give any budget.
+    vm.set_steps_per_charge(1);
     vm.increase_budget(0);
     assert_eq!(
         vm.execute(&code, &mut guard).unwrap_err(),
@@ -59,8 +60,9 @@ fn module_budgeting() {
         &guard,
     )
     .unwrap();
-    let mut vm = Vm::new(&guard);
+    let vm = Vm::new(&guard);
     // Turn on budgeting, but don't give any budget.
+    vm.set_steps_per_charge(1);
     vm.increase_budget(0);
     assert_eq!(
         vm.execute(&code, &mut guard).unwrap_err(),
@@ -97,7 +99,7 @@ fn invoke() {
         &guard,
     )
     .unwrap();
-    let mut vm = Vm::new(&guard);
+    let vm = Vm::new(&guard);
     vm.execute(&code, &mut guard).unwrap();
 
     let Value::Int(result) = vm