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

make summary sync by using Arc not Rc #14260

Merged
merged 2 commits into from
Jul 17, 2024
Merged

Conversation

Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented Jul 16, 2024

What does this PR try to resolve?

For my PubGrub testing work I want to be able to read the entire crates.io index into memory and then run lots of resolution questions against that data in parallel. Currently cargoes Summary and Dependency types use Rc internally which prevents this pattern. Using Arc in cargo makes my life a lot easier! It does not noticeably slow down single threaded performance. (Measured by running my PubGrub testing in single threaded mode before and after.)

The team largely agreed to this at a meeting and in discussions https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Make.20summary.20sync

How should we test and review this PR?

Tests still pass

Additional information

Should be add a test a this is sync?

@rustbot
Copy link
Collaborator

rustbot commented Jul 16, 2024

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-crate-dependencies Area: [dependencies] of any kind S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 16, 2024
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Should be add a test a this is sync?

Yes please.

@weihanglo
Copy link
Member

Nice!

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 17, 2024

📌 Commit c27579a has been approved by weihanglo

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 Jul 17, 2024
@bors
Copy link
Collaborator

bors commented Jul 17, 2024

⌛ Testing commit c27579a with merge 52a2630...

@bors
Copy link
Collaborator

bors commented Jul 17, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 52a2630 to master...

@bors bors merged commit 52a2630 into rust-lang:master Jul 17, 2024
22 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 21, 2024
Update cargo

9 commits in a2b58c3dad4d554ba01ed6c45c41ff85390560f2..5f6b9a92201d78af75dc24f14662c3e2dacbbbe1
2024-07-16 00:52:02 +0000 to 2024-07-19 18:09:17 +0000
- Add `TomlPackage::new`, `Default` for `TomlWorkspace` (rust-lang/cargo#14271)
- fix(test): Move 'cargo_home' from 'install' to 'paths' (rust-lang/cargo#14270)
- fix(test)!: Clarify extension trait role with rename (rust-lang/cargo#14269)
- feat(test): Re-export ProcessBuilder (rust-lang/cargo#14268)
- fix(test): Move path2url to CargoPathExt::to_url (rust-lang/cargo#14266)
- Fix passing of links-overrides with target-applies-to-host and an implicit target (rust-lang/cargo#14205)
- fix(toml): Improve error on missing package and workspace (rust-lang/cargo#14261)
- Migrate global_cache_tracker snapbox (rust-lang/cargo#14244)
- make summary sync by using Arc not Rc (rust-lang/cargo#14260)
@rustbot rustbot added this to the 1.82.0 milestone Jul 21, 2024
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Jul 28, 2024
Update cargo

9 commits in a2b58c3dad4d554ba01ed6c45c41ff85390560f2..5f6b9a92201d78af75dc24f14662c3e2dacbbbe1
2024-07-16 00:52:02 +0000 to 2024-07-19 18:09:17 +0000
- Add `TomlPackage::new`, `Default` for `TomlWorkspace` (rust-lang/cargo#14271)
- fix(test): Move 'cargo_home' from 'install' to 'paths' (rust-lang/cargo#14270)
- fix(test)!: Clarify extension trait role with rename (rust-lang/cargo#14269)
- feat(test): Re-export ProcessBuilder (rust-lang/cargo#14268)
- fix(test): Move path2url to CargoPathExt::to_url (rust-lang/cargo#14266)
- Fix passing of links-overrides with target-applies-to-host and an implicit target (rust-lang/cargo#14205)
- fix(toml): Improve error on missing package and workspace (rust-lang/cargo#14261)
- Migrate global_cache_tracker snapbox (rust-lang/cargo#14244)
- make summary sync by using Arc not Rc (rust-lang/cargo#14260)
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 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