fix: simplify compilation (#429)

* fix: simplify compilation

* improve opcodecache structure

core/engine/src/model/decision_content.rs

@@ -1,15 +1,8 @@
-use ahash::{HashMap, HashMapExt};
 use serde::{Deserialize, Serialize};
 use std::sync::Arc;
-use zen_expression::compiler::Opcode;
-use zen_expression::{ExpressionKind, Isolate};
+use zen_expression::{ExpressionKind, Isolate, OpcodeCache};
 use zen_types::decision::{DecisionEdge, DecisionNode, DecisionNodeKind};
 
-#[derive(Clone, Debug, Eq, PartialEq, Hash)]
-pub struct CompilationKey {
-    pub kind: ExpressionKind,
-    pub source: Arc<str>,
-}
 #[derive(Clone, Debug, PartialEq, Deserialize, Serialize, Default)]
 #[serde(rename_all = "camelCase")]
 pub struct DecisionContent {
@@ -17,33 +10,19 @@ pub struct DecisionContent {
     pub edges: Vec<Arc<DecisionEdge>>,
 
     #[serde(skip)]
-    pub compiled_cache: Option<Arc<HashMap<CompilationKey, Vec<Opcode>>>>,
+    pub compiled_cache: Option<Arc<OpcodeCache>>,
 }
 
 impl DecisionContent {
     pub fn compile(&mut self) {
-        let mut compiled_cache: HashMap<CompilationKey, Vec<Opcode>> = HashMap::new();
-        let mut isolate = Isolate::new();
+        let mut sources: Vec<(Arc<str>, ExpressionKind)> = Vec::new();
 
         for node in &self.nodes {
             match &node.kind {
                 DecisionNodeKind::ExpressionNode { content } => {
-                    for expression in content.expressions.iter() {
-                        if expression.key.is_empty() || expression.value.is_empty() {
-                            continue;
-                        }
-
-                        let key = CompilationKey {
-                            kind: ExpressionKind::Standard,
-                            source: Arc::clone(&expression.value),
-                        };
-
-                        if compiled_cache.contains_key(&key) {
-                            continue;
-                        }
-
-                        if let Ok(comp_expression) = isolate.compile_standard(&expression.value) {
-                            compiled_cache.insert(key, comp_expression.bytecode().to_vec());
+                    for expr in content.expressions.iter() {
+                        if !expr.key.is_empty() && !expr.value.is_empty() {
+                            sources.push((expr.value.clone(), ExpressionKind::Standard));
                         }
                     }
                 }
@@ -54,42 +33,13 @@ impl DecisionContent {
                                 continue;
                             };
 
-                            if rule_value.is_empty() {
-                                continue;
-                            }
+                            let kind = if input.field.is_some() {
+                                ExpressionKind::Unary
+                            } else {
+                                ExpressionKind::Standard
+                            };
 
-                            match &input.field {
-                                None => {
-                                    let key = CompilationKey {
-                                        kind: ExpressionKind::Standard,
-                                        source: Arc::clone(rule_value),
-                                    };
-
-                                    if !compiled_cache.contains_key(&key) {
-                                        if let Ok(comp_expression) =
-                                            isolate.compile_standard(rule_value)
-                                        {
-                                            compiled_cache
-                                                .insert(key, comp_expression.bytecode().to_vec());
-                                        }
-                                    }
-                                }
-                                Some(_field) => {
-                                    let key = CompilationKey {
-                                        kind: ExpressionKind::Unary,
-                                        source: Arc::clone(rule_value),
-                                    };
-
-                                    if !compiled_cache.contains_key(&key) {
-                                        if let Ok(comp_expression) =
-                                            isolate.compile_unary(rule_value)
-                                        {
-                                            compiled_cache
-                                                .insert(key, comp_expression.bytecode().to_vec());
-                                        }
-                                    }
-                                }
-                            }
+                            sources.push((rule_value.clone(), kind));
                         }
 
                         for output in content.outputs.iter() {
@@ -97,28 +47,39 @@ impl DecisionContent {
                                 continue;
                             };
 
-                            if rule_value.is_empty() {
-                                continue;
-                            }
-
-                            let key = CompilationKey {
-                                kind: ExpressionKind::Standard,
-                                source: Arc::clone(rule_value),
-                            };
-
-                            if !compiled_cache.contains_key(&key) {
-                                if let Ok(comp_expression) = isolate.compile_standard(rule_value) {
-                                    compiled_cache.insert(key, comp_expression.bytecode().to_vec());
-                                }
-                            }
+                            sources.push((rule_value.clone(), ExpressionKind::Standard));
                         }
                     }
                 }
-
                 _ => {}
             }
         }
 
-        self.compiled_cache.replace(Arc::new(compiled_cache));
+        let mut cache: OpcodeCache = OpcodeCache::new();
+        let mut isolate = Isolate::new();
+
+        for (source, kind) in &sources {
+            let map = match kind {
+                ExpressionKind::Standard => &mut cache.standard,
+                ExpressionKind::Unary => &mut cache.unary,
+            };
+            if map.contains_key(source) {
+                continue;
+            }
+
+            let result = match kind {
+                ExpressionKind::Standard => isolate
+                    .compile_standard(source)
+                    .map(|e| e.bytecode().to_vec()),
+                ExpressionKind::Unary => {
+                    isolate.compile_unary(source).map(|e| e.bytecode().to_vec())
+                }
+            };
+            if let Ok(bytecode) = result {
+                map.insert(source.clone(), Arc::from(bytecode));
+            }
+        }
+
+        self.compiled_cache.replace(Arc::new(cache));
     }
 }

core/engine/src/model/mod.rs

@@ -1,4 +1,3 @@
 pub use zen_types::decision::*;
 mod decision_content;
-pub use decision_content::CompilationKey;
 pub use decision_content::DecisionContent;

core/engine/src/nodes/decision_table/mod.rs

@@ -1,4 +1,3 @@
-use crate::model::CompilationKey;
 use crate::nodes::definition::NodeHandler;
 use crate::nodes::result::NodeResult;
 use crate::nodes::{NodeContext, NodeResponse};
@@ -8,7 +7,7 @@ use std::ops::Deref;
 use std::rc::Rc;
 use std::sync::Arc;
 use zen_expression::variable::ToVariable;
-use zen_expression::{ExpressionKind, Isolate};
+use zen_expression::Isolate;
 use zen_types::decision::{DecisionTableContent, DecisionTableHitPolicy, TransformAttributes};
 use zen_types::variable::Variable;
 #[derive(Debug, Clone)]
@@ -39,8 +38,8 @@ impl NodeHandler for DecisionTableNodeHandler {
 
 impl DecisionTableNodeHandler {
     fn handle_first_hit(&self, ctx: DecisionTableContext) -> NodeResult {
-        let mut isolate = Isolate::new();
-        isolate.set_environment(ctx.input.depth_clone(1));
+        let mut isolate = Isolate::with_environment(ctx.input.depth_clone(1))
+            .with_cache(ctx.extensions.compiled_cache.clone());
 
         for (index, rule) in ctx.node.rules.iter().enumerate() {
             if let Some(result) = self.evaluate_row(&ctx, rule, &mut isolate) {
@@ -72,10 +71,10 @@ impl DecisionTableNodeHandler {
     }
 
     fn handle_collect(&self, ctx: DecisionTableContext) -> NodeResult {
-        let mut isolate = Isolate::new();
         let mut outputs = Vec::new();
         let mut traces = Vec::new();
-        isolate.set_environment(ctx.input.depth_clone(1));
+        let mut isolate = Isolate::with_environment(ctx.input.depth_clone(1))
+            .with_cache(ctx.extensions.compiled_cache.clone());
 
         for (index, rule) in ctx.node.rules.iter().enumerate() {
             if let Some(result) = self.evaluate_row(&ctx, rule, &mut isolate) {
@@ -121,44 +120,15 @@ impl DecisionTableNodeHandler {
 
             match &input.field {
                 None => {
-                    let key = CompilationKey {
-                        kind: ExpressionKind::Standard,
-                        source: Arc::from(rule_value.clone()),
-                    };
-                    let result: Variable;
-                    if let Some(codes) = ctx
-                        .extensions
-                        .compiled_cache
-                        .as_ref()
-                        .and_then(|cc| cc.get(&key))
-                    {
-                        result = isolate.run_compiled(codes).ok()?;
-                    } else {
-                        result = isolate.run_standard(rule_value).ok()?;
-                    }
+                    let result = isolate.run_standard(rule_value).ok()?;
                     if !result.as_bool().unwrap_or(false) {
                         return None;
                     }
                 }
                 Some(field) => {
                     isolate.set_reference(&field).ok()?;
-                    let key = CompilationKey {
-                        kind: ExpressionKind::Unary,
-                        source: Arc::from(rule_value.clone()),
-                    };
-                    if let Some(codes) = ctx
-                        .extensions
-                        .compiled_cache
-                        .as_ref()
-                        .and_then(|cc| cc.get(&key))
-                    {
-                        if !isolate.run_unary_compiled(codes).ok()? {
-                            return None;
-                        }
-                    } else {
-                        if !isolate.run_unary(&rule_value).ok()? {
-                            return None;
-                        }
+                    if !isolate.run_unary(rule_value).ok()? {
+                        return None;
                     }
                 }
             }
@@ -171,21 +141,7 @@ impl DecisionTableNodeHandler {
                 continue;
             }
 
-            let key = CompilationKey {
-                kind: ExpressionKind::Standard,
-                source: Arc::from(rule_value.clone()),
-            };
-            let res: Variable;
-            if let Some(codes) = ctx
-                .extensions
-                .compiled_cache
-                .as_ref()
-                .and_then(|cc| cc.get(&key))
-            {
-                res = isolate.run_compiled(codes).ok()?;
-            } else {
-                res = isolate.run_standard(rule_value).ok()?;
-            }
+            let res = isolate.run_standard(rule_value).ok()?;
             outputs.dot_insert(output.field.deref(), res);
         }
 

core/engine/src/nodes/expression/mod.rs

@@ -1,4 +1,3 @@
-use crate::model::CompilationKey;
 use crate::model::ExpressionNodeContent;
 use crate::nodes::result::NodeResult;
 use ahash::HashMap;
@@ -7,7 +6,7 @@ use std::rc::Rc;
 use crate::nodes::context::{NodeContext, NodeContextExt};
 use crate::nodes::definition::NodeHandler;
 use zen_expression::variable::{ToVariable, Variable};
-use zen_expression::{ExpressionKind, Isolate};
+use zen_expression::Isolate;
 use zen_types::decision::TransformAttributes;
 
 #[derive(Debug, Clone)]
@@ -29,31 +28,19 @@ impl NodeHandler for ExpressionNodeHandler {
 
     async fn handle(&self, ctx: NodeContext<Self::NodeData, Self::TraceData>) -> NodeResult {
         let result = Variable::empty_object();
-        let mut isolate = Isolate::new();
-        isolate.set_environment(ctx.input.depth_clone(1));
+        let mut isolate = Isolate::with_environment(ctx.input.depth_clone(1))
+            .with_cache(ctx.extensions.compiled_cache.clone());
 
         for expression in ctx.node.expressions.iter() {
             if expression.key.is_empty() || expression.value.is_empty() {
                 continue;
             }
-            let key = CompilationKey {
-                kind: ExpressionKind::Standard,
-                source: expression.value.clone(),
-            };
-            let value;
-            match ctx
-                .extensions
-                .compiled_cache
-                .as_ref()
-                .and_then(|cc| cc.get(&key))
-            {
-                Some(codes) => value = isolate.run_compiled(codes.as_slice()),
-                None => value = isolate.run_standard(&expression.value),
-            }
 
-            let value = value.with_node_context(&ctx, |_| {
-                format!(r#"Failed to evaluate expression: "{}""#, &expression.value)
-            })?;
+            let value = isolate
+                .run_standard(&expression.value)
+                .with_node_context(&ctx, |_| {
+                    format!(r#"Failed to evaluate expression: "{}""#, &expression.value)
+                })?;
             ctx.trace(|trace| {
                 trace.insert(
                     Rc::from(&*expression.key),

core/engine/src/nodes/extensions.rs

@@ -1,5 +1,4 @@
 use crate::loader::{DynamicLoader, NoopLoader};
-use crate::model::CompilationKey;
 use crate::nodes::custom::{DynamicCustomNode, NoopCustomNode};
 use crate::nodes::function::http_handler::DynamicHttpHandler;
 use crate::nodes::function::v2::function::{Function, FunctionConfig};
@@ -7,11 +6,10 @@ use crate::nodes::function::v2::module::console::ConsoleListener;
 use crate::nodes::function::v2::module::http::listener::HttpListener;
 use crate::nodes::function::v2::module::zen::ZenListener;
 use crate::nodes::validator_cache::ValidatorCache;
-use ahash::HashMap;
 use anyhow::Context;
 use std::cell::OnceCell;
 use std::sync::Arc;
-use zen_expression::compiler::Opcode;
+use zen_expression::OpcodeCache;
 
 /// This is created on every graph evaluation
 #[derive(Debug, Clone)]
@@ -21,7 +19,7 @@ pub struct NodeHandlerExtensions {
     pub(crate) loader: DynamicLoader,
     pub(crate) custom_node: DynamicCustomNode,
     pub(crate) http_handler: DynamicHttpHandler,
-    pub(crate) compiled_cache: Option<Arc<HashMap<CompilationKey, Vec<Opcode>>>>,
+    pub(crate) compiled_cache: Option<Arc<OpcodeCache>>,
 }
 
 impl Default for NodeHandlerExtensions {

core/expression/src/expression.rs

@@ -16,21 +16,36 @@ pub enum ExpressionKind {
     Unary,
 }
 
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct OpcodeCache {
+    pub standard: ahash::HashMap<Arc<str>, Arc<[Opcode]>>,
+    pub unary: ahash::HashMap<Arc<str>, Arc<[Opcode]>>,
+}
+
+impl OpcodeCache {
+    pub fn new() -> Self {
+        Self {
+            standard: Default::default(),
+            unary: Default::default(),
+        }
+    }
+}
+
 /// Compiled expression
 #[derive(Debug, Clone)]
 pub struct Expression<Kind> {
-    bytecode: Arc<Vec<Opcode>>,
+    bytecode: Arc<[Opcode]>,
     _marker: PhantomData<Kind>,
 }
 
 impl<Kind> Expression<Kind> {
-    pub fn bytecode(&self) -> &Arc<Vec<Opcode>> {
+    pub fn bytecode(&self) -> &Arc<[Opcode]> {
         &self.bytecode
     }
 }
 
 impl Expression<Standard> {
-    pub fn new_standard(bytecode: Arc<Vec<Opcode>>) -> Self {
+    pub fn new_standard(bytecode: Arc<[Opcode]>) -> Self {
         Expression {
             bytecode,
             _marker: PhantomData,
@@ -47,13 +62,13 @@ impl Expression<Standard> {
     }
 
     pub fn evaluate_with(&self, context: Variable, vm: &mut VM) -> Result<Variable, IsolateError> {
-        let output = vm.run(self.bytecode.as_slice(), context)?;
+        let output = vm.run(self.bytecode.as_ref(), context)?;
         Ok(output)
     }
 }
 
 impl Expression<Unary> {
-    pub fn new_unary(bytecode: Arc<Vec<Opcode>>) -> Self {
+    pub fn new_unary(bytecode: Arc<[Opcode]>) -> Self {
         Expression {
             bytecode,
             _marker: PhantomData,
@@ -80,7 +95,7 @@ impl Expression<Unary> {
         }
 
         let output = vm
-            .run(self.bytecode.as_slice(), context)?
+            .run(self.bytecode.as_ref(), context)?
             .as_bool()
             .ok_or_else(|| IsolateError::ValueCastError)?;
         Ok(output)

core/expression/src/isolate.rs

@@ -7,7 +7,7 @@ use thiserror::Error;
 
 use crate::arena::UnsafeArena;
 use crate::compiler::{Compiler, CompilerError, Opcode};
-use crate::expression::{Standard, Unary};
+use crate::expression::{OpcodeCache, Standard, Unary};
 use crate::lexer::{Lexer, LexerError};
 use crate::parser::{Parser, ParserError};
 use crate::variable::Variable;
@@ -29,6 +29,7 @@ pub struct Isolate<'arena> {
 
     environment: Option<Variable>,
     references: HashMap<String, Variable>,
+    cache: Option<Arc<OpcodeCache>>,
 }
 
 impl<'a> Isolate<'a> {
@@ -42,6 +43,7 @@ impl<'a> Isolate<'a> {
 
             environment: None,
             references: Default::default(),
+            cache: None,
         }
     }
 
@@ -52,10 +54,19 @@ impl<'a> Isolate<'a> {
         isolate
     }
 
+    pub fn with_cache(mut self, cache: Option<Arc<OpcodeCache>>) -> Self {
+        self.cache = cache;
+        self
+    }
+
     pub fn set_environment(&mut self, variable: Variable) {
         self.environment.replace(variable);
     }
 
+    pub fn set_cache(&mut self, cache: Arc<OpcodeCache>) {
+        self.cache = Some(cache);
+    }
+
     pub fn update_environment<F>(&mut self, mut updater: F)
     where
         F: FnMut(Option<&mut Variable>),
@@ -122,10 +133,18 @@ impl<'a> Isolate<'a> {
         self.run_internal(source, ExpressionKind::Standard)?;
         let bytecode = self.compiler.get_bytecode().to_vec();
 
-        Ok(Expression::new_standard(Arc::new(bytecode)))
+        Ok(Expression::new_standard(Arc::from(bytecode)))
     }
 
     pub fn run_standard(&mut self, source: &'a str) -> Result<Variable, IsolateError> {
+        let cached = self
+            .cache
+            .as_ref()
+            .and_then(|c| c.standard.get(source).cloned());
+        if let Some(codes) = cached {
+            return self.run_compiled(codes.as_ref());
+        }
+
         self.run_internal(source, ExpressionKind::Standard)?;
 
         let bytecode = self.compiler.get_bytecode();
@@ -147,10 +166,18 @@ impl<'a> Isolate<'a> {
         self.run_internal(source, ExpressionKind::Unary)?;
         let bytecode = self.compiler.get_bytecode().to_vec();
 
-        Ok(Expression::new_unary(Arc::new(bytecode)))
+        Ok(Expression::new_unary(Arc::from(bytecode)))
     }
 
     pub fn run_unary(&mut self, source: &'a str) -> Result<bool, IsolateError> {
+        let cached = self
+            .cache
+            .as_ref()
+            .and_then(|c| c.unary.get(source).cloned());
+        if let Some(codes) = cached {
+            return self.run_unary_compiled(codes.as_ref());
+        }
+
         self.run_internal(source, ExpressionKind::Unary)?;
 
         let bytecode = self.compiler.get_bytecode();

core/expression/src/lib.rs

@@ -72,6 +72,6 @@ pub mod vm;
 pub use exports::{
     compile_expression, compile_unary_expression, evaluate_expression, evaluate_unary_expression,
 };
-pub use expression::{Expression, ExpressionKind};
+pub use expression::{Expression, ExpressionKind, OpcodeCache};
 pub use isolate::{Isolate, IsolateError};
 pub use variable::Variable;