Skip to content

perf(cache): move package.json path into parse instead of cloning#1186

Merged
graphite-app[bot] merged 1 commit into
mainfrom
perf/find-package-json-move-path
May 30, 2026
Merged

perf(cache): move package.json path into parse instead of cloning#1186
graphite-app[bot] merged 1 commit into
mainfrom
perf/find-package-json-move-path

Conversation

@Boshen

@Boshen Boshen commented May 30, 2026

Copy link
Copy Markdown
Member

find_package_json_impl cloned package_json_path to pass into PackageJson::parse so the original could outlive the call for the .inspect/.inspect_err closures that record the file dependency.

This replaces the combinator chain with a match that moves package_json_path into parse and reads the dependency path back from:

  • package_json.path() on success — PackageJson stores the path verbatim, and
  • JSONError.path on error — set from the same input path,

dropping one unconditional PathBuf allocation on the parse-success path. The whole block is behind path.package_json.get_or_try_init, so this only fires on the first parse of each unique package.json over the resolver's lifetime (a cache miss).

Behavior is unchanged: file/missing-dependency recording, error contents, and return values are identical.

🤖 Generated with Claude Code

@codecov

codecov Bot commented May 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.77%. Comparing base (be27c2f) to head (aba01c3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1186      +/-   ##
==========================================
- Coverage   93.78%   93.77%   -0.01%     
==========================================
  Files          22       22              
  Lines        4216     4210       -6     
==========================================
- Hits         3954     3948       -6     
  Misses        262      262              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq

codspeed-hq Bot commented May 30, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 3.64%

⚡ 1 improved benchmark
❌ 2 regressed benchmarks
✅ 18 untouched benchmarks
⏩ 5 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
pm/pnpm-isolated 1.1 ms 1.1 ms -3.82%
pm/yarn-flat 899.4 µs 996 µs -9.7%
large 26.6 µs 25.8 µs +3.02%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing perf/find-package-json-move-path (aba01c3) with main (be27c2f)2

Open in CodSpeed

Footnotes

  1. 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.

  2. No successful run was found on main (aba01c3) during the generation of this report, so be27c2f was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@Boshen Boshen added the merge label May 30, 2026

Boshen commented May 30, 2026

Copy link
Copy Markdown
Member Author

Merge activity

)

`find_package_json_impl` cloned `package_json_path` to pass into `PackageJson::parse` so the original could outlive the call for the `.inspect`/`.inspect_err` closures that record the file dependency.

This replaces the combinator chain with a `match` that **moves** `package_json_path` into `parse` and reads the dependency path back from:
- `package_json.path()` on success — `PackageJson` stores the path verbatim, and
- `JSONError.path` on error — set from the same input path,

dropping one unconditional `PathBuf` allocation on the parse-success path. The whole block is behind `path.package_json.get_or_try_init`, so this only fires on the first parse of each unique `package.json` over the resolver's lifetime (a cache miss).

Behavior is unchanged: file/missing-dependency recording, error contents, and return values are identical.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app Bot force-pushed the perf/find-package-json-move-path branch from 48b4149 to aba01c3 Compare May 30, 2026 03:43
@graphite-app graphite-app Bot merged commit aba01c3 into main May 30, 2026
18 checks passed
@graphite-app graphite-app Bot removed the merge label May 30, 2026
@graphite-app graphite-app Bot deleted the perf/find-package-json-move-path branch May 30, 2026 03:46
@oxc-guard oxc-guard Bot mentioned this pull request May 29, 2026
Boshen pushed a commit that referenced this pull request Jun 3, 2026
## 🤖 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>
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.

1 participant