Skip to content

perf(pm): resolve install seeded cache synchronously#2967

Draft
elrrrrrrr wants to merge 1 commit into
perf/pm-resolver-demand-bfsfrom
exp/pm-install-sync-seeded-probe-b33d922
Draft

perf(pm): resolve install seeded cache synchronously#2967
elrrrrrrr wants to merge 1 commit into
perf/pm-resolver-demand-bfsfrom
exp/pm-install-sync-seeded-probe-b33d922

Conversation

@elrrrrrrr
Copy link
Copy Markdown
Contributor

@elrrrrrrr elrrrrrrr commented May 18, 2026 •
edited
Loading

Summary

  • resolve ruborist-seeded install cache paths synchronously inside the install scheduler
  • remove the per-package tokio task used only for cache marker probing
  • keep clone/download/extract behavior unchanged for this AB

Hypothesis

p4 warm-link still pays a per-package async cache-probe cost before falling through to the registry cache path. Moving the _resolved marker probe into the scheduler should reduce context switches without changing IO volume.

Validation

  • cargo fmt
  • cargo test -p utoo-pm service::install_scheduler::tests
  • cargo test -p utoo-pm util::downloader::tests
  • cargo clippy -p utoo-pm --all-targets -- -D warnings --no-deps

Benchmark Plan

@elrrrrrrr elrrrrrrr added A-Pkg Manager Area: Package Manager benchmark Run pm-bench on PR labels May 18, 2026
@elrrrrrrr elrrrrrrr mentioned this pull request May 18, 2026
Draft
gemini-code-assist[bot]
gemini-code-assist Bot reviewed May 18, 2026
View reviewed changes
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 refactors the cache lookup logic by converting several asynchronous functions in downloader.rs to synchronous versions and updating install_scheduler.rs to call them directly, eliminating unnecessary task spawning. Additionally, the test suite has been updated to use more specific package names and to reflect the change in task management. The reviewer suggested restoring the documentation for git_cache_lookup_sync and slot_cache_lookup_sync that was removed during the refactoring.

Comment thread crates/pm/src/util/downloader.rs
/// Git packages are cloned during BFS resolution (inside ruborist) and
/// stored at `<cache_dir>/<name>/<commit_sha>/`.
pub async fn git_cache_lookup(name: &str, version: &str, tarball_url: &str) -> Option<PathBuf> {
pub fn git_cache_lookup_sync(name: &str, version: &str, tarball_url: &str) -> Option<PathBuf> {
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 18, 2026

Choose a reason for hiding this comment

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

medium

The documentation for git_cache_lookup was removed during the transition to the synchronous version. This docstring provided valuable context about the cache layout for git packages and should be restored.

/// Look up the cache path for a git-resolved package.
///
/// Git packages are cloned during BFS resolution (inside ruborist) and
/// stored at <cache_dir>/<name>/<commit_sha>/.
pub fn git_cache_lookup_sync(name: &str, version: &str, tarball_url: &str) -> Option<PathBuf> {

Comment thread crates/pm/src/util/downloader.rs
/// otherwise returns `None` so the caller can fall through to the next
/// routing step (typically the registry download path).
async fn slot_cache_lookup(name: &str, slot: String) -> Option<PathBuf> {
fn slot_cache_lookup_sync(name: &str, slot: String) -> Option<PathBuf> {
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 18, 2026

Choose a reason for hiding this comment

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

medium

The documentation for slot_cache_lookup was removed. Restoring it helps maintainers understand the purpose of the _resolved marker and the fall-through behavior to the registry download path.

/// Look up a ruborist-seeded cache slot at <cache_dir>/<name>/<slot>/.
///
/// Returns Some(path) only if the slot's _resolved marker exists —
/// otherwise returns None so the caller can fall through to the next
/// routing step (typically the registry download path).
fn slot_cache_lookup_sync(name: &str, slot: String) -> Option<PathBuf> {

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 18, 2026

📊 pm-bench-phases · 84964a4 · 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 10.27s 0.66s 10.47s 10.33s 600M 302.7K
utoo-next 9.88s 1.79s 10.97s 12.95s 966M 117.9K
utoo-npm 12.01s 3.50s 11.34s 13.29s 1.01G 125.4K
utoo 8.68s 0.87s 11.70s 12.91s 899M 152.7K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 18.9K 18.1K 1.20G 7M 1.86G 1.74G 1M
utoo-next 138.6K 108.1K 1.17G 5M 1.73G 1.72G 2M
utoo-npm 156.4K 113.6K 1.17G 6M 1.73G 1.72G 2M
utoo 126.6K 91.2K 1.18G 6M 1.73G 1.72G 3M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.57s 0.14s 3.95s 1.11s 465M 183.2K
utoo-next 3.40s 0.02s 5.67s 2.28s 615M 84.2K
utoo-npm 3.41s 0.04s 5.77s 2.26s 618M 83.8K
utoo 2.86s 0.36s 6.10s 1.80s 643M 123.9K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 13.5K 3.3K 203M 3M 108M - 1M
utoo-next 82.1K 89.1K 201M 3M 7M 3M 2M
utoo-npm 83.5K 91.2K 201M 3M 7M 3M 2M
utoo 16.6K 19.3K 204M 3M 7M 3M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 6.84s 0.47s 6.38s 10.34s 579M 199.3K
utoo-next 10.76s 3.31s 5.09s 11.33s 478M 59.8K
utoo-npm 7.03s 0.52s 5.25s 11.04s 481M 61.0K
utoo 10.01s 2.35s 5.15s 11.27s 489M 60.5K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 7.5K 8.7K 1.00G 4M 1.77G 1.77G 1M
utoo-next 122.7K 56.4K 1000M 3M 1.72G 1.72G 2M
utoo-npm 108.7K 57.6K 1000M 3M 1.72G 1.72G 2M
utoo 115.8K 76.6K 1000M 3M 1.72G 1.72G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 3.38s 0.03s 0.19s 2.38s 135M 32.5K
utoo-next 2.46s 0.11s 0.47s 3.81s 79M 18.6K
utoo-npm 2.27s 0.14s 0.51s 3.79s 79M 18.2K
utoo 2.30s 0.12s 0.49s 3.87s 62M 14.1K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 377 29 5M 36K 1.93G 1.76G 1M
utoo-next 42.3K 18.7K 8K 28K 1.72G 1.72G 2M
utoo-npm 41.2K 18.8K 7K 9K 1.72G 1.72G 2M
utoo 47.8K 22.6K 7K 7K 1.73G 1.72G 2M

npmmirror.com: no output captured.

@elrrrrrrr
Copy link
Copy Markdown
Contributor Author

elrrrrrrr commented May 18, 2026

GHA run 1 read

Run: https://github.com/utooland/utoo/actions/runs/26016267872

phase utoo wall utoo ctx same-run utoo-next same-run bun read
p3_cold_install 10.01s ±2.35 115.8K / 76.6K 10.76s, 122.7K / 56.4K 6.84s, 7.5K / 8.7K wall and iCtx regress; not useful
p4_warm_link 2.30s ±0.12 47.8K / 22.6K 2.46s, 42.3K / 18.7K 3.38s, 377 / 29 not positive; ctx remains high and worse than same-run utoo-next/npm

Conclusion: do not fold this as-is. The seeded-cache marker probe alone is not the p4 lever; #2965 points to clone worker scheduling as the useful boundary. This PR also has a mac e2e failure, but the Linux perf result is already enough to reject the experiment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@elrrrrrrr