Skip to content

perf(pm): reduce install hot path overhead#2846

Closed
killagu wants to merge 1 commit into
nextfrom
agent/egg-dev/4ca3ec78
Closed

perf(pm): reduce install hot path overhead#2846
killagu wants to merge 1 commit into
nextfrom
agent/egg-dev/4ca3ec78

Conversation

@killagu
Copy link
Copy Markdown
Contributor

@killagu killagu commented Apr 27, 2026

Summary

  • avoid no-op binary mirror processing for official npm registry installs
  • reduce hot-path debug log volume during pm installs by moving per-package cache/clone/bin messages to trace
  • keep aggregate debug summaries for valid package paths and binary linking

Verification

  • cargo fmt
  • cargo build -p utoo-pm --profile release-local
  • cargo test -p utoo-pm
  • cargo clippy -p utoo-pm --all-targets -- -D warnings --no-deps
  • BENCH_RUNS=1 PM_LIST=utoo bash bench/pm-bench-phases.sh

Note: full workspace cargo clippy --all-targets -- -D warnings --no-deps is blocked in this environment by missing pkg-config/OpenSSL for unrelated pack dependencies (openssl-sys).

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 logging levels from debug to trace for several operations and introduces structured logging with counters for binary linking and package cleaning. Additionally, it adds a conditional check to skip binary mirroring. Feedback suggests refining the binary linking logic to ensure counters and success logs accurately reflect whether binaries were actually processed, rather than just checking if the package has binary definitions.

if !package.bin_files.is_empty() {
tracing::debug!("Linking binary files for {}", package.name);
tracing::trace!("Linking binary files for {}", package.name);
linked_packages += 1;
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 linked_packages counter is incremented for every package that has a non-empty bin_files list, regardless of whether any of those binaries are actually linked successfully. If all binaries for a package are skipped (e.g., because they don't exist on disk at line 292), the summary log at the end will still count this as a linked package. Consider incrementing this counter only if at least one binary was successfully processed for the package.

linked_bins += 1;
}
tracing::debug!("Linking binary files for {} successfully", package.name);
tracing::trace!("Linking binary files for {} successfully", package.name);
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

This trace log message is emitted even if no binaries were actually linked for the package (e.g., if all entries in bin_files were skipped due to missing files). This can be misleading when debugging. It would be better to only log this if at least one binary was successfully linked.

@elrrrrrrr
Copy link
Copy Markdown
Contributor

Closing as stale: this draft is a one-off agent experiment from 2026-04-27 with no follow-up, and overlaps with sibling PRs exploring the same optimization. Reopen if revisited.

@elrrrrrrr elrrrrrrr closed this May 25, 2026
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.

2 participants