⚡ Bolt: optimize Context map lookups and lock contention

.jules/bolt.md

@@ -0,0 +1,3 @@
+## 2024-05-24 - Context Map Lookups and Locking Contention
+**Learning:** Double lookups inside `MutexGuard` sections combined with unnecessary entire Enum cloning create notable lock contention and memory allocation overhead. Specifically, `binding.get(name).is_none()` followed by `binding.get(name).unwrap()` traverses the map twice unnecessarily.
+**Action:** Always perform a single `.get(name)` map lookup, and match on the borrowed enum reference to clone ONLY the internal value (like an `Arc` or `Value`), bypassing full `ContextValue` copying. Drop the `MutexGuard` explicitly BEFORE invoking external closures/functions (like `func(Vec::new())`) to reduce contention and prevent deadlocks on reentrant code.

benches/display_expression.rs

@@ -1,4 +1,4 @@
-use criterion::{criterion_group, criterion_main, Criterion, black_box};
+use criterion::{black_box, criterion_group, criterion_main, Criterion};
 use expression_engine::{parse_expression, ExprAST};
 
 fn bench_display_expression(c: &mut Criterion) {

src/context.rs

@@ -31,15 +31,17 @@ impl Context {
     }
 
     pub fn get_func(&self, name: &str) -> Option<Arc<InnerFunction>> {
-        let value = self.get(name)?;
+        let binding = self.0.lock().unwrap();
+        let value = binding.get(name)?;
         match value {
             ContextValue::Function(func) => Some(func.clone()),
             ContextValue::Variable(_) => None,
         }
     }
 
     pub fn get_variable(&self, name: &str) -> Option<Value> {
-        let value = self.get(name)?;
+        let binding = self.0.lock().unwrap();
+        let value = binding.get(name)?;
         match value {
             ContextValue::Variable(v) => Some(v.clone()),
             ContextValue::Function(_) => None,
@@ -53,15 +55,15 @@ impl Context {
     }
 
     pub fn value(&self, name: &str) -> Result<Value> {
-        let binding = self.0.lock().unwrap();
-        if binding.get(name).is_none() {
-            return Ok(Value::None);
-        }
-        let value = binding.get(name).unwrap();
-        match value {
-            ContextValue::Variable(v) => Ok(v.clone()),
-            ContextValue::Function(func) => func(Vec::new()),
-        }
+        let func = {
+            let binding = self.0.lock().unwrap();
+            match binding.get(name) {
+                Some(ContextValue::Variable(v)) => return Ok(v.clone()),
+                Some(ContextValue::Function(func)) => func.clone(),
+                None => return Ok(Value::None),
+            }
+        };
+        func(Vec::new())
     }
 }
 
@@ -104,3 +106,38 @@ macro_rules! create_context {
         ctx
     }};
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_context_lookups() {
+        let mut ctx = Context::new();
+        ctx.set_variable("my_var", Value::from(42));
+        ctx.set_func("my_func", Arc::new(|_| Ok(Value::from(100))));
+
+        // get_variable
+        assert_eq!(ctx.get_variable("my_var"), Some(Value::from(42)));
+        assert_eq!(ctx.get_variable("my_func"), None);
+        assert_eq!(ctx.get_variable("non_existent"), None);
+
+        // get_func
+        assert!(ctx.get_func("my_func").is_some());
+        assert!(ctx.get_func("my_var").is_none());
+        assert!(ctx.get_func("non_existent").is_none());
+
+        // value
+        assert_eq!(ctx.value("my_var").unwrap(), Value::from(42));
+        assert_eq!(ctx.value("my_func").unwrap(), Value::from(100));
+        assert_eq!(ctx.value("non_existent").unwrap(), Value::None);
+
+        // get
+        assert!(matches!(ctx.get("my_var"), Some(ContextValue::Variable(_))));
+        assert!(matches!(
+            ctx.get("my_func"),
+            Some(ContextValue::Function(_))
+        ));
+        assert!(matches!(ctx.get("non_existent"), None));
+    }
+}

src/parser.rs

@@ -52,22 +52,16 @@ impl<'a> fmt::Display for ExprAST<'a> {
             Self::Unary(op, rhs) => {
                 write!(f, "Unary AST: Op: {}, Rhs: {}", op, rhs)
             }
-            Self::Binary(op, lhs, rhs) => write!(
-                f,
-                "Binary AST: Op: {}, Lhs: {}, Rhs: {}",
-                op,
-                lhs,
-                rhs
-            ),
+            Self::Binary(op, lhs, rhs) => {
+                write!(f, "Binary AST: Op: {}, Lhs: {}, Rhs: {}", op, lhs, rhs)
+            }
             Self::Postfix(lhs, op) => {
                 write!(f, "Postfix AST: Lhs: {}, Op: {}", lhs, op,)
             }
             Self::Ternary(condition, lhs, rhs) => write!(
                 f,
                 "Ternary AST: Condition: {}, Lhs: {}, Rhs: {}",
-                condition,
-                lhs,
-                rhs
+                condition, lhs, rhs
             ),
             Self::Reference(name) => write!(f, "Reference AST: reference: {}", name),
             Self::Function(name, params) => {