Avoid unnecessary cloning of the code string#6262
Conversation
I noticed that we make a copy of all code before parsing, which is unnecessary.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#avoid-code-cloneNotice: Ensure you have installed the latest nightly Rust toolchain. If you haven't installed it yet, please see https://www.rust-lang.org/tools/install to learn how to download Rustup and install Rust. or load it into the REPL: |
There was a problem hiding this comment.
Pull request overview
This PR optimizes memory usage in the NAPI bindings by avoiding an unnecessary string clone operation when parsing JavaScript/TypeScript code. The change replaces self.code.clone() with mem::take(&mut self.code) in the async parse task, which transfers ownership of the string instead of duplicating it. Additionally, the PR removes the unused swc_atoms dependency from the parse_ast crate.
Changes:
- Replaced string cloning with
mem::takein the NAPI async parse task to avoid duplicating potentially large code strings - Removed unused
swc_atomsdependency fromparse_astcrate
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| rust/bindings_napi/src/lib.rs | Added std::mem import and replaced self.code.clone() with mem::take(&mut self.code) in ParseTask::compute to avoid string duplication |
| rust/parse_ast/Cargo.toml | Removed unused swc_atoms dependency |
| rust/Cargo.lock | Updated lockfile to reflect removal of swc_atoms dependency |
Performance report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6262 +/- ##
=======================================
Coverage 98.76% 98.76%
=======================================
Files 273 273
Lines 10714 10714
Branches 2855 2855
=======================================
Hits 10582 10582
Misses 89 89
Partials 43 43 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This PR has been released as part of rollup@4.58.0. You can test it via |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
A small thing I spotted, we first clone the code before parsing it, whch is not strictly necessary.