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

fix(resolver): avoid cloning when iterating using RcVecIter #14690

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

x-hgg-x
Copy link
Contributor

@x-hgg-x x-hgg-x commented Oct 15, 2024

What does this PR try to resolve?

Follow up of Eh2406/pubgrub-crates-benchmark#6 (comment).

This PR modifies the internal RcVecIter struct so that we can iterate on its items without cloning them. This improves performance of the resolver because eagerly cloning Arc<T> is not free.

Comparison of performance using solana-core = "=1.0.5" in Cargo.toml:

branch duration
master 213s
PR 202s

r? Eh2406

@rustbot rustbot added A-dependency-resolution Area: dependency resolution and the resolver S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2024
Copy link
Contributor

@Eh2406 Eh2406 left a comment

Choose a reason for hiding this comment

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

Overall, a good improvement. I'm surprised it makes such a difference. Thank you for finding it.

src/cargo/core/resolver/types.rs Outdated Show resolved Hide resolved
src/cargo/core/resolver/types.rs Outdated Show resolved Hide resolved
src/cargo/core/resolver/types.rs Outdated Show resolved Hide resolved
@x-hgg-x x-hgg-x force-pushed the resolver-perf-3 branch 2 times, most recently from f7f209f to e80553f Compare October 15, 2024 17:11
@Eh2406
Copy link
Contributor

Eh2406 commented Oct 15, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 15, 2024

📌 Commit 5acc1ca has been approved by Eh2406

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2024
@bors
Copy link
Collaborator

bors commented Oct 15, 2024

⌛ Testing commit 5acc1ca with merge 8040c00...

@bors
Copy link
Collaborator

bors commented Oct 15, 2024

☀️ Test successful - checks-actions
Approved by: Eh2406
Pushing 8040c00 to master...

@bors bors merged commit 8040c00 into rust-lang:master Oct 15, 2024
22 checks passed
@x-hgg-x x-hgg-x deleted the resolver-perf-3 branch October 15, 2024 19:16
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 19, 2024
Update cargo

7 commits in 8c30ce53688e25f7e9d860b33cc914fb2957ca9a..cf53cc54bb593b5ec3dc2be4b1702f50c36d24d5
2024-10-15 16:43:16 +0000 to 2024-10-18 13:56:15 +0000
- feat: Stabilize MSRV-aware resolver config (rust-lang/cargo#14639)
- Help with `[patch.crates.io]` (rust-lang/cargo#14700)
- test: Migrate publish snapshotting to snapbox (rust-lang/cargo#14642)
- Bump to 0.85.0; update changelog (rust-lang/cargo#14695)
- Fix typo in faq.md (rust-lang/cargo#14696)
- fix(registry): `HttpRegistry` `block_until_ready` returns early when work is still pending (rust-lang/cargo#14694)
- fix(resolver): avoid cloning when iterating using RcVecIter (rust-lang/cargo#14690)
@rustbot rustbot added this to the 1.84.0 milestone Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants