perf: reduce PathBuf allocations in resolution pipeline#1027
Closed
Brooooooklyn wants to merge 1 commit intomainfrom
Closed
perf: reduce PathBuf allocations in resolution pipeline#1027Brooooooklyn wants to merge 1 commit intomainfrom
Brooooooklyn wants to merge 1 commit intomainfrom
Conversation
Member
Author
How to use the Graphite Merge QueueAdd the label merge 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. |
- Use `Box::from(path)` instead of `path.to_path_buf().into_boxed_path()` in `Cache::value()` to avoid intermediate PathBuf allocation - Change `Resolution` to store `CachedPath` instead of `PathBuf`, deferring the PathBuf allocation to callers who actually need owned paths - Change `canonicalize()` and `load_realpath()` to return `CachedPath` instead of `PathBuf`, avoiding unnecessary cloning - Store `Box<Path>` instead of `PathBuf` in canonicalized cache fallback In a 10K module rolldown benchmark, this eliminates ~87K `to_path_buf` allocations (10.7 MB) from the resolution pipeline, reducing them to effectively zero. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Brooooooklyn
force-pushed
the
perf/reduce-path-allocations
branch
from
c9e8136 to
924d4d7
Compare
Merging this PR will degrade performance by 8.49%
Performance Changes
Comparing 
Footnotes
|
Member
Author
|
The CodSpeed regression appears to be noise |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1027 +/- ##
==========================================
+ Coverage 94.09% 94.14% +0.05%
==========================================
Files 19 19
Lines 3436 3433 -3
==========================================
- Hits 3233 3232 -1
+ Misses 203 201 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
|
Code was working as intended. |
graphite-app bot
pushed a commit
to rolldown/rolldown
that referenced
this pull request
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]> <!-- 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 -->
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
2 participants
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.
Box::from(path)instead ofpath.to_path_buf().into_boxed_path()in
Cache::value()to avoid intermediate PathBuf allocationResolutionto storeCachedPathinstead ofPathBuf, deferringthe PathBuf allocation to callers who actually need owned paths
canonicalize()andload_realpath()to returnCachedPathinstead of
PathBuf, avoiding unnecessary cloningBox<Path>instead ofPathBufin canonicalized cache fallbackIn a 10K module rolldown benchmark, this eliminates ~87K
to_path_bufallocations (10.7 MB) from the resolution pipeline, reducing them to
effectively zero.
Co-Authored-By: Claude Opus 4.6 [email protected]
Note
Medium Risk
Mostly a perf-driven type/ownership refactor across the resolver’s core path plumbing; low behavioral intent but subtle changes around canonicalization and path equality/formatting could surface platform-specific regressions (notably Windows/symlinks).
Overview
Reduces allocation churn in the resolution pipeline by switching from owned
PathBufreturns to cachedCachedPathvalues, including updatingResolutionto store aCachedPathand only materializing aPathBufon demand.Updates cache internals to avoid intermediate
PathBufcreation (Box::from(path)inCache::value) and to store canonicalization fallbacks asBox<Path>; adjusts Windows canonicalization prefix-stripping to re-enter the cache. Tests and tracing were updated to use the newResolution/realpath APIs.Written by Cursor Bugbot for commit 924d4d7. This will update automatically on new commits. Configure here.