Skip to content

perf: avoid redundant PathBuf allocations in resolve paths#8435

Merged
graphite-app[bot] merged 1 commit intomainfrom
perf/reduce-path-allocations
Feb 24, 2026
Merged

perf: avoid redundant PathBuf allocations in resolve paths#8435
graphite-app[bot] merged 1 commit intomainfrom
perf/reduce-path-allocations

Conversation

@Brooooooklyn
Copy link
Copy Markdown
Member

@Brooooooklyn Brooooooklyn commented Feb 24, 2026

  • Skip cwd.join(importer) when importer is already an absolute path,
    passing &Path directly to resolve_file() instead of allocating a
    new PathBuf (~50K avoided allocations per 10K module build)
  • Apply the same optimization in rolldown_plugin_vite_resolve (2 sites)
  • Point oxc_resolver to local path for coordinated optimization

Combined with the oxc-resolver changes oxc-project/oxc-resolver#1027, this eliminates ~174K
to_path_buf allocations (21.3 MB) down to effectively zero,
yielding ~9% faster JS API median on a 10K module benchmark.

Co-Authored-By: Claude Opus 4.6 [email protected]


Note

Low Risk
Small, localized change to how importer paths are passed into the resolver; primary risk is subtle behavior differences if callers pass unusual absolute/relative importer strings.

Overview
Reduces resolve-time allocations by detecting when an importer is already an absolute path and calling resolve_file(importer, ...) directly instead of cwd.join(importer)/root.join(importer).

Applies this optimization in rolldown_resolver::Resolver::resolve and in two resolution paths in rolldown_plugin_vite_resolve::Resolver::resolve_raw (including the try_prefix fallback).

Written by Cursor Bugbot for commit 51016de. This will update automatically on new commits. Configure here.

Copilot AI review requested due to automatic review settings February 24, 2026 02:36
Copy link
Copy Markdown
Member Author


How to use the Graphite Merge Queue

Add the label graphite: merge-when-ready to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@netlify
Copy link
Copy Markdown

netlify bot commented Feb 24, 2026

Deploy Preview for rolldown-rs ready!

Name Link
🔨 Latest commit 5f4bde9
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/699dbe8a3e542d000834d4cf
😎 Deploy Preview https://deploy-preview-8435--rolldown-rs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@Brooooooklyn Brooooooklyn force-pushed the perf/reduce-path-allocations branch from 493d5b7 to c031d41 Compare February 24, 2026 02:38
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 493d5b7b0a

ℹ️ 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
  • 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 address that feedback".

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR focuses on reducing path-related allocations in module resolution hot paths by avoiding unnecessary PathBuf creation when the importer is already absolute, and aligns the resolver dependency to a local checkout for coordinated optimization work.

Changes:

  • Avoid cwd/root.join(importer) when importer is absolute, passing the importer directly to resolve_file() in two resolver implementations.
  • Update the workspace oxc_resolver dependency to a local path.
  • Update Cargo.lock to reflect the dependency/source changes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
crates/rolldown_resolver/src/resolver.rs Skip cwd.join(importer) when importer is absolute to avoid allocations.
crates/rolldown_plugin_vite_resolve/src/resolver.rs Apply the same absolute-importer fast path at two resolve sites.
Cargo.toml Switch oxc_resolver from crates.io to a sibling path dependency.
Cargo.lock Lockfile updates showing both path-overridden and registry-sourced oxc_resolver.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 24, 2026

Benchmarks Rust

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     61.6±0.89ms        ? ?/sec    1.05     64.4±1.04ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     66.1±0.87ms        ? ?/sec    1.05     69.3±2.58ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00     94.1±1.19ms        ? ?/sec    1.01     95.5±2.23ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    103.6±1.36ms        ? ?/sec    1.02    106.0±1.71ms        ? ?/sec
bundle/bundle@threejs                                        1.01     33.7±2.20ms        ? ?/sec    1.00     33.3±0.36ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     37.5±0.74ms        ? ?/sec    1.01     37.8±0.39ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    342.9±2.34ms        ? ?/sec    1.03    352.9±4.47ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    395.0±2.92ms        ? ?/sec    1.03    405.2±3.08ms        ? ?/sec
scan/scan@rome_ts                                            1.00     73.3±1.16ms        ? ?/sec    1.02     74.7±1.37ms        ? ?/sec
scan/scan@threejs                                            1.01     26.3±1.71ms        ? ?/sec    1.00     26.1±0.29ms        ? ?/sec
scan/scan@threejs10x                                         1.00    264.4±3.75ms        ? ?/sec    1.03    271.1±3.95ms        ? ?/sec

hyf0
hyf0 previously approved these changes Feb 24, 2026
@hyf0 hyf0 requested a review from sapphi-red February 24, 2026 03:21
@Brooooooklyn Brooooooklyn changed the title perf: avoid redundant PathBuf allocations in resolve paths perf: avoid redundant PathBuf allocations in resolve paths [DO NOT MERGE, RUNNING BENCHMARK] Feb 24, 2026
Copilot AI review requested due to automatic review settings February 24, 2026 03:46
@Brooooooklyn Brooooooklyn force-pushed the perf/reduce-path-allocations branch from 897aa58 to efc9123 Compare February 24, 2026 03:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

@Brooooooklyn Brooooooklyn force-pushed the perf/reduce-path-allocations branch from efc9123 to 51016de Compare February 24, 2026 03:56
@Brooooooklyn Brooooooklyn changed the title perf: avoid redundant PathBuf allocations in resolve paths [DO NOT MERGE, RUNNING BENCHMARK] perf: avoid redundant PathBuf allocations in resolve paths Feb 24, 2026
@hyf0 hyf0 dismissed their stale review February 24, 2026 04:18

Let me see more carefully

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 24, 2026

@hyf0 I've opened a new pull request, #8437, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI review requested due to automatic review settings February 24, 2026 14:49
Copy link
Copy Markdown
Member

hyf0 commented Feb 24, 2026

Merge activity

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

- Skip `cwd.join(importer)` when importer is already an absolute path,
  passing `&Path` directly to `resolve_file()` instead of allocating a
  new PathBuf (~50K avoided allocations per 10K module build)
- Apply the same optimization in rolldown_plugin_vite_resolve (2 sites)
- Point oxc_resolver to local path for coordinated optimization

Combined with the oxc-resolver changes oxc-project/oxc-resolver#1027, this eliminates ~174K
`to_path_buf` allocations (21.3 MB) down to effectively zero,
yielding ~9% faster JS API median on a 10K module benchmark.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Small, localized change to how importer paths are passed into the resolver; primary risk is subtle behavior differences if callers pass unusual absolute/relative importer strings.
>
> **Overview**
> Reduces resolve-time allocations by detecting when an `importer` is already an absolute path and calling `resolve_file(importer, ...)` directly instead of `cwd.join(importer)`/`root.join(importer)`.
>
> Applies this optimization in `rolldown_resolver::Resolver::resolve` and in two resolution paths in `rolldown_plugin_vite_resolve::Resolver::resolve_raw` (including the `try_prefix` fallback).
>
> <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 51016de. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@graphite-app graphite-app bot force-pushed the perf/reduce-path-allocations branch from 28562e1 to 5f4bde9 Compare February 24, 2026 15:06
@graphite-app graphite-app bot merged commit 5f4bde9 into main Feb 24, 2026
34 checks passed
@graphite-app graphite-app bot deleted the perf/reduce-path-allocations branch February 24, 2026 15:18
shulaoda added a commit that referenced this pull request Feb 26, 2026
## [1.0.0-rc.6] - 2026-02-26

### 💥 BREAKING CHANGES

- css: remove `css_entry_filenames` , `css_chunk_filenames` and related code (#8402) by @hyf0
- css: drop builtin CSS bundling to explore alternative solutions (#8399) by @hyf0

### 🚀 Features

- rust/data-url: use hash as id for data url modules to prevent long string overhead (#8420) by @hyf0
- validate bundle stays within output dir (#8441) by @sapphi-red
- rust: support `PluginOrder::PinPost` (#8417) by @hyf0
- support `ModuleType:Copy` (#8407) by @hyf0
- expose `ESTree` types from `rolldown/utils` (#8400) by @sapphi-red

### 🐛 Bug Fixes

- incorrect sourcemap when postBanner/postFooter is used with shebang (#8459) by @Copilot
- resolver: disable node_path option to align ESM resolver behavior (#8472) by @sapphi-red
- parse `.js` within `"type": "commonjs"` as ESM for now (#8470) by @sapphi-red
- case-insensitive filename conflict detection for chunk deduplication (#8458) by @Copilot
- prevent inlining CJS exports that are mutated by importers (#8456) by @IWANABETHATGUY
- parse `.cjs` / `.cts` / `.js` within `"type": "commonjs"` as CommonJS (#8455) by @sapphi-red
- plugin/copy-module: correct hooks' priority (#8423) by @hyf0
- plugin/chunk-import-map: ensure `render_chunk_meta` run after users plugin (#8422) by @hyf0
- rust: correct hooks order of `DataUriPlugin` (#8418) by @hyf0
- `jsx.preserve` should also considering tsconfig json preserve (#8324) by @IWANABETHATGUY
- `deferred_scan_data.rs "Should have resolved id: NotFound"` error (#8379) by @sapphi-red
- cli: require value for `--dir`/`-d` and `--file`/`-o` (#8378) by @Copilot
- dev: avoid mutex deadlock caused by inconsistent lock order (#8370) by @sapphi-red

### 🚜 Refactor

- watch: rename TaskStart/TaskEnd to BundleStart/BundleEnd (#8463) by @hyf0
- rust: rename `rolldown_plugin_data_uri` to `rolldown_plugin_data_url` (#8421) by @hyf0
- bindingify-build-hook: extract helper for PluginContextImpl (#8438) by @ShroXd
- give source loading a proper name (#8436) by @IWANABETHATGUY
- ban holding DashMap refs across awaits (#8362) by @sapphi-red

### 📚 Documentation

- add glob pattern usage example to input option (#8469) by @IWANABETHATGUY
- remove `https://rolldown.rs` from links in reference docs (#8454) by @sapphi-red
- mention execution order issue in `output.codeSplitting` docs (#8452) by @sapphi-red
- clarify `output.comments` behavior a bit (#8451) by @sapphi-red
- replace npmjs package links with npmx.dev (#8439) by @Boshen
- reference: add `Exported from` for values / types exported from subpath exports (#8394) by @sapphi-red
- add JSDocs for APIs exposed from subpath exports (#8393) by @sapphi-red
- reference: generate reference pages for APIs exposed from subpath exports (#8392) by @sapphi-red
- avoid pipe character in codeSplitting example to fix broken rendering (#8391) by @IWANABETHATGUY

### ⚡ Performance

- avoid redundant PathBuf allocations in resolve paths (#8435) by @Brooooooklyn
- bump to `sugar_path@2` (#8432) by @hyf0
- use flag-based convergence detection in include_statements (#8412) by @Brooooooklyn

### 🧪 Testing

- execute `_test.mjs` even if `executeOutput` is false (#8398) by @sapphi-red
- add retry to tree-shake/module-side-effects-proxy4 as it is flaky (#8397) by @sapphi-red
- avoid `expect.assertions()` as it is not concurrent test friendly (#8383) by @sapphi-red
- disable `mockReset` option (#8382) by @sapphi-red
- fix flaky failure caused by concurrent resolveId calls (#8381) by @sapphi-red

### ⚙️ Miscellaneous Tasks

- deps: update dependency rollup to v4.59.0 [security] (#8471) by @renovate[bot]
- ai/design: add design doc about watch mode (#8453) by @hyf0
- deps: update oxc resolver to v11.19.0 (#8461) by @renovate[bot]
- ai: introduce progressive spec-driven development pattern (#8446) by @hyf0
- deprecate output.legalComments (#8450) by @sapphi-red
- deps: update dependency oxlint-tsgolint to v0.15.0 (#8448) by @renovate[bot]
- ai: make CLAUDE.md a symlink of AGENTS.md (#8445) by @hyf0
- deps: update rollup submodule for tests to v4.59.0 (#8433) by @sapphi-red
- deps: update test262 submodule for tests (#8434) by @sapphi-red
- deps: update oxc to v0.115.0 (#8430) by @renovate[bot]
- deps: update oxc apps (#8429) by @renovate[bot]
- deps: update npm packages (#8426) by @renovate[bot]
- deps: update rust crate owo-colors to v4.3.0 (#8428) by @renovate[bot]
- deps: update github-actions (#8424) by @renovate[bot]
- deps: update rust crates (#8425) by @renovate[bot]
- deps: update oxc resolver to v11.18.0 (#8406) by @renovate[bot]
- deps: update dependency oxlint-tsgolint to v0.14.2 (#8405) by @renovate[bot]
- ban `expect.assertions` in all fixture tests (#8395) by @sapphi-red
- deps: update oxc apps (#8389) by @renovate[bot]
- ban `expect.assertions` in fixture tests (#8387) by @sapphi-red
- enable lint for `_config.ts` files (#8386) by @sapphi-red
- deps: update dependency oxlint-tsgolint to v0.14.1 (#8385) by @renovate[bot]

Co-authored-by: shulaoda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants