Skip to content

perf(pm): update resolver cache ownership docs#3048

Closed
elrrrrrrr wants to merge 1 commit into
perf/pm-split-resolver-remove-memory-cachefrom
perf/pm-split-resolver-doc-cleanup
Closed

perf(pm): update resolver cache ownership docs#3048
elrrrrrrr wants to merge 1 commit into
perf/pm-split-resolver-remove-memory-cachefrom
perf/pm-split-resolver-doc-cleanup

Conversation

@elrrrrrrr
Copy link
Copy Markdown
Contributor

Summary

  • update full-manifest extraction docs to point at resolver-owned inflight extract jobs
  • clarify RegistryClient::registry_url fallback behavior for raw-fetch callers

Validation

  • cargo fmt
  • cargo check -p utoo-ruborist
  • cargo clippy --all-targets -- -D warnings --no-deps

Split Plan

Part of the resolver stack split from source PR #3028. This is the final documentation cleanup after cache ownership moved out of UnifiedRegistry.

@elrrrrrrr elrrrrrrr added A-Pkg Manager Area: Package Manager benchmark Run pm-bench on PR labels May 21, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates documentation comments in manifest.rs and registry.rs to reflect architectural changes regarding the BFS resolver's de-duplication of in-flight jobs and the intended usage of the registry URL. Feedback was provided to clarify that de-duplication prevents redundant parsing rather than bounding the cost of a single call, and to replace the informal term 'regular trait methods' with specific high-level resolution methods in the trait documentation.

Comment on lines +131 to +132
/// The BFS resolver de-duplicates in-flight extract jobs per key, so the
/// per-call full-tree parse cost is bounded.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The phrasing "per-call full-tree parse cost is bounded" is somewhat ambiguous. Since the method performs a full JSON parse of the raw bytes on every invocation, the cost of a single call is always proportional to the manifest size. The de-duplication mentioned actually limits the number of times this cost is incurred for concurrent requests. Consider rephrasing to clarify that the de-duplication prevents redundant parsing of the same version.

    /// The BFS resolver de-duplicates in-flight extract jobs per key, ensuring
    /// this expensive full-tree parse is not performed redundantly.

///
/// Implementations without a concrete URL can keep the default.
/// Implementations that cannot expose a concrete URL can keep the default;
/// callers should fall back to the regular trait methods when this is empty.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The term "regular trait methods" is a bit informal. Explicitly mentioning that callers should use high-level resolution or fetching methods (like resolve_package) would improve clarity for implementers and consumers of the trait, especially when they need to obtain absolute URLs for raw fetches.

    /// callers should use high-level resolution methods (e.g., resolve_package)
    /// to obtain absolute URLs when this is empty.

@elrrrrrrr elrrrrrrr force-pushed the perf/pm-split-resolver-remove-memory-cache branch from f029690 to 0225a9c Compare May 21, 2026 19:28
@elrrrrrrr elrrrrrrr force-pushed the perf/pm-split-resolver-doc-cleanup branch from 953e26b to 9b50ad8 Compare May 21, 2026 19:28
@elrrrrrrr elrrrrrrr force-pushed the perf/pm-split-resolver-doc-cleanup branch from 9b50ad8 to 9a0819b Compare May 21, 2026 22:32
@elrrrrrrr elrrrrrrr force-pushed the perf/pm-split-resolver-remove-memory-cache branch from 0225a9c to 83396aa Compare May 21, 2026 22:32
@elrrrrrrr elrrrrrrr force-pushed the perf/pm-split-resolver-doc-cleanup branch from 9a0819b to bb23373 Compare May 21, 2026 23:09
@elrrrrrrr elrrrrrrr force-pushed the perf/pm-split-resolver-remove-memory-cache branch from 83396aa to 8962d83 Compare May 21, 2026 23:09
@elrrrrrrr elrrrrrrr force-pushed the perf/pm-split-resolver-doc-cleanup branch from bb23373 to 711835d Compare May 21, 2026 23:39
@github-actions
Copy link
Copy Markdown

📊 pm-bench-phases · e0db640 · linux (ubuntu-latest)

Workflow run — ant-design

PMs: utoo (this branch) · utoo-npm (latest published) · bun (latest)

npmjs.org

p0_full_cold

PM wall ±σ user sys RSS pgMinor
bun 9.17s 0.26s 10.38s 10.38s 773M 345.8K
utoo-next 8.80s 1.13s 10.67s 12.47s 992M 119.0K
utoo-npm 8.34s 0.07s 10.77s 12.53s 947M 128.2K
utoo 8.14s 0.44s 11.18s 12.39s 978M 137.9K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 14.1K 17.7K 1.19G 6M 1.88G 1.76G 1M
utoo-next 116.7K 84.3K 1.16G 5M 1.71G 1.70G 2M
utoo-npm 117.3K 84.9K 1.16G 5M 1.71G 1.70G 2M
utoo 104.7K 60.7K 1.16G 6M 1.71G 1.70G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.03s 0.13s 4.02s 1.06s 518M 172.1K
utoo-next 2.86s 0.10s 5.40s 1.70s 608M 82.3K
utoo-npm 3.06s 0.08s 5.61s 2.08s 610M 89.3K
utoo 2.36s 0.06s 5.97s 1.66s 653M 120.9K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 8.2K 4.6K 202M 3M 107M - 1M
utoo-next 46.3K 67.5K 200M 2M 7M 3M 2M
utoo-npm 71.2K 88.3K 200M 2M 7M 3M 2M
utoo 14.8K 18.2K 202M 3M 7M 3M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 6.89s 0.49s 6.32s 10.11s 627M 207.4K
utoo-next 5.81s 0.11s 5.00s 11.05s 484M 61.0K
utoo-npm 7.02s 2.13s 5.21s 11.50s 493M 60.2K
utoo 6.94s 2.17s 5.00s 11.01s 508M 67.2K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 4.8K 6.6K 1019M 3M 1.76G 1.76G 1M
utoo-next 95.8K 50.0K 989M 3M 1.70G 1.70G 2M
utoo-npm 108.7K 50.7K 989M 3M 1.70G 1.70G 2M
utoo 102.1K 51.4K 989M 3M 1.70G 1.70G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 3.35s 0.06s 0.22s 2.35s 134M 33.0K
utoo-next 2.37s 0.21s 0.54s 3.93s 78M 18.5K
utoo-npm 2.30s 0.05s 0.53s 3.86s 78M 18.2K
utoo 2.32s 0.06s 0.54s 3.95s 79M 18.7K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 301 26 5M 15K 1.91G 1.75G 1M
utoo-next 42.1K 19.3K 3K 29K 1.70G 1.70G 2M
utoo-npm 42.0K 19.8K 1K 6K 1.70G 1.70G 2M
utoo 43.5K 20.6K 1K 5K 1.71G 1.70G 2M

npmmirror.com: no output captured.

@elrrrrrrr
Copy link
Copy Markdown
Contributor Author

Closing this temporary fine-grained draft split. We are replacing it with a smaller 9-PR review stack based on the source PRs.

@elrrrrrrr elrrrrrrr closed this May 25, 2026
@elrrrrrrr elrrrrrrr deleted the perf/pm-split-resolver-doc-cleanup branch May 25, 2026 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Pkg Manager Area: Package Manager benchmark Run pm-bench on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant