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

Replace generic thread parker with explicit no-op parker #105695

Merged
merged 1 commit into from
May 4, 2023

Conversation

joboet
Copy link
Member

@joboet joboet commented Dec 14, 2022

With #98391 merged, all platforms supporting threads now have their own parking implementations. Therefore, the generic implementation can be removed. On the remaining platforms (really just WASM without atomics), parking is not supported, so calls to thread::park now return instantly, which is allowed by their API. This is a change in behaviour, as spurious wakeups do not currently occur since all platforms guard against them. It is invalid to depend on this, but I'm still going to tag this as libs-api for confirmation.

@rustbot label +T-libs +T-libs-api +A-atomic

r? rust-lang/libs

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 14, 2022
@rustbot
Copy link
Collaborator

rustbot commented Dec 14, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@joboet

This comment was marked as duplicate.

@rustbot rustbot added A-atomic Area: Atomics, barriers, and sync primitives T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 31, 2023
@m-ou-se
Copy link
Member

m-ou-se commented Feb 13, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Feb 13, 2023

📌 Commit 8809e72fbf6c414173f206a5144175074a2613e0 has been approved by m-ou-se

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 Feb 13, 2023
@m-ou-se m-ou-se assigned m-ou-se and unassigned joshtriplett Feb 13, 2023
@bors
Copy link
Contributor

bors commented Feb 15, 2023

⌛ Testing commit 8809e72fbf6c414173f206a5144175074a2613e0 with merge d78f962ac77dbb846a67366d5f450f72b8251253...

@bors
Copy link
Contributor

bors commented Feb 15, 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 Feb 15, 2023
@ChrisDenton
Copy link
Member

@bors retry error: failed to download llvm from ci

@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 Feb 15, 2023
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 16, 2023

☔ The latest upstream changes (presumably #108116) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 16, 2023
@joboet
Copy link
Member Author

joboet commented Feb 16, 2023

Rebased onto master.
@rustbot ready

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 16, 2023
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 16, 2023
@m-ou-se
Copy link
Member

m-ou-se commented May 1, 2023

@bors r+

@bors
Copy link
Contributor

bors commented May 1, 2023

📌 Commit 9622cde has been approved by m-ou-se

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 May 1, 2023
Manishearth added a commit to Manishearth/rust that referenced this pull request May 3, 2023
…ou-se

Replace generic thread parker with explicit no-op parker

With rust-lang#98391 merged, all platforms supporting threads now have their own parking implementations. Therefore, the generic implementation can be removed. On the remaining platforms (really just WASM without atomics), parking is not supported, so calls to `thread::park` now return instantly, which is [allowed by their API](https://doc.rust-lang.org/nightly/std/thread/fn.park.html). This is a change in behaviour, as spurious wakeups do not currently occur since all platforms guard against them. It is invalid to depend on this, but I'm still going to tag this as libs-api for confirmation.

`@rustbot` label +T-libs +T-libs-api +A-atomic

r? rust-lang/libs
Manishearth added a commit to Manishearth/rust that referenced this pull request May 3, 2023
…ou-se

Replace generic thread parker with explicit no-op parker

With rust-lang#98391 merged, all platforms supporting threads now have their own parking implementations. Therefore, the generic implementation can be removed. On the remaining platforms (really just WASM without atomics), parking is not supported, so calls to `thread::park` now return instantly, which is [allowed by their API](https://doc.rust-lang.org/nightly/std/thread/fn.park.html). This is a change in behaviour, as spurious wakeups do not currently occur since all platforms guard against them. It is invalid to depend on this, but I'm still going to tag this as libs-api for confirmation.

``@rustbot`` label +T-libs +T-libs-api +A-atomic

r? rust-lang/libs
Manishearth added a commit to Manishearth/rust that referenced this pull request May 3, 2023
…ou-se

Replace generic thread parker with explicit no-op parker

With rust-lang#98391 merged, all platforms supporting threads now have their own parking implementations. Therefore, the generic implementation can be removed. On the remaining platforms (really just WASM without atomics), parking is not supported, so calls to `thread::park` now return instantly, which is [allowed by their API](https://doc.rust-lang.org/nightly/std/thread/fn.park.html). This is a change in behaviour, as spurious wakeups do not currently occur since all platforms guard against them. It is invalid to depend on this, but I'm still going to tag this as libs-api for confirmation.

```@rustbot``` label +T-libs +T-libs-api +A-atomic

r? rust-lang/libs
bors added a commit to rust-lang-ci/rust that referenced this pull request May 4, 2023
…earth

Rollup of 10 pull requests

Successful merges:

 - rust-lang#97594 (Implement tuple<->array convertions via `From`)
 - rust-lang#105452 (Add cross-language LLVM CFI support to the Rust compiler)
 - rust-lang#105695 (Replace generic thread parker with explicit no-op parker)
 - rust-lang#110371 (rustdoc: restructure type search engine to pick-and-use IDs)
 - rust-lang#110928 (tests: Add tests for LoongArch64)
 - rust-lang#110970 (tidy: remove ENTRY_LIMIT maximum checking and set it to 900)
 - rust-lang#111104 (Update ICU4X to 1.2)
 - rust-lang#111127 (Constify slice flatten method)
 - rust-lang#111146 (rustc_middle: Fix `opt_item_ident` for non-local def ids)
 - rust-lang#111154 (Use builtin FFX isolation for Fuchsia test runner)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3fa0c08 into rust-lang:master May 4, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 4, 2023
@joboet joboet deleted the remove_generic_parker branch May 4, 2023 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-atomic Area: Atomics, barriers, and sync primitives S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants