perf: eliminate symlink stat syscalls by reusing canonicalization#1184
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1184 +/- ##
==========================================
+ Coverage 93.46% 93.78% +0.32%
==========================================
Files 22 22
Lines 4208 4216 +8
==========================================
+ Hits 3933 3954 +21
+ Misses 275 262 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Boshen
force-pushed
the
perf/eliminate-symlink-stat
branch
from
15c54c3 to
08c49fa
Compare
![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 automated review suggestions for this pull request.
Reviewed commit: 08c49fa980
ℹ️ 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".
Boshen
force-pushed
the
perf/eliminate-symlink-stat
branch
from
08c49fa to
79a2692
Compare
Merging this PR will not alter performance
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | pm/yarn-isolated |
1.1 ms | 1 ms | +6.49% |
| ❌ | pm/bun-isolated |
1.1 ms | 1.1 ms | -3.31% |
| ❌ | pm/bun-flat |
996.6 µs | 1,041.9 µs | -4.35% |
| ❌ | resolver_memory[multi-thread] |
385.4 µs | 399.4 µs | -3.5% |
| ❌ | resolver_real[multi-thread] |
393.3 µs | 407.7 µs | -3.52% |
| ❌ | pm/npm-flat |
942.1 µs | 985.1 µs | -4.37% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing perf/eliminate-symlink-stat (be27c2f) with main (4e9ac5e)2
Footnotes
-
5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(be27c2f) during the generation of this report, so 4e9ac5e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
Merge activity
|
) ## What `is_file`/`is_dir` on a symlinked path issued an `lstat` (to detect the symlink) followed by a `stat` (to follow it and learn file-vs-dir). But canonicalization already `read_link`s the same symlink for the final resolved path, so the `stat` duplicated work the resolver does anyway. `Cache::is_file`/`is_dir` now take a `symlinks` flag. For a symlink, when it's set, they reuse `canonicalize_impl` and read the canonical target's already-cached `lstat` instead of issuing a standalone `stat`. `ResolverGeneric` injects its `options.symlinks` through thin `is_file`/`is_dir` wrappers; the cache-internal callers (`find_package_json`, the `node_modules` lookup helpers) thread it explicitly. When disabled — or if canonicalization fails — it falls back to a direct `stat`, the previous behavior. ## Why it's correct `stat` and canonicalization both report the **final target's** file/dir type, so the cached `followed` value is identical regardless of which path computes it. Resolution decisions (which only use the resulting bool) are unchanged — canonicalization is simply warmed slightly earlier and reused. ## Impact Measured over a fixed 16-request workload across the 8 package-manager layouts in `fixtures/bench-pm`: | layout | before | after | | --- | --- | --- | | npm-flat | 91 | 89 | | pnpm-isolated | 122 | 114 | | pnpm-hoisted | 122 | 114 | | yarn-flat | 91 | 89 | | yarn-isolated | 115 | 107 | | yarn-pnp | 106 | 106 | | bun-flat | 91 | 89 | | bun-isolated | 122 | 114 | | **total** | **860** | **822** | All standalone symlink `stat` syscalls are eliminated. yarn-pnp is unchanged because it resolves through `VPath` and never follows symlinks.
graphite-app
Bot
force-pushed
the
perf/eliminate-symlink-stat
branch
from
79a2692 to
be27c2f
Compare
## 🤖 New release * `oxc_resolver`: 11.20.0 -> 11.21.0 * `oxc_resolver_napi`: 11.20.0 -> 11.21.0 <details><summary><i><b>Changelog</b></i></summary><p> ## `oxc_resolver` <blockquote> ## [11.21.0](v11.20.0...v11.21.0) - 2026-06-03 ### <!-- 0 -->🚀 Features - *(tsconfig)* support package.json imports field in extends ([#1199](#1199)) (by @Boshen) ### <!-- 1 -->🐛 Bug Fixes - *(tsconfig)* apply each referenced project's own `allowJs` ([#1198](#1198)) (by @shulaoda) - make symlink_metadata VPath-aware for Yarn PnP ([#1183](#1183)) (by @Boshen) ### <!-- 4 -->⚡ Performance - borrow relative main field instead of allocating a "./" prefix ([#1187](#1187)) (by @Boshen) - *(cache)* move package.json path into parse instead of cloning ([#1186](#1186)) (by @Boshen) - eliminate symlink stat syscalls by reusing canonicalization ([#1184](#1184)) (by @Boshen) - reduce resolution syscalls by unifying stat and lstat ([#1182](#1182)) (by @Boshen) ### <!-- 9 -->💼 Other - add baselines for each package manager x node_modules layout ([#1176](#1176)) (by @Boshen) ### Contributors * @shulaoda * @Boshen * @renovate[bot] </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). Co-authored-by: oxc-guard[bot] <276638029+oxc-guard[bot]@users.noreply.github.com>
1 participant
What
is_file/is_diron a symlinked path issued anlstat(to detect the symlink) followed by astat(to follow it and learn file-vs-dir). But canonicalization alreadyread_links the same symlink for the final resolved path, so thestatduplicated work the resolver does anyway.Cache::is_file/is_dirnow take asymlinksflag. For a symlink, when it's set, they reusecanonicalize_impland read the canonical target's already-cachedlstatinstead of issuing a standalonestat.ResolverGenericinjects itsoptions.symlinksthrough thinis_file/is_dirwrappers; the cache-internal callers (find_package_json, thenode_moduleslookup helpers) thread it explicitly. When disabled — or if canonicalization fails — it falls back to a directstat, the previous behavior.Why it's correct
statand canonicalization both report the final target's file/dir type, so the cachedfollowedvalue is identical regardless of which path computes it. Resolution decisions (which only use the resulting bool) are unchanged — canonicalization is simply warmed slightly earlier and reused.Impact
Measured over a fixed 16-request workload across the 8 package-manager layouts in
fixtures/bench-pm:All standalone symlink
statsyscalls are eliminated. yarn-pnp is unchanged because it resolves throughVPathand never follows symlinks.