perf(pm): skip binary mirror lookup for npm registry#2852
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a check to skip binary mirror updates in the update_package_binary function. A review comment identifies a potential race condition where should_skip_binary_mirror() might cache a stale value before registry initialization, and suggests an alternative implementation to ensure the check is performed correctly.
| if should_skip_binary_mirror() { | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
Using should_skip_binary_mirror() here can lead to a race condition. Because it uses std::sync::OnceLock, it might cache a stale value if called before the async registry initialization is complete. This could cause it to incorrectly attempt a binary mirror lookup against the official npm registry.
To fix this, I suggest performing the check directly here. This avoids the caching issue, and the performance impact should be minimal. I've also preserved the debug logging from the original function.
A more comprehensive fix would be to make should_skip_binary_mirror async and use tokio::sync::OnceCell, which would solve this for all call sites.
| if should_skip_binary_mirror() { | |
| return Ok(()); | |
| } | |
| let registry = get_registry(); | |
| if is_npm_registry(®istry) { | |
| tracing::debug!("Skipping binary mirror update for npm registry: {}", registry); | |
| return Ok(()); | |
| } |
|
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. |
2 participants
Summary:
Verification:
Benchmark slice:
Risk: mirror rewriting is unchanged for npmmirror and other non-npm registries.