Skip to content

perf: more efficient deno cache and npm package info usage #16592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 11, 2022

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Nov 11, 2022

  1. There was a lot of cloning going on with NpmPackageInfo. This is now stored in an Arc<NpmPackageInfo> and cloning only happens on the individual version.
  2. The package cache is now cleared from memory after resolution.
  3. This surfaced a bug in deno cache and I noticed it can be more efficient if we have multiple root specifiers if we provide all the specifiers as roots.

@dsherret dsherret requested a review from bartlomieju November 11, 2022 01:36
unresolved_tasks.push(tokio::task::spawn(async move {
let info = match maybe_info {
Some(info) => info?,
None => api.package_info(&package_req.name).await?,
Copy link
Member Author

Choose a reason for hiding this comment

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

This was previously had a high chance of making multiple requests for the same package when there were multiple different specifiers for the same package.

@@ -1,5 +1,4 @@
Download http://localhost:4545/npm/registry/chalk
Download http://localhost:4545/npm/registry/chalk/chalk-5.0.1.tgz
Copy link
Member Author

Choose a reason for hiding this comment

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

No longer downloads this twice...

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM! Nice fix

@dsherret dsherret merged commit 8dc242f into denoland:main Nov 11, 2022
@dsherret dsherret deleted the perf_more_efficient_npm_info branch November 11, 2022 16:34
bartlomieju pushed a commit to bartlomieju/deno that referenced this pull request Nov 12, 2022
…d#16592)

1. There was a lot of cloning going on with `NpmPackageInfo`. This is
now stored in an `Arc<NpmPackageInfo>` and cloning only happens on the
individual version.
2. The package cache is now cleared from memory after resolution.
3. This surfaced a bug in `deno cache` and I noticed it can be more
efficient if we have multiple root specifiers if we provide all the
specifiers as roots.
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