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.

Sign up for GitHub

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

Jump to bottom

Switch from AtomicU64 to Mutex. #12981

Merged
merged 1 commit into from
Nov 16, 2023
Merged

Switch from AtomicU64 to Mutex. #12981

merged 1 commit into from
Nov 16, 2023

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Nov 15, 2023

Not all platforms support AtomicU64, so this swaps the usage with Mutex. The difference in performance should be imperceptible.

@rustbot
Copy link
Collaborator

rustbot commented Nov 15, 2023

r? @epage

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 15, 2023
epage
epage approved these changes Nov 15, 2023
View reviewed changes
Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

r= me when its ready

@ehuss
Switch from AtomicU64 to Mutex.
1d35833 
Not all platforms support AtomicU64, so this swaps the usage with
Mutex. The difference in performance should be imperceptible.
@weihanglo
Copy link
Member

Going to merge this now as it blocks submodule update in rust-lang/rust

@bors r+

@bors
Copy link
Contributor

bors commented Nov 16, 2023

📌 Commit 1d35833 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 Nov 16, 2023
@bors
Copy link
Contributor

bors commented Nov 16, 2023

⌛ Testing commit 1d35833 with merge 4ecf479...

bors added a commit that referenced this pull request Nov 16, 2023
@bors
Auto merge of #12981 - ehuss:du-atomicu64, r=weihanglo
4ecf479 
Switch from AtomicU64 to Mutex.

Not all platforms support AtomicU64, so this swaps the usage with Mutex. The difference in performance should be imperceptible.
@bors
Copy link
Contributor

bors commented Nov 16, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 16, 2023
@Eh2406
Copy link
Contributor

Eh2406 commented Nov 16, 2023

I think there's a user adjustable clippy lint to ban individual types, do we want to ban the reintroduction of AtomicU##?

@ehuss
Copy link
Contributor Author

ehuss commented Nov 16, 2023

@bors retry

Looks like these jobs are updating Cargo.lock to the latest versions. There appears to have been a bad release of primeorder which have been yanked, so I think it should pass.

(I don't know why those jobs are updating Cargo.lock, though.)

@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 Nov 16, 2023
@bors
Copy link
Contributor

bors commented Nov 16, 2023

⌛ Testing commit 1d35833 with merge a100226...

@ehuss
Copy link
Contributor Author

ehuss commented Nov 16, 2023

I think there's a user adjustable clippy lint to ban individual types, do we want to ban the reintroduction of AtomicU##?

If someone wants to add that, it's probably ok. However, I don't know how that would interact if it is made conditional on target_has_atomic, in case someone wanted to have different variants for different platforms. I think 32 and smaller is available on all our platforms, it's just 64 and bigger.

@bors
Copy link
Contributor

bors commented Nov 16, 2023

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

@bors bors merged commit a100226 into rust-lang:master Nov 16, 2023
22 checks passed
@weihanglo weihanglo mentioned this pull request Nov 16, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 16, 2023
@bors
Auto merge of rust-lang#117916 - weihanglo:update-cargo, r=weihanglo
525c91d 
Update cargo

19 commits in 6790a5127895debec95c24aefaeb18e059270df3..2c03e0e2dcd05dd064fcf10cc1050d342eaf67e3
2023-11-10 17:09:35 +0000 to 2023-11-16 04:21:44 +0000
- docs(ref): Find a place to comment on --cap-lints (rust-lang/cargo#12976)
- Switch from AtomicU64 to Mutex. (rust-lang/cargo#12981)
- If the only path is a loop then counted as the shortest path. (rust-lang/cargo#12977)
- fix(resolver): Prefer MSRV, rather than ignore incompatible (rust-lang/cargo#12950)
- fix error message for duplicate links (rust-lang/cargo#12973)
- Only filter out target if its in the package root (rust-lang/cargo#12944)
- Ignore changing_spec_relearns_crate_types on windows-gnu (rust-lang/cargo#12972)
- fix: do not panic when failed to parse rustc commit-hash (rust-lang/cargo#12965)
- query{_vec} use IndexSummary (rust-lang/cargo#12970)
- Bump to 0.77.0; update changelog (rust-lang/cargo#12966)
- Improve about information of `cargo search` (rust-lang/cargo#12962)
- Fix --quiet being used with nested subcommands. (rust-lang/cargo#12959)
- make some debug assertion failures more informative (rust-lang/cargo#12963)
- refactor(toml): Consistently lead with 'Toml' prefix (rust-lang/cargo#12960)
- refactor(toml): Remove unused method (rust-lang/cargo#12961)
- Fix non-deterministic behavior in last-use repopulation (rust-lang/cargo#12958)
- Add cache garbage collection (rust-lang/cargo#12634)
- refactor(toml): Improve consistency (rust-lang/cargo#12954)
- Fix typo (rust-lang/cargo#12956)
epage added a commit to epage/cargo that referenced this pull request Nov 16, 2023
@epage
chore(ci): Catch naive use of AtomicU64 early
93cd5ef 
AtomicU64 is not portable and use of it, without `target_has_atomic`,
will fail CI for rust-lang/rust when the cargo submodule gets updated
(see rust-lang#12981).

If there is a reason to use it, we could always `allow` in that one
case.
@epage epage mentioned this pull request Nov 16, 2023
Merged
bors added a commit that referenced this pull request Nov 16, 2023
@bors
Auto merge of #12988 - epage:clippy-type, r=weihanglo
be9f24d 
chore(ci): Catch naive use of AtomicU64 early

AtomicU64 is not portable and use of it, without `target_has_atomic`, will fail CI for rust-lang/rust when the cargo submodule gets updated (see #12981).

If there is a reason to use it, we could always `allow` in that one case.
@ehuss ehuss added this to the 1.76.0 milestone Dec 6, 2023
@weihanglo weihanglo mentioned this pull request Jan 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epage epage epage approved these changes

Assignees

@epage epage

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.76.0
Development

Successfully merging this pull request may close these issues.
6 participants
@ehuss @rustbot @weihanglo @bors @Eh2406 @epage