perf: optimize V8 engine by eliminating redundant instantiation #70
+802
−32
Conversation
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
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
- Reverted thread-local isolate caching that caused test failures - Kept the original simpler V8 implementation for better stability - Added comprehensive test coverage from the refactor attempt - All benchmarks still work with the original implementation
Removed duplicate V8JsRuleEngine creation in evaluate() method to reuse the existing instance, resulting in ~61% performance improvement (from ~1.2ms to ~550µs per evaluation). - Reuse existing engine instance via cloning in evaluate() - Eliminate redundant script compilation during validation - Keep isolate creation per-request for simplicity and stability - Extract execute_with_isolate() as a static method for cleaner code Performance improvements: - V8 engine: 61% faster (1.2ms → 550µs) - Now 2.7x faster than shell engine - All tests passing with no stability issues 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ammario
changed the title
- Add Rust cache sharing with other Linux workflows using shared-key - Use same runner (ubuntu-latest-8-cores) as test workflows for cache reuse - Switch to dtolnay/rust-toolchain for consistency with other workflows - Add build step to warm cache before running benchmarks - Enable cache-targets and cache-all-crates for comprehensive caching This allows the benchmark workflow to reuse cached dependencies from test runs and vice versa, significantly reducing CI build times.
- Update per-request latency based on actual benchmark measurements - JavaScript (V8): 550µs-1.3ms (previously ~100µs) - Shell Script: 700µs-1.6ms (previously ~1-3ms) - Line Processor: 70-90µs (previously ~100µs) - Add performance note explaining V8 isolate creation overhead - Clean up formatting in code examples
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.
Summary
Optimize V8 JavaScript rule engine performance by eliminating redundant engine instantiation, achieving ~61% performance improvement.
Changes
Performance Optimization
evaluate()methodCode Improvements
execute_with_isolate()as a static method for cleaner separationPerformance Results
The V8 engine is now 2.7x faster than the shell engine, though proc remains the performance leader.
Test Plan
cargo fmtImplementation Notes
This is a minimal, safe optimization that avoids the complexity of thread-local storage or isolate pooling. The previous attempt with thread-local caching was reverted due to cleanup issues. This approach is simpler and achieves most of the performance benefit by just eliminating unnecessary work in the hot path.
🤖 Generated with Claude Code