⚡ perf(eval): wrap function parameters and Markdown in Box for improved memory management#1703
Merged
Conversation
Merging this PR will improve performance by 45.93%
Performance Changes
Comparing 
Footnotes |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates mq-lang’s runtime representation to reduce move/clone costs by boxing (heap-allocating) heavier RuntimeValue payloads—specifically Markdown nodes and function parameter lists—and then adjusts downstream crates and tests to the new representation.
Changes:
- Changed
RuntimeValue::Markdownto storeBox<mq_markdown::Node>andRuntimeValue::Functionto storeBox<AstParams>. - Added
RuntimeValue::new_markdown(...)and updated call sites to correctly clone/move out ofBox<Node>as needed. - Updated multiple consumers (CLI/grep, web API, WASM, LSP, crawler) and a large set of tests to match the new runtime layout.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/mq-web-api/src/api.rs | Adjust Markdown extraction to clone from Box<Node>. |
| crates/mq-wasm/src/script.rs | Move Node out of boxed Markdown runtime values when rendering. |
| crates/mq-run/src/grep.rs | Adjust Markdown-to-nodes conversion for boxed nodes. |
| crates/mq-run/src/cli.rs | Update Markdown collection/flattening utilities for boxed nodes. |
| crates/mq-lsp/src/execute_command.rs | Move Node out of boxed runtime values for Markdown output. |
| crates/mq-lang/tests/integration_tests.rs | Update integration tests to construct/compare boxed Markdown runtime values. |
| crates/mq-lang/src/lib.rs | Update unit tests and Markdown rendering logic for boxed nodes. |
| crates/mq-lang/src/eval/runtime_value.rs | Core change: box Markdown nodes + function params; add new_markdown; update helpers. |
| crates/mq-lang/src/eval/env.rs | Update tests for boxed function params. |
| crates/mq-lang/src/eval/builtin/convert.rs | Update Markdown construction and markdown-string conversion to account for boxed nodes. |
| crates/mq-lang/src/eval/builtin.rs | Update multiple builtins for boxed nodes/params and node extraction/replacement. |
| crates/mq-lang/src/eval.rs | Update evaluator to construct/use boxed Markdown values and boxed function params. |
| crates/mq-crawler/src/crawler.rs | Move Node out of boxed runtime values when building Markdown output. |
…kdown constructor - Box<Node> in RuntimeValue::Markdown reduces enum size from 176 → 40 bytes - Add RuntimeValue::new_markdown(node) as the canonical constructor - Replace all Markdown(Box::new(...), None) call sites with new_markdown() - Restructure nested Node pattern matches in builtin functions to use &mut **node - Update all crates (mq-run, mq-wasm, mq-lsp, mq-crawler, mq-web-api) accordingly
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… Node enum with Default
| } | ||
|
|
||
| Ok(RuntimeValue::Markdown( | ||
| Box::new(std::mem::take(node)), |
Comment on lines
90
to
96
| /// An array of runtime values. | ||
| Array(Vec<RuntimeValue>), | ||
| /// A markdown node with an optional selector for indexing. | ||
| Markdown(Node, Option<Selector>), | ||
| Markdown(Box<Node>, Option<Selector>), | ||
| /// A user-defined function with parameters, body (program), and captured environment. | ||
| Function(AstParams, Program, Shared<SharedCell<Env>>), | ||
| Function(Box<AstParams>, Program, Shared<SharedCell<Env>>), | ||
| /// A built-in native function identified by name. |
| }) | ||
| .into()), | ||
| [RuntimeValue::Markdown(node, _), RuntimeValue::Boolean(ordered)] | ||
| if matches!(**node, mq_markdown::Node::List(_)) => |
Comment on lines
+1916
to
+1918
| [RuntimeValue::Markdown(node, _)] | ||
| if matches!(**node, mq_markdown::Node::Definition(_) | mq_markdown::Node::Link(_)) => | ||
| { |
Comment on lines
+1927
to
+1929
| [RuntimeValue::Markdown(node, _)] if matches!(**node, mq_markdown::Node::Image(_)) => { | ||
| if let mq_markdown::Node::Image(mq_markdown::Image { title, .. }) = &mut **node { | ||
| std::mem::take(title) |
| }) | ||
| .into()), | ||
| [RuntimeValue::Markdown(node, _), RuntimeValue::Boolean(checked)] | ||
| if matches!(**node, mq_markdown::Node::List(_)) => |
Comment on lines
+2036
to
+2038
| [RuntimeValue::Markdown(node, _), RuntimeValue::String(lang)] | ||
| if matches!(**node, mq_markdown::Node::Code(_)) => | ||
| { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.