wip

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/scheme/modules/parameters.scm

@@ -148,10 +148,16 @@
   (lambda (x y)
     (let ([lx (length x)]
           [ly (length y)])
-      (let loop ([x (if (> lx ly) (list-tail x (- lx ly)) x)]
-                 [y (if (> ly lx) (list-tail y (- ly lx)) y)])
-
-        (if (equal? x y) x (loop (cdr x) (cdr y)))))))
+      (let loop ([x (if (> lx ly)
+                        (list-tail x (- lx ly))
+                        x)]
+                 [y (if (> ly lx)
+                        (list-tail y (- ly lx))
+                        y)])
+
+        (if (equal? x y)
+            x
+            (loop (cdr x) (cdr y)))))))
 
 (define do-wind
   (lambda (new)

crates/steel-core/src/scheme/stdlib.scm

@@ -181,7 +181,9 @@
 
     ;; Internal, we don't do anything special
     [(quasisyntax #%internal-crunch x)
-     (if (empty? 'x) (#%syntax/raw '() '() (#%syntax-span x)) (#%syntax/raw 'x 'x (#%syntax-span x)))]
+     (if (empty? 'x)
+         (#%syntax/raw '() '() (#%syntax-span x))
+         (#%syntax/raw 'x 'x (#%syntax-span x)))]
 
     [(quasisyntax (x xs ...))
      (syntax (#%syntax/raw (quote (x xs ...))
@@ -526,7 +528,9 @@
 ; (define compose (lambda (f g) (lambda (arg) (f (g arg)))))
 
 (define (foldl func accum lst)
-  (if (null? lst) accum (foldl func (func (car lst) accum) (cdr lst))))
+  (if (null? lst)
+      accum
+      (foldl func (func (car lst) accum) (cdr lst))))
 
 (define (map func lst . lsts)
 
@@ -550,11 +554,16 @@
 ;     (transduce lst (mapping func) (into-list))))
 
 (define foldr
-  (lambda (func accum lst) (if (null? lst) accum (func (car lst) (foldr func accum (cdr lst))))))
+  (lambda (func accum lst)
+    (if (null? lst)
+        accum
+        (func (car lst) (foldr func accum (cdr lst))))))
 
 (define unfold
   (lambda (func init pred)
-    (if (pred init) (cons init '()) (cons init (unfold func (func init) pred)))))
+    (if (pred init)
+        (cons init '())
+        (cons init (unfold func (func init) pred)))))
 
 (define fold (lambda (f a l) (foldl f a l)))
 (define reduce (lambda (f a l) (fold f a l)))
@@ -594,13 +603,25 @@
     [else (contains? pred? (cdr lst))]))
 
 (define (assoc thing alist)
-  (if (null? alist) #f (if (equal? (car (car alist)) thing) (car alist) (assoc thing (cdr alist)))))
+  (if (null? alist)
+      #f
+      (if (equal? (car (car alist)) thing)
+          (car alist)
+          (assoc thing (cdr alist)))))
 
 (define (assq thing alist)
-  (if (null? alist) #f (if (eq? (car (car alist)) thing) (car alist) (assq thing (cdr alist)))))
+  (if (null? alist)
+      #f
+      (if (eq? (car (car alist)) thing)
+          (car alist)
+          (assq thing (cdr alist)))))
 
 (define (assv thing alist)
-  (if (null? alist) #f (if (eq? (car (car alist)) thing) (car alist) (assv thing (cdr alist)))))
+  (if (null? alist)
+      #f
+      (if (eq? (car (car alist)) thing)
+          (car alist)
+          (assv thing (cdr alist)))))
 
 ;;@doc
 ;; Returns new list, keeping elements from `lst` which applying `pred` to the element
@@ -613,7 +634,9 @@
 ;; (filter even? (range 0 5)) ;; '(0 2 4)
 ;; ```
 (define (filter pred lst)
-  (if (empty? lst) '() (transduce lst (filtering pred) (into-list))))
+  (if (empty? lst)
+      '()
+      (transduce lst (filtering pred) (into-list))))
 
 ; (define (fact n)
 ;   (define factorial-tail (lambda (n acc)
@@ -622,8 +645,16 @@
 ;                                (factorial-tail (- n 1)  (* acc n )))))
 ;   (factorial-tail n 1))
 
-(define even-rec? (lambda (x) (if (= x 0) #t (odd-rec? (- x 1)))))
-(define odd-rec? (lambda (x) (if (= x 0) #f (even-rec? (- x 1)))))
+(define even-rec?
+  (lambda (x)
+    (if (= x 0)
+        #t
+        (odd-rec? (- x 1)))))
+(define odd-rec?
+  (lambda (x)
+    (if (= x 0)
+        #f
+        (even-rec? (- x 1)))))
 
 (define sum (lambda (x) (reduce + 0 x)))
 ;; (define head car)
@@ -645,7 +676,9 @@
 
 (define (drop lst n)
   (define (loop x l)
-    (if (zero? x) l (loop (sub1 x) (cdr l))))
+    (if (zero? x)
+        l
+        (loop (sub1 x) (cdr l))))
   (loop n lst))
 
 (define (slice l offset n)
@@ -663,7 +696,9 @@
     [else (gcd b (modulo a b))]))
 
 (define (lcm a b)
-  (if (or (zero? a) (zero? b)) 0 (abs (* b (floor (/ a (gcd a b)))))))
+  (if (or (zero? a) (zero? b))
+      0
+      (abs (* b (floor (/ a (gcd a b)))))))
 
 (define (for-each func lst)
   (if (null? lst)

crates/steel-core/src/steel_vm/engine.rs

@@ -1696,6 +1696,7 @@ impl Engine {
                 .global_env
                 .bindings_vec
                 .write()
+                .unwrap()
                 .truncate(checkpoint.globals_offset);
         }
 
@@ -1727,7 +1728,12 @@ impl Engine {
             #[cfg(feature = "sync")]
             {
                 GlobalSlotRecycler::free_shadowed_rooted_values(
-                    &mut self.virtual_machine.global_env.bindings_vec.write(),
+                    &mut self
+                        .virtual_machine
+                        .global_env
+                        .bindings_vec
+                        .write()
+                        .unwrap(),
                     &mut self.virtual_machine.compiler.write().symbol_map,
                     &mut self.virtual_machine.heap.lock().unwrap(),
                 );