Skip to content
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

Loading of a git source is slow due to manifest parsing #14395

Open
epage opened this issue Aug 13, 2024 · 3 comments
Open

Loading of a git source is slow due to manifest parsing #14395

epage opened this issue Aug 13, 2024 · 3 comments
Labels
A-crate-dependencies Area: [dependencies] of any kind A-git Area: anything dealing with git A-patch Area: [patch] table override C-enhancement Category: enhancement Performance Gotta go fast! S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@epage
Copy link
Contributor

epage commented Aug 13, 2024

When you have some git dependencies or patches, loading them takes a long time

For the traces from #14238, notice how slow the first resolve is compared to the second. Almost all of that is taken up in Manifest parsing
image

Note: this slow down is most relevant for tooling like rust-analyzer or rust-defined completions when it may be called in interactive contexts without any compilation happening.

Related: #13724, #14603

@epage epage added A-git Area: anything dealing with git A-crate-dependencies Area: [dependencies] of any kind A-patch Area: [patch] table override Performance Gotta go fast! labels Aug 13, 2024
@epage epage changed the title Loading of a git source is slow due to manifest parsi Loading of a git source is slow due to manifest parsing Aug 13, 2024
@epage
Copy link
Contributor Author

epage commented Aug 13, 2024

@epage
Copy link
Contributor Author

epage commented Aug 13, 2024

The core problem is that git sources are indexed by name. To find a name, we walk the source to find all Cargo.toml files except if they are in a submodule or a hidden directory except if a Cargo.toml has a path dependency in it in an ignored location.

Without that second exception, we could parse Cargo.toml to toml::Table, look up the name, and be done. That would cut out probably 60% of the manifest parsing performance hit.

With the second exception, we have to do a full parse of the Cargo.toml file.

Ideas

  • Allow dependencies to specify the path within the repo (Git dependency with a path into a sub-folder location #11858)
    • Needs design work
    • Requires people to opt-in to get performance gain
  • Store the path in Cargo.lock
  • Store a cache in .cargo-ok
    • Content in the file shouldn't cause checkout thrashing
    • Being in that file keeps the checkout self-contained (delete it and everything is gone) and avoids fingerprinting issues with old cargo (they ignore this file)
    • Likely should only have one entry (thrashing on version changes) as a simple reclamation policy
    • Either fingerprint cargo like we do rustc or use the git commit sha if its not dirty, not caching when it is dirty
  • Tell people to avoid ambiguous names, or test files (only reduces part of the problem)
  • A file in the repo that indexes it (require opt-in by the dependency)
  • A .cargoignmore file in the repo for what not to index (requires opt-in by the dependency)
  • Speed up parsing by caching immutable Cargo.tomls in a faster format (likely cut 1/6 of parse time out)

@epage epage added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. C-enhancement Category: enhancement labels Aug 13, 2024
@epage
Copy link
Contributor Author

epage commented Aug 13, 2024

Main risk: there aren't enough manifests for us to skip loading

@epage epage added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review labels Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-crate-dependencies Area: [dependencies] of any kind A-git Area: anything dealing with git A-patch Area: [patch] table override C-enhancement Category: enhancement Performance Gotta go fast! S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

1 participant