Clean up the VM

Cargo.toml

@@ -19,7 +19,7 @@ path = "src/main.rs"
 
 [workspace.dependencies]
 # This has to line up with the workspace version above
-steel-core = { path = "./crates/steel-core", version = "0.6.0", features = ["dylibs", "markdown", "stacker", "sync"] }
+steel-core = { path = "./crates/steel-core", version = "0.6.0", features = ["dylibs", "markdown", "stacker", "sync", "rooted-instructions"] }
 
 [features]
 default = ["mimalloc"]

crates/steel-core/Cargo.toml

@@ -27,7 +27,10 @@ serde = { version = "1.0.193", features = ["derive", "rc"] }
 serde_derive = "1.0.193"
 bincode = "1.3.3"
 pretty = "0.12.1"
-im-lists = "0.8.1"
+# im-lists = "0.8.1"
+
+im-lists = { git = "https://github.com/mattwparas/im-lists.git", branch = "mwp-node-iter" }
+
 strsim = "0.11.0"
 quickscope = "0.2.0"
 
@@ -83,6 +86,9 @@ compact_str = { version = "0.8.0", features = ["serde"] }
 
 git2 = { version = "0.19.0", optional = true, features = ["vendored-openssl"] }
 
+# For the constant map
+arc-swap = "1.7.1"
+
 [target.'cfg(target_arch = "wasm32")'.dependencies]
 getrandom = { version = "*", features = ["js"] }
 

crates/steel-core/src/compiler/compiler.rs

@@ -667,6 +667,9 @@ impl Compiler {
             results.push(instructions);
         }
 
+        // Push down the readable constant map
+        self.constant_map.flush();
+
         // This... cannot be efficient?
         // for idx in index_buffer {
         //     let extracted: Vec<Instruction> = instruction_buffer.drain(0..idx).collect();

crates/steel-core/src/compiler/constants.rs

@@ -9,7 +9,9 @@
 };
 
 use std::collections::HashMap;
+use std::sync::Arc;
 
+use arc_swap::ArcSwap;
 // TODO add the serializing and deserializing for constants
 use serde::{Deserialize, Serialize};
 use steel_parser::parser::{lower_entire_ast, SourceId};
@@ -20,6 +22,8 @@
 pub struct ConstantMap {
     map: SharedMut<HashMap<SteelVal, usize>>,
     values: SharedMut<Vec<SteelVal>>,
+    // TODO: Flush to these values after a compilation. - maybe have two of them to
+    reified_values: Arc<ArcSwap<Vec<SteelVal>>>,
 }
 
 #[derive(Serialize, Deserialize)]
@@ -36,6 +40,7 @@
         Self {
             values: Shared::clone(&self.values),
             map: Shared::clone(&self.map),
+            reified_values: Arc::clone(&self.reified_values),
         }
     }
 }
@@ -45,9 +50,16 @@
         ConstantMap {
             values: Shared::new(MutContainer::new(Vec::new())),
             map: Shared::new(MutContainer::new(HashMap::new())),
+            // Does this help at all?
+            reified_values: Arc::new(ArcSwap::from_pointee(Vec::new())),
         }
     }
 
+    pub fn flush(&self) {
+        let values = self.values.read().clone();
+        self.reified_values.store(Arc::new(values));
+    }
+
     pub fn deep_clone(&self) -> ConstantMap {
         Self {
             map: Shared::new(MutContainer::new(
@@ -60,6 +72,9 @@
             values: Shared::new(MutContainer::new(
                 self.values.read().iter().cloned().collect(),
             )),
+            reified_values: Arc::new(ArcSwap::from_pointee(
+                self.values.read().iter().cloned().collect(),
+            )),
         }
     }
 
@@ -82,7 +97,7 @@
     }
 
     pub fn from_vec(vec: Vec<SteelVal>) -> ConstantMap {
         ConstantMap {
             map: Shared::new(MutContainer::new(
                 vec.clone()
                     .into_iter()
@@ -91,6 +106,7 @@
                     .collect(),
             )),
             values: Shared::new(MutContainer::new(vec)),
+            reified_values: todo!(),
         }
     }
 
@@ -167,6 +183,10 @@
         self.values.read()[idx].clone()
     }
 
+    pub fn get_value(&self, idx: usize) -> SteelVal {
+        self.reified_values.load()[idx].clone()
+    }
+
     pub fn try_get(&self, idx: usize) -> Option<SteelVal> {
         self.values.read().get(idx).cloned()
     }

crates/steel-core/src/env.rs

@@ -1,5 +1,9 @@
+// #[cfg(feature = "sync")]
+// use parking_lot::{RwLock, RwLockReadGuard};
+
 #[cfg(feature = "sync")]
-use parking_lot::{RwLock, RwLockReadGuard};
+use std::sync::{RwLock, RwLockReadGuard};
+
 #[cfg(feature = "sync")]
 use std::sync::Arc;
 
@@ -16,19 +20,40 @@ pub struct Env {
     // there needs to be a lock on all globals since that way
     // things are relatively consistent.
     #[cfg(feature = "sync")]
+    // pub(crate) bindings_vec: Arc<RwLock<Vec<SteelVal>>>,
+
+    // TODO: This is NO GOOD! - This causes much contention when
+    // trying to read the variables. What we really want to do
+    // is rethink how globals are stored and accessed.
+    //
+    // Perhaps updates using `set!` are instead atomic w.r.t a thread,
+    // however would require using an atomic box in order to see
+    // an update that occurs on another thread? - In addition, we could
+    // push down defines to other threads, in order for them to see it.
+    //
+    // At a safepoint, we could "refresh" if dirty? It might be faster?
+    //
+    // That way we don't need to necessarily have _every_ thread constantly
+    // checking its own stuff.
+    //
+    // TODO: Just make set! and `define` make all threads come to a safepoint
+    // before continuing to work, and then apply the definition across the board.
+    //
+    // This will remove the need to use a RwLock at all, and we can get away with
+    // just pushing the changes across together.
     pub(crate) bindings_vec: Arc<RwLock<Vec<SteelVal>>>,
     // Keep a copy of the globals that we can access
     // just by offset.
-    // #[cfg(feature = "sync")]
-    // pub(crate) thread_local_bindings: Vec<SteelVal>,
+    #[cfg(feature = "sync")]
+    pub(crate) thread_local_bindings: Vec<SteelVal>,
 }
 
 #[cfg(feature = "sync")]
 impl Clone for Env {
     fn clone(&self) -> Self {
         Self {
             bindings_vec: self.bindings_vec.clone(),
-            // thread_local_bindings: self.thread_local_bindings.clone(),
+            thread_local_bindings: self.thread_local_bindings.clone(),
         }
     }
 }
@@ -123,27 +148,31 @@ impl Env {
 #[cfg(feature = "sync")]
 impl Env {
     pub fn extract(&self, idx: usize) -> Option<SteelVal> {
-        self.bindings_vec.read().get(idx).cloned()
+        self.bindings_vec.read().unwrap().get(idx).cloned()
     }
 
     pub fn len(&self) -> usize {
-        self.bindings_vec.read().len()
+        self.bindings_vec.read().unwrap().len()
     }
 
     /// top level global env has no parent
     pub fn root() -> Self {
         Env {
             bindings_vec: Arc::new(RwLock::new(Vec::with_capacity(1024))),
-            // thread_local_bindings: Vec::with_capacity(1024),
+            thread_local_bindings: Vec::with_capacity(1024),
         }
     }
 
     pub fn deep_clone(&self) -> Self {
+        let guard = self.bindings_vec.read().unwrap();
+        let bindings_vec = Arc::new(RwLock::new(guard.iter().map(|x| x.clone()).collect()));
+        // let thread_local_bindings = guard.iter().map(|x| x.clone()).collect();
+
+        let thread_local_bindings = self.thread_local_bindings.clone();
+
         Self {
-            bindings_vec: Arc::new(RwLock::new(
-                self.bindings_vec.read().iter().map(|x| x.clone()).collect(),
-            )),
-            // thread_local_bindings: self.thread_local_bindings.clone(),
+            bindings_vec,
+            thread_local_bindings,
         }
     }
 
@@ -161,8 +190,11 @@ impl Env {
 
     #[inline(always)]
     pub fn repl_lookup_idx(&self, idx: usize) -> SteelVal {
-        self.bindings_vec.read()[idx].clone()
-        // self.thread_local_bindings[idx].clone()
+        // TODO: Signal to the other threads to update their stuff?
+        // get them all to a safepoint? Is that worth it?
+
+        // self.bindings_vec.read().unwrap()[idx].clone()
+        self.thread_local_bindings[idx].clone()
     }
 
     // /// Get the value located at that index
@@ -172,36 +204,43 @@ impl Env {
 
     #[inline]
     pub fn repl_define_idx(&mut self, idx: usize, val: SteelVal) {
-        let mut guard = self.bindings_vec.write();
+        let mut guard = self.bindings_vec.write().unwrap();
 
-        if idx < guard.len() {
+        // println!("{} - {}", guard.len(), self.thread_local_bindings.len());
+
+        // if idx < guard.len() {
+        if idx < self.thread_local_bindings.len() {
             guard[idx] = val.clone();
-            // self.thread_local_bindings[idx] = val;
+            self.thread_local_bindings[idx] = val;
         } else {
-            if idx > guard.len() {
+            // if idx > guard.len() {
+            if idx > self.thread_local_bindings.len() {
                 // TODO: This seems suspect. Try to understand
                 // what is happening here. This would be that values
                 // are getting interned to be at a global offset in the
                 // wrong order, which seems to be fine in general,
                 // assuming that the values then get actually updated
                 // to the correct values.
-                for _ in 0..(idx - guard.len()) {
+                // for _ in 0..(idx - guard.len()) {
+                for _ in 0..(idx - self.thread_local_bindings.len()) {
                     guard.push(SteelVal::Void);
-                    // self.thread_local_bindings.push(SteelVal::Void);
+                    self.thread_local_bindings.push(SteelVal::Void);
                 }
             }
 
             guard.push(val.clone());
-            // self.thread_local_bindings.push(val);
-            assert_eq!(guard.len() - 1, idx);
+            self.thread_local_bindings.push(val);
+            // assert_eq!(guard.len() - 1, idx);
         }
+
+        // assert_eq!(self.thread_local_bindings.len(), guard.len())
     }
 
     pub fn repl_set_idx(&mut self, idx: usize, val: SteelVal) -> Result<SteelVal> {
-        let mut guard = self.bindings_vec.write();
+        let mut guard = self.bindings_vec.write().unwrap();
         let output = guard[idx].clone();
         guard[idx] = val.clone();
-        // self.thread_local_bindings[idx] = val;
+        self.thread_local_bindings[idx] = val;
         Ok(output)
     }
 
@@ -214,7 +253,7 @@ impl Env {
     // TODO: This needs to be fixed!
     #[cfg(feature = "sync")]
     pub fn roots(&self) -> RwLockReadGuard<'_, Vec<SteelVal>> {
-        self.bindings_vec.read()
+        self.bindings_vec.read().unwrap()
     }
 
     #[cfg(not(feature = "sync"))]

crates/steel-core/src/primitives/lists.rs

@@ -1,15 +1,9 @@
+use crate::rvals::{IntoSteelVal, Result, SteelVal};
 use crate::{
     gc::Gc,
-    steel_vm::{
-        builtin::BuiltInModule,
-        vm::{VmContext, APPLY_DEFINITION},
-    },
+    steel_vm::{builtin::BuiltInModule, vm::APPLY_DEFINITION},
     values::lists::Pair,
 };
-use crate::{
-    rvals::{IntoSteelVal, Result, SteelVal},
-    steel_vm::vm::VmCore,
-};
 use crate::{stop, throw};
 
 use crate::values::lists::List;
@@ -96,11 +90,21 @@ pub fn list_module() -> BuiltInModule {
         .register_native_fn_definition(PLIST_TRY_GET_POSITIONAL_DEFINITION)
         .register_native_fn_definition(PLIST_GET_POSITIONAL_LIST_DEFINITION)
         .register_native_fn_definition(PLIST_VALIDATE_ARGS_DEFINITION)
-        .register_native_fn_definition(DROP_START_DEFINITION);
+        .register_native_fn_definition(DROP_START_DEFINITION)
+        .register_native_fn_definition(CHUNKS_DEFINITION);
 
     module
 }
 
+#[steel_derive::function(name = "list-chunks", constant = true)]
+pub fn chunks(list: &List<SteelVal>) -> Result<SteelVal> {
+    let nodes = list.nodes();
+
+    Ok(SteelVal::ListV(
+        nodes.into_iter().map(|x| SteelVal::ListV(x)).collect(),
+    ))
+}
+
 /// Get the second element of the list. Raises an error if the list does not have an element in the second position.
 ///
 /// (second l) -> any/c
@@ -143,32 +147,6 @@ pub(crate) fn third(list: &List<SteelVal>) -> Result<SteelVal> {
     list.get(2).cloned().ok_or_else(throw!(Generic => "third: Index out of bounds - list did not have an element in the second position: {:?}", list))
 }
 
-fn _test_map(ctx: &mut VmCore, args: &[SteelVal]) -> Result<SteelVal> {
-    arity_check!(test_map, args, 2);
-
-    let mut arg_iter = args.iter();
-    let arg1 = arg_iter.next().unwrap();
-    let arg2 = arg_iter.next().unwrap();
-
-    if let SteelVal::ListV(l) = arg2 {
-        if arg1.is_function() {
-            // unimplemented!()
-
-            Ok(SteelVal::ListV(
-                l.into_iter()
-                    .map(|x| ctx.call_function_one_arg(arg1, x.clone()))
-                    .collect::<Result<_>>()?,
-            ))
-
-            // ctx.call_function_one_arg_or_else(function, arg)
-        } else {
-            stop!(TypeMismatch => "test-map expected a function")
-        }
-    } else {
-        stop!(TypeMismatch => "test-map expects a list")
-    }
-}
-
 #[steel_derive::function(name = "list-tail")]
 pub fn list_tail(list_or_pair: &SteelVal, pos: usize) -> Result<SteelVal> {
     match list_or_pair {