Migrate runtime from AST post-order execution to bytecode IR + VM with execution-only benchmarks and VM stack optimization#64
Conversation
Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/6f8c8ce5-888e-4fa5-b7c2-91c9b8001b87 Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/6f8c8ce5-888e-4fa5-b7c2-91c9b8001b87 Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/6f8c8ce5-888e-4fa5-b7c2-91c9b8001b87 Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
|
@copilot give me a comparative overview of the execution performance of these two execution algorithms |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #64 +/- ##
==========================================
+ Coverage 90.66% 90.96% +0.29%
==========================================
Files 11 12 +1
Lines 1211 1394 +183
==========================================
+ Hits 1098 1268 +170
- Misses 113 126 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I benchmarked both versions with the existing Criterion target (
So on this benchmark, execution is currently ~5–7% slower with VM, while parse-only is slightly faster (small difference, likely noise-level). The current |
|
@copilot compare the execution performance from multiple dimensions I'm curious about why Bytecode based algorithm is slower |
There was a problem hiding this comment.
Pull request overview
This PR migrates the expression runtime from recursive AST execution to a bytecode compilation step executed by a stack-based VM, while keeping the existing parse-to-AST API and expression semantics intact.
Changes:
- Updated
execute()to parse → compile (bytecode) → execute (VM). - Added a new bytecode IR, compiler (AST→IR), and VM interpreter with parity tests against AST execution.
- Updated crate/docs wording to reflect the parse/compile/execute pipeline.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/lib.rs | Routes execute() through AST parsing, bytecode compilation, and VM execution; updates crate-level docs. |
| src/bytecode.rs | Introduces Program/Instruction, compiler, VM executor, and AST-vs-VM parity tests. |
| README.md | Updates introduction to describe the new AST→bytecode→VM architecture. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mod tests { | ||
| use super::{compile_expression, execute_program, Instruction}; | ||
| use crate::{create_context, parse_expression, Value}; | ||
| fn run_ast(expr: &str) -> crate::Result<Value> { | ||
| let ast = parse_expression(expr)?; | ||
| let mut ctx = create_context!( | ||
| "d" => 3, | ||
| "f" => Arc::new(|_| Ok(Value::from(3))) | ||
| ); | ||
| ast.exec(&mut ctx) | ||
| } | ||
|
|
||
| fn run_vm(expr: &str) -> crate::Result<Value> { | ||
| let ast = parse_expression(expr)?; | ||
| let program = compile_expression(&ast)?; | ||
| let mut ctx = create_context!( | ||
| "d" => 3, | ||
| "f" => Arc::new(|_| Ok(Value::from(3))) | ||
| ); |
There was a problem hiding this comment.
Arc is used in the test context setup (Arc::new(...)) but the test module does not import it, so this file won’t compile. Add use std::sync::Arc; in the tests module (or qualify the uses) to fix the compilation error.
From the same Criterion run (
Why VM is slower right now (likely contributors):
So the current numbers are expected for one-shot execution. The VM architecture should show its advantage when we reuse compiled bytecode across repeated evaluations (amortizing compile cost). |
|
@copilot Compare the execution performance solely from the execution level, for instance execute AST and Byte code directly. The benchmark should cover multiple scenarios like short expression, long expression, function call, and so on. finally, please dump the benchmark into an |
Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/0fad966f-4a13-423f-82d2-20ae0387e8f8 Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
Implemented in commit I added execution-only benchmarks that compare direct I also dumped the measured benchmark output into No UI changes in this PR, so there is no UI screenshot to provide. |
ashyanSpada
left a comment
There was a problem hiding this comment.
@copilot current implementation of value stack is rather slow. Consider pre allocating value stack with limit length and using current pointer to push and pop value. Notice if current pointer exceed value stack, expand it at first for more space
Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/2079d0c4-cc01-4f29-9ccf-60ea9d2ccad5 Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/2079d0c4-cc01-4f29-9ccf-60ea9d2ccad5 Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
Implemented in commit The VM now preallocates stack capacity from instruction count, tracks an explicit Screenshot: N/A (no UI changes in this backend/runtime update). |
| @@ -0,0 +1,33 @@ | |||
| # Execution-only benchmark: AST vs Bytecode VM | |||
There was a problem hiding this comment.
@copilot According to the current implementation, update the performance comparison data.
There was a problem hiding this comment.
Updated in commit aa0d38b.
benchmarks_execution_only.md now reflects freshly re-run execution-only Criterion data from the current implementation (including the VM stack optimization), with updated per-scenario deltas and takeaways.
Screenshot: N/A (no UI changes; benchmark documentation update only).
Agent-Logs-Url: https://github.com/ashyanSpada/expression_engine_rs/sessions/0933e2b1-7511-4ed4-aac7-5ed8d31abd53 Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
|
@codex[agent] Please increase the coverage rate of Unit tests. |
Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
Added VM-focused unit tests covering stack growth/reserve, jump validation bounds, JumpIfFalse type errors, and pop underflow to raise execution-level coverage (commit 9879a9f). |
This PR transitions execution from recursive AST post-order traversal to a linear bytecode pipeline while preserving public API behavior and expression semantics. Expressions are still parsed into AST, then compiled into bytecode, and executed by a stack-based VM.
Runtime architecture
parse_expressionand AST types intact for compatibility.Bytecode IR and compiler
src/bytecode.rswith:Program { instructions, constants }Instructionopcodes for literals, references, function calls, unary/binary/postfix ops, list/map construction, statement chaining, and control-flow jumps.=,+=, etc.) with explicit reference target metadata.VM executor
InfixOpManager,PrefixOpManager,PostfixOpManagerInnerFunctionManagerAPI integration
executeto useparse_expression→compile_expression→execute_program..exec()path in place for compatibility and parity reference.bytecodemodule publicly to support direct execution benchmarking.Performance benchmarking additions
benches/execute_expression.rswith execution-only comparisons between:ast.exec(&mut ctx)bytecode::execute_program(&program, &mut ctx)benchmarks_execution_only.mdwith captured benchmark results and summary.Testing/coverage
Docs update